799308 - Mach command for running marionette tests

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Gregory Szorc [:gps]]

Attachment: Proof of concept

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.

Comment 1

[Jonathan Griffin (:jgriffin)]

Sweet!

Comment 3

[Jonathan Griffin (:jgriffin)]

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.

Comment 4

[Jonathan Griffin (:jgriffin)]

Attachment: Mach command for running Marionette,

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.

Comment 5

[Gregory Szorc [: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?

Comment 6

[Jonathan Griffin (:jgriffin)]

(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.

Comment 7

[Jonathan Griffin (:jgriffin)]

Attachment: Mach command for running Marionette,

Address review comments.  Will revisit after bug 856392 lands.

Comment 8

[Jonathan Griffin (:jgriffin)]

Comment on attachment 739546 [details] [diff] [review]
Mach command for running Marionette,

Carry r+ forward.

Comment 9

[Jonathan Griffin (:jgriffin)]

Updated for some bitrot and pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=545926ead0f9

Comment 10

[Jonathan Griffin (:jgriffin)]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6c1668dfaaed