926693 - Add B2G support to mach's marionette command

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Jonathan Griffin (:jgriffin)]

We already have a mach command for Marionette on desktop Firefox ... bug 799308 ... let's add B2G support for it.

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: Add B2G support to marionette's mach command,

Comment 2

[Andrew Halberstadt [:ahal] (PTO Until Apr 3rd)]

Comment on attachment 816855 [details] [diff] [review]
Add B2G support to marionette's mach command,

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

Lgtm with a few comments.

::: testing/marionette/mach_commands.py
@@ +27,5 @@
> +    parser = MarionetteTestOptions()
> +    options, args = parser.parse_args()
> +
> +    if not tests:
> +        tests = [os.path.join(topsrcdir, 

nit: whitespace

@@ +74,5 @@
> +            emulator='arm'
> +
> +        if self.substs.get('ENABLE_MARIONETTE') != '1':
> +            print("Marionette doesn't appear to be enabled; please "
> +                  "make sure you're using an engineering build.")

Why do we say to use an eng build here, but to rebuild with ENABLE_MARIONETTE=1 for desktop? Wouldn't putting ENABLE_MARIONETTE=1 in the .userconfig also work? (That's an actual question, I'm not sure). If not, a hint on how to build or download an eng build would be nice.

@@ +98,5 @@
> +        try:
> +            bin = self.get_binary_path('app')
> +        except Exception as e:
> +            print("It looks like your program isn't built.",
> +                  "You can run |mach build| to build it.")

By putting in the is_firefox condition, this command should be disabled if there is no build so I think this check is unnecessary. Though I guess it doesn't hurt to just leave it anyway.

@@ +105,5 @@
>  
> +        if self.substs.get('ENABLE_MARIONETTE') != '1':
> +            print("Marionette doesn't appear to be enabled; please "
> +                  "add ENABLE_MARIONETTE=1 to your mozconfig and "
> +                  "perform a clobber build.")

Could you change this to be consistent with the message in http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/mach_commands.py#60 ? I like the mach commands to be as helpful as possible when something goes wrong.

Comment 3

[Jonathan Griffin (:jgriffin)]

Review comments addressed; will land when inbound opens.

> Why do we say to use an eng build here, but to rebuild with ENABLE_MARIONETTE=1 for desktop? Wouldn't 
> putting ENABLE_MARIONETTE=1 in the .userconfig also work? (That's an actual question, I'm not sure). 
> If not, a hint on how to build or download an eng build would be nice.

Basically, because user builds (the opposite of eng builds) aren't a supported configuration.  It's possible it would work, but no one is using Marionette with a user build, so it's quite possible that the combination will break in the future.  I've added a note about how to build an engineering build (which is the default).

Comment 4

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/15e91a96d77d

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/15e91a96d77d