Closed Bug 926693 Opened 6 years ago Closed 6 years ago

Add B2G support to mach's marionette command

Categories

(Testing :: Marionette, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla27

People

(Reporter: jgriffin, Assigned: jgriffin)

References

Details

Attachments

(1 file)

We already have a mach command for Marionette on desktop Firefox ... bug 799308 ... let's add B2G support for it.
Attachment #816855 - Flags: review?(ahalberstadt)
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.
Attachment #816855 - Flags: review?(ahalberstadt) → review+
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).
Target Milestone: --- → mozilla27
https://hg.mozilla.org/mozilla-central/rev/15e91a96d77d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Blocks: 929121
You need to log in before you can comment on or make changes to this bug.