Auditing dotest's command line options

I would like to do a complete audit of dotest’s command line options, find out who’s using what, and then potentially delete anything that isn’t being used. There’s a mess of command line options in use, to the point that it’s often hard to find free letters to use for new options.

I created this spreadsheet with a complete list of command line options, their descriptions, and a place for people to enter what options they’re using or do not want to be deleted.

https://docs.google.com/spreadsheets/d/1wkxAY7l0_cJOHhhsSlh3aKKlQShlX1D7X1Dn8kpqxy4/edit?usp=sharing

If someone has already written YES in the box that indicates they need the option, please don’t overwrite it. If you write YES in a box, please provide at least a small rationale for why this option is useful to you. Feel free to add additional rationale if someone has already added some rationale.

I’m going to have a couple days in mid-December and do this cleanup, so I’d like to get a solid picture of what options are not needed before then. After people have had some time to look over this, I’ll go through the results and decide what to do with each one, and then send out another email with a proposed action column for each command line option.

Please do take the time to have a look at this, because any option that doesn’t have a YES in it after a couple of weeks I’m going to assume is a candidate for deletion.

We’re reviewing these. I’ve added the ones that I use or need for infrastructure. I’ve pinged everyone else internally and we’ll be sure to update any others over today and tomorrow.

-Todd

Zach, I would also like to get rid of all global variables in the process of this change. The history goes like this: a long time ago someone wrote the initial dotest.py and parsed the options manually and stored results in global variables. Later, someone converted the options over to use a python library to parse the options, but we mostly copied the options from the options dictionary over into the globals and still use the globals all over the code. It would be great if we had at most one global variable that is something like "g_options" and anyone that was using any global variables will switch over to use the "g_options.XXXX" instead. Then we don't have to make copies and we can let the g_options contain all settings that are required.

Seems reasonable. I will make a best effort to get as many of them as I can.

Yeah that would be awesome, and also would be a lot of work. Good luck!

I have updated the list of command line options with a proposed action. It looks like exactly half of the command line options can be deleted, which is going to be a huge win for being able to understand what the test suite does and how to use it effectively. Please take a look at the updated spreadsheet and make final comments. Last change to keep an option if I’ve marked it as delete.

https://docs.google.com/spreadsheets/d/1wkxAY7l0_cJOHhhsSlh3aKKlQShlX1D7X1Dn8kpqxy4/edit?pli=1#gid=0

For a few of the ones people wrote that they use the option, but it seems like the same functionality is provided by a different option. I marked these as PREFER DELETE, but feel free to comment here if you still think it should remain. I’ll tackle those last and make sure there’s consensus that it’s ok to delete them before doing so.

If there’s no otehr comments, I plan to start working on this on Wednesday.

Hi Greg,

Take a look at dotest.py next time you get some free time and let me know what you think. There should be no more globals. Everything that used to be a global is now stored in its own module configuration.py, and everything in configuration.py can be referenced from everywhere in the entire test suite.

Do we not want to have an "options" global variable in this module that contains everything instead of having separate global variables in this file? The idea would be that you could assign directly when parsing arguments:

(configuration.options, args) = parser.parse_args(sys.argv[1:])

Its OK if we don't do this, but this is what I was originally thinking. Then we don't need to do any transfer out of the options dictionary that is returned by the option parser. The drawback with this approach is the "configuration.options" would probably need to be initialized in case someone tries to access the "configuration.options" without first parsing arguments. So in that respect the global approach is nicer.

Greg

There’s no way to avoid doing a transfer out of the options dictionary at some level, because it’s not a straight transfer. There’s a ton of post-processing that gets done on the options dictionary in order to convert the raw options into a useful format.

That might be solvable with more advanced use of argparse. This approach does get rid of one level of option transfer though. Because we would transfer

  1. From the class returned by argparse into the global
  2. From the global into the lldb module

Now we only transfer from the argparse class into the configuration module, and everything else just uses that.

One advantage of this approach is that it makes the options available to the entire test suite. Even if we have no transferring going on, and we get argparse to return us a perfectly organized structure with everything in the right format, in order to make all the options accessible to the rest of the test suite, we still need to stick it in a global module somewhere. And then you would write configuration.options.test_categories, whereas with this approach we just write configuration.test_categories. It’s a minor point, but I like the shorter member access personally.

Sounds good, looks good then.

I think it’s a nice improvement.

Passing the options around via the argparse results (as I do in many programs) makes it easier to unit test, but having configuration variables all in a module makes it really simple to find and use everywhere without having them as globals.

Thanks for cleaning that up, Zachary!

-Todd