Mach command for running marionette tests

RESOLVED FIXED in mozilla23

Status

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: gps, Assigned: jgriffin)

Tracking

unspecified
mozilla23
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mach])

Attachments

(1 attachment, 2 obsolete attachments)

Posted patch Proof of concept (obsolete) — Splinter Review
Marionette should get the mach treatment.

The attached patch shows about what it takes to hook something up to mach.

To do this right will require a little more refactoring of runtests.py and possibly mach. But, I'm up to the challenge.
Sweet!
Duplicate of this bug: 840007
Blocks: b2g-testing
See Also: → 853250
We should make this mach command smart enough so that it can tell if Mn is enabled in the build (by inspecting mozconfig?) and telling people what to do to fix it if it isn't.  See bug 861015.
Depends on: 860091
Depends on: 794506
No longer depends on: 794506
I took your prototype and fleshed it out a bit...it tests out ok for me.  Ideally, we'd have mach in the B2G repo with its own commands, and the emulator stuff could live there, but that's a bigger problem and another bug.  This should get us going in the short term.
Attachment #738494 - Flags: review?(gps)
Comment on attachment 738494 [details] [diff] [review]
Mach command for running Marionette,

Review of attachment 738494 [details] [diff] [review]:
-----------------------------------------------------------------

I don't believe I have r+ power for runtests.py. Therefore, my r+ doesn't cover it.

But, mach_bootstrap.py and mach_commands.py look good aside from the points addressed.

You may also be interested in the patch in bug 856392 which is changing how mach command dispatching works. I don't think it will bit rot you. But, it makes it easier for us to hook up more powerful command-line processing. I'm thinking about allowing mach commands to say "pass arguments to this function" (as opposed to requiring commands to define all the arguments). We could leverage this to allow test harnesses to continue parsing low-level arguments, complete with help/usage info via mach! Something to think about.

::: testing/marionette/mach_commands.py
@@ +19,5 @@
> +    @Command('marionette-test', help='Run a Marionette test.')
> +    @CommandArgument('--homedir', dest='b2g_path',
> +        help='For B2G testing, the path to the B2G repo.')
> +    @CommandArgument('--emulator', choices=['x86', 'arm'],
> +        help='Run an emulator of the specMachCommandBaseified architecture. This will only'

"specMachCommandBaseified"

@@ +75,5 @@
> +            if 'ENABLE_MARIONETTE' not in config:
> +                print("Marionette doesn't appear to be enabled; please "
> +                      "add ENABLE_MARIONETTE=1 to your mozconfig and "
> +                      "perform a clobber build.")
> +                return 1

We actually have an API for accessing config.status natively! Since this command class inherits from MachCommandBase, you can do something like:

   if 'ENABLE_MARIONETTE' in self.substs:

or

   if 'ENABLE_MARIONETTE' in self.defines:

(I can't remember exactly what the data structures are or where ENABLE_MARIONETTE is defined.)

@@ +83,5 @@
> +        options, tests = verify_parser_result(tests, options)
> +
> +        runner = startTestRunner(MarionetteTestRunner, options, tests)
> +        if runner.failed > 0:
> +            return 10

10?
Attachment #738494 - Flags: review?(gps) → review+
(In reply to Gregory Szorc [:gps] from comment #5)
> Comment on attachment 738494 [details] [diff] [review]
> Mach command for running Marionette,
> 
> Review of attachment 738494 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't believe I have r+ power for runtests.py. Therefore, my r+ doesn't
> cover it.
> 
> But, mach_bootstrap.py and mach_commands.py look good aside from the points
> addressed.
> 
> You may also be interested in the patch in bug 856392 which is changing how
> mach command dispatching works. I don't think it will bit rot you. But, it
> makes it easier for us to hook up more powerful command-line processing. I'm
> thinking about allowing mach commands to say "pass arguments to this
> function" (as opposed to requiring commands to define all the arguments). We
> could leverage this to allow test harnesses to continue parsing low-level
> arguments, complete with help/usage info via mach! Something to think about.

Awesome, I was thinking while writing this it would be really nice to have such a feature.

> 
> ::: testing/marionette/mach_commands.py
> @@ +19,5 @@
> > +    @Command('marionette-test', help='Run a Marionette test.')
> > +    @CommandArgument('--homedir', dest='b2g_path',
> > +        help='For B2G testing, the path to the B2G repo.')
> > +    @CommandArgument('--emulator', choices=['x86', 'arm'],
> > +        help='Run an emulator of the specMachCommandBaseified architecture. This will only'
> 
> "specMachCommandBaseified"

Haha, I crack myself up.

> 
> @@ +75,5 @@
> > +            if 'ENABLE_MARIONETTE' not in config:
> > +                print("Marionette doesn't appear to be enabled; please "
> > +                      "add ENABLE_MARIONETTE=1 to your mozconfig and "
> > +                      "perform a clobber build.")
> > +                return 1
> 
> We actually have an API for accessing config.status natively! Since this
> command class inherits from MachCommandBase, you can do something like:
> 
>    if 'ENABLE_MARIONETTE' in self.substs:
> 
> or
> 
>    if 'ENABLE_MARIONETTE' in self.defines:
> 
> (I can't remember exactly what the data structures are or where
> ENABLE_MARIONETTE is defined.)

Great, thanks!

> 
> @@ +83,5 @@
> > +        options, tests = verify_parser_result(tests, options)
> > +
> > +        runner = startTestRunner(MarionetteTestRunner, options, tests)
> > +        if runner.failed > 0:
> > +            return 10
> 
> 10?

It's what runtests.py does.  It's sort of a special value that clues the mozharness script that there were test failures, vs some other kind of failure.  In this case we don't need to do this, since mozharness won't be calling the mach command.
Address review comments.  Will revisit after bug 856392 lands.
Attachment #738494 - Attachment is obsolete: true
Attachment #669361 - Attachment is obsolete: true
Comment on attachment 739546 [details] [diff] [review]
Mach command for running Marionette,

Carry r+ forward.
Attachment #739546 - Flags: review+
Assignee: gps → jgriffin
Updated for some bitrot and pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=545926ead0f9
Try is green.

Most of the Marionette changes were landed earlier in bug 842633, so this contains only a few Mn tweaks to get it to work nicely with mach.

I'll update in a separate patch for bug 856392 if needed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c1668dfaaed
Target Milestone: --- → mozilla23
https://hg.mozilla.org/mozilla-central/rev/6c1668dfaaed
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.