The default bug view has changed. See this FAQ.

Mach command for running marionette tests

RESOLVED FIXED in mozilla23

Status

Testing
Marionette
RESOLVED FIXED
5 years ago
4 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)

(Reporter)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
Sweet!
(Reporter)

Updated

4 years ago
Duplicate of this bug: 840007
(Assignee)

Updated

4 years ago
Blocks: 846091
See Also: → bug 853250
(Assignee)

Comment 3

4 years ago
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.
(Assignee)

Updated

4 years ago
Depends on: 860091
(Assignee)

Updated

4 years ago
Depends on: 794506
(Assignee)

Updated

4 years ago
No longer depends on: 794506
(Assignee)

Comment 4

4 years ago
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.
Attachment #738494 - Flags: review?(gps)
(Reporter)

Comment 5

4 years ago
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+
(Assignee)

Comment 6

4 years ago
(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.
(Assignee)

Comment 7

4 years ago
Created attachment 739546 [details] [diff] [review]
Mach command for running Marionette,

Address review comments.  Will revisit after bug 856392 lands.
(Assignee)

Updated

4 years ago
Attachment #738494 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Attachment #669361 - Attachment is obsolete: true
(Assignee)

Comment 8

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

Carry r+ forward.
Attachment #739546 - Flags: review+
(Assignee)

Updated

4 years ago
Assignee: gps → jgriffin
(Assignee)

Comment 9

4 years ago
Updated for some bitrot and pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=545926ead0f9
(Assignee)

Comment 10

4 years ago
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.