Closed Bug 1082686 Opened 5 years ago Closed 2 years ago

Refactor marionette runner's option handling


(Testing :: Marionette, defect, P3)



(Not tracked)



(Reporter: ahal, Unassigned)



(Keywords: pi-marionette-runner)

Currently the default options for the marionette runner live here:

This class is a bit clumsy to override. For example, there is no way to remove options that a suite doesn't care about (e.g the emulator related ones). I'd propose a general refactor of how options are handled where we do the following:

1) Move option related logic into its own file.
2) Upgrade to argparse (we have 2.7 across the board, right?)
3) Pull out options that aren't generically useable into their own subclasses (or mixins or whatever)
4) Use technique similar to [1] so it's easy to concatenate and/or slice options.

Any objections?

(In reply to Andrew Halberstadt [:ahal] from comment #0)
> 2) Upgrade to argparse (we have 2.7 across the board, right?)

I don't think so. At least bug 711299 is still open.
We are using 2.7.x on all buildbot slaves.
Hmm, I don't see why we are building something like a "generic and extendable" command line stuff here. For example:

this is about mixins, but I don't see this BaseMarionetteOptions class used in that way anywhere. only mach seems to use that class outside of marionette_client, and not in that way.

I would vote for refactoring all this stuff by moving to argparse, remove the "generic and extendable" stuff and just offer a single "main" or "cli" entry point function that mach would use (and the marionnette command line also).

What do you think ? And is this not possible for some reason that I am missing ?
Yeah, it has some out of tree consumers in gaia at least, and likely other random Github repos as well:
Hmm, thanks :ahal. I only checked into mozilla-central...

Well, forget my previous comment. :)
Note the work in bug 1163801 if you're thinking of tackling this.
[mass update] Setting Harness bugs to all P3
Priority: -- → P3
We will probably never get around to fix this considering the vast number of dependencies.
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.