Closed Bug 950641 Opened 10 years ago Closed 10 years ago

Automatically use the appropriate GeckoInstance class

Categories

(Remote Protocol :: Marionette, defect)

Other
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v1.3 fixed)

RESOLVED FIXED
mozilla29
Tracking Status
b2g-v1.3 --- fixed

People

(Reporter: zcampbell, Assigned: davehunt)

Details

Attachments

(1 file, 2 obsolete files)

We get regular complaints about the difficulty in running the gaia-ui-tests. 

While command line options are nice for flexibility they are the opposite of user experience; with fewer CL options we can make it easier to get up and running in the limited amount of scenarios that cover the majority of user cases.

When running from gaiatest if a binary is declared on the command line then we can assume that that binary is b2gdesktop. Thus we can initialise Marionette with `app=b2gdesktop` without having to pass it in from the command line.

If it's not a b2gdesktop binary then it's going to fail anyway so we don't really need to worry about that scenario.
Rather than setting the app value based on the binary, I would be more inclined to use the application.ini located in the binary path to identify the application in use and select the appropriate gecko instance class. We could retain the command line argument for overriding this, or deprecate and remove it.

The relevent code is here: http://hg.mozilla.org/mozilla-central/file/0d11fce4f845/testing/marionette/client/marionette/marionette.py#l454
Whiteboard: [mentor=davehunt][lang=js]
Component: Gaia::UI Tests → Marionette
Product: Firefox OS → Testing
Summary: Init Marionette with app=desktopb2g when a binary is passed into it → Automatically use the appropriate GeckoInstance class
It's also possible we could use the future mozversion package discussed in bug 928452 to identify the application from the binary.
Can't we just instantiate BaseMarionetteTestRunner in gaiatest with app=b2gdesktop if binary exists? 

Let's just do the simple solution rather than the complicated one.
Attached patch bug950641.patch (obsolete) — Splinter Review
I decided to keep the --app command line argument so it's still possible to override, however I have changed 'b2gdesktop' to 'b2g'. This means that anyone using --app=b2gdesktop (TBPL and Travis) will get an error. I would be happy to add this back in until we've transitioned off the argument, or even to keep it as legacy code.

The error is pretty clear, so hopefully will be easy for any users to see how to fix: "NotImplementedError: Application "b2gdesktop" unknown (should be one of ['b2g'])"
Assignee: nobody → dave.hunt
Status: NEW → ASSIGNED
Attachment #8348778 - Flags: review?(jgriffin)
Whiteboard: [mentor=davehunt][lang=js]
Last patch was missing commit message.
Attachment #8348778 - Attachment is obsolete: true
Attachment #8348778 - Flags: review?(jgriffin)
Attachment #8348779 - Flags: review?(jgriffin)
Comment on attachment 8348779 [details] [diff] [review]
Automatically use the appropriate GeckoInstance class. v1.0

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

Looks good.  We should also fix this call site in the same patch:

http://mxr.mozilla.org/mozilla-central/source/testing/marionette/mach_commands.py#59

In order to avoid wreaking havoc with Travis jobs, let's add a deprecated mapping for b2gdesktop as well, and then remove that after the Travis jobs have been updated.
Attachment #8348779 - Flags: review?(jgriffin) → review+
Carrying r+ and running on try before landing: https://tbpl.mozilla.org/?tree=Try&rev=6d5e7819b44a
Attachment #8348779 - Attachment is obsolete: true
Attachment #8349305 - Flags: review+
Thanks for keeping the old setting alive for a bit longer; syncing Gecko and Gaia = nightmare!
https://hg.mozilla.org/mozilla-central/rev/526ae6094c53
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.