Last Comment Bug 799308 - Mach command for running marionette tests
: Mach command for running marionette tests
Status: RESOLVED FIXED
[mach]
:
Product: Testing
Classification: Components
Component: Marionette (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla23
Assigned To: Jonathan Griffin (:jgriffin)
:
Mentors:
: 840007 (view as bug list)
Depends on: 799262 860091
Blocks: b2g-testing
  Show dependency treegraph
 
Reported: 2012-10-08 18:06 PDT by Gregory Szorc [:gps]
Modified: 2013-05-07 19:42 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proof of concept (5.10 KB, patch)
2012-10-08 18:06 PDT, Gregory Szorc [:gps]
no flags Details | Diff | Splinter Review
Mach command for running Marionette, (17.54 KB, patch)
2013-04-17 07:30 PDT, Jonathan Griffin (:jgriffin)
gps: review+
Details | Diff | Splinter Review
Mach command for running Marionette, (17.11 KB, patch)
2013-04-19 05:20 PDT, Jonathan Griffin (:jgriffin)
jgriffin: review+
Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2012-10-08 18:06:32 PDT
Created attachment 669361 [details] [diff] [review]
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) 2012-10-09 11:34:08 PDT
Sweet!
Comment 2 Gregory Szorc [:gps] 2013-02-11 11:25:57 PST
*** Bug 840007 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Griffin (:jgriffin) 2013-04-11 17:54:26 PDT
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) 2013-04-17 07:30:23 PDT
Created attachment 738494 [details] [diff] [review]
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] 2013-04-17 11:22:48 PDT
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) 2013-04-18 00:47:58 PDT
(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) 2013-04-19 05:20:50 PDT
Created attachment 739546 [details] [diff] [review]
Mach command for running Marionette,

Address review comments.  Will revisit after bug 856392 lands.
Comment 8 Jonathan Griffin (:jgriffin) 2013-04-19 05:21:47 PDT
Comment on attachment 739546 [details] [diff] [review]
Mach command for running Marionette,

Carry r+ forward.
Comment 9 Jonathan Griffin (:jgriffin) 2013-05-03 10:32:57 PDT
Updated for some bitrot and pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=545926ead0f9
Comment 10 Jonathan Griffin (:jgriffin) 2013-05-06 14:03:32 PDT
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] 2013-05-07 19:42:07 PDT
https://hg.mozilla.org/mozilla-central/rev/6c1668dfaaed

Note You need to log in before you can comment on or make changes to this bug.