Last Comment Bug 787203 - Get marionette working on Fennec
: Get marionette working on Fennec
Status: RESOLVED FIXED
[marionette=1.0]
:
Product: Testing
Classification: Components
Component: Marionette (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: mozilla50
Assigned To: Maja Frydrychowicz (:maja_zf)
:
:
Mentors:
: 642520 1180895 (view as bug list)
Depends on: 787280 787545 791793 1278590
Blocks: 1194860 1284960 793661 1284874
  Show dependency treegraph
 
Reported: 2012-08-30 13:41 PDT by David Burns :automatedtester
Modified: 2016-08-29 07:24 PDT (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
MozReview Request: Bug 787203: Get Marionette working in Fennec (58 bytes, text/x-review-board-request)
2016-03-10 01:01 PST, David Burns :automatedtester
no flags Details | Review
MozReview Request: Bug 787203: Get Marionette working in Fennec. (58 bytes, text/x-review-board-request)
2016-03-13 17:20 PDT, Nick Alexander :nalexander
no flags Details | Review
marionette.log (190.09 KB, text/x-log)
2016-03-13 17:22 PDT, Nick Alexander :nalexander
no flags Details
Bug 787203 - Get Marionette working in Fennec; (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
dburns: review+
Details | Review
Bug 787203 - Enable Marionette on debug Fennec build (android-api-15-debug); (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
gps: review+
Details | Review
Bug 787203 - [mozrunner] Update `disconnected` when cleaning up after emulator shutdown; (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
ahalberstadt: review+
Details | Review
Bug 787203 - [mozrunner] Expose a start_logcat method in Device; (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
ahalberstadt: review+
Details | Review
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner; (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
ahalberstadt: review+
Details | Review
Bug 787203 - [mozdevice] Add remove_forward method to DeviceManagerADB; (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
ahalberstadt: review+
Details | Review
Bug 787203 - Add emulator and Fennec support to Marionette harness and client; (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
dburns: review+
Details | Review
Bug 787203 - Allow mach marionette-test to run with android build; (58 bytes, text/x-review-board-request)
2016-06-29 12:31 PDT, Maja Frydrychowicz (:maja_zf)
dburns: review+
Details | Review

Description David Burns :automatedtester 2012-08-30 13:41:26 PDT
WebApps need to have marionette working in Fennec so this is a tracking bug related to that
Comment 1 David Burns :automatedtester 2015-06-09 13:14:51 PDT
Moving this to a Q3 goal. 

* Finish off Marionette support on Fennec
* Stand up tests on buildbot/TC (whatever we decide)
Comment 2 David Burns :automatedtester 2015-07-06 13:51:30 PDT
*** Bug 1180895 has been marked as a duplicate of this bug. ***
Comment 3 Dave Hunt (:davehunt) 2015-12-22 09:13:34 PST
*** Bug 642520 has been marked as a duplicate of this bug. ***
Comment 4 Nick Alexander :nalexander 2016-03-09 20:24:12 PST
dburns: it seems like you had some WIP on this.  Did you post that somewhere?  What's the status?
Comment 5 David Burns :automatedtester 2016-03-10 01:01:09 PST
Created attachment 8728855 [details]
MozReview Request: Bug 787203: Get Marionette working in Fennec

Review commit: https://reviewboard.mozilla.org/r/39167/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39167/
Comment 6 David Burns :automatedtester 2016-03-10 01:03:28 PST
Uploaded my WIP. We have a session but because I just hacked the tab code to "work" you can't do anything more than get a session.
Comment 7 Nick Alexander :nalexander 2016-03-13 17:20:44 PDT
Created attachment 8730043 [details]
MozReview Request: Bug 787203: Get Marionette working in Fennec.

Review commit: https://reviewboard.mozilla.org/r/39643/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39643/
Comment 8 Nick Alexander :nalexander 2016-03-13 17:21:44 PDT
(In reply to Nick Alexander :nalexander from comment #7)
> Created attachment 8730043 [details]
> MozReview Request: Bug 787203: Get Marionette working in Fennec.
> 
> Review commit: https://reviewboard.mozilla.org/r/39643/diff/#index_header
> See other reviews: https://reviewboard.mozilla.org/r/39643/

With this, I get good results with:

nalexander@chocho ~/M/g/t/m/h/marionette> python runtests.py --address=localhost:2828 --type=b2g tests/unit/unit-tests.ini > out
nalexander@chocho ~/M/g/t/m/h/marionette> pwd
/Users/nalexander/Mozilla/gecko/testing/marionette/harness/marionette

(Log attached.)
Comment 9 Nick Alexander :nalexander 2016-03-13 17:22:14 PDT
Created attachment 8730044 [details]
marionette.log
Comment 10 David Burns :automatedtester 2016-03-14 03:58:50 PDT
https://reviewboard.mozilla.org/r/39643/#review36315

Great work!

::: testing/marionette/driver.js:3058
(Diff revision 1)
>   * identified within BrowserObject by a tab.
>   */
>  Object.defineProperty(BrowserObj.prototype, "curFrameId", {
>    get() {
>      let rv = null;
> -    if (this.driver.appName != "Firefox") {
> +    if (this.driver.appName != "Firefox" && this.driver.appName != "Fennec") {

this could just be `if (this.driver.appName == "B2G")` instead. This is not going to work in iOS so no need to be explicit about the "nots"
Comment 11 Nick Alexander :nalexander 2016-03-17 12:32:42 PDT
dburns: I'm not really plugged into the Marionette / A-Team world.  What else needs to happen to land (some version) of this patch?  Some automated testing would be nice, but shouldn't block getting these small fixes out the door.

My interest: I'm pursuing some additional Android-specific testing work that I might want to use Marionette for.  I want to investigate an espresso-web style testing API.  espresso-web is an ad-hoc wrapper around some WebDriver "Atoms", as far as I can tell; see https://google.github.io/android-testing-support-library/docs/espresso/web/.  I'm investigating an espresso-style API wrapping WebDriver/WebElement.
Comment 12 David Burns :automatedtester 2016-03-21 03:24:21 PDT
(In reply to Nick Alexander :nalexander from comment #11)
> dburns: I'm not really plugged into the Marionette / A-Team world.  What
> else needs to happen to land (some version) of this patch?  Some automated
> testing would be nice, but shouldn't block getting these small fixes out the
> door.

If we have this working locally then we should get them up for review ASAP and landed.

> 
> My interest: I'm pursuing some additional Android-specific testing work that
> I might want to use Marionette for.  I want to investigate an espresso-web
> style testing API.  espresso-web is an ad-hoc wrapper around some WebDriver
> "Atoms", as far as I can tell; see
> https://google.github.io/android-testing-support-library/docs/espresso/web/.
> I'm investigating an espresso-style API wrapping WebDriver/WebElement.

Speaking to my Google contact that led the WebDriver team there he said that the espresso team had little interest in supporting web testing. As mentioned on IRC, I would prefer we used something like Selendroid/Appium as they would be much easier to support (and also don't use Fluent APIs!) but I have no skin in the fennec game so won't argue much on this as you know the workflow the mobile team want.
Comment 13 Nick Alexander :nalexander 2016-03-21 13:28:57 PDT
(In reply to David Burns :automatedtester from comment #12)
> (In reply to Nick Alexander :nalexander from comment #11)
> > dburns: I'm not really plugged into the Marionette / A-Team world.  What
> > else needs to happen to land (some version) of this patch?  Some automated
> > testing would be nice, but shouldn't block getting these small fixes out the
> > door.
> 
> If we have this working locally then we should get them up for review ASAP
> and landed.
> 
> > 
> > My interest: I'm pursuing some additional Android-specific testing work that
> > I might want to use Marionette for.  I want to investigate an espresso-web
> > style testing API.  espresso-web is an ad-hoc wrapper around some WebDriver
> > "Atoms", as far as I can tell; see
> > https://google.github.io/android-testing-support-library/docs/espresso/web/.
> > I'm investigating an espresso-style API wrapping WebDriver/WebElement.
> 
> Speaking to my Google contact that led the WebDriver team there he said that
> the espresso team had little interest in supporting web testing.

Interesting, thanks.  That explains why espresso-web is just a stub.

 As
> mentioned on IRC, I would prefer we used something like Selendroid/Appium as
> they would be much easier to support (and also don't use Fluent APIs!) but I
> have no skin in the fennec game so won't argue much on this as you know the
> workflow the mobile team want.

I would be pleased to use either Selendroid or Appium, but both appear very tied to Android WebViews.  I didn't spend too much time understanding the details of Selendroid and would be happy to dig into it further.

I would like to understand how you feel about Fluent APIs.  I am interested in integrating with Espresso for timing and robustness purposes.  I evaluated espresso-web as an API for driving Marionette with WebDriver and it's not a great fit: I ended up saying to myself, "why bother?"  Any more context on the Espresso-style vs. imperative style API would be useful.  Thanks!
Comment 14 David Burns :automatedtester 2016-04-04 15:44:59 PDT
(In reply to Nick Alexander :nalexander from comment #13)
> (In reply to David Burns :automatedtester from comment #12)
> > (In reply to Nick Alexander :nalexander from comment #11)
> > > dburns: I'm not really plugged into the Marionette / A-Team world.  What
> > > else needs to happen to land (some version) of this patch?  Some automated
> > > testing would be nice, but shouldn't block getting these small fixes out the
> > > door.
> > 
> > If we have this working locally then we should get them up for review ASAP
> > and landed.
> > 
> > > 
> > > My interest: I'm pursuing some additional Android-specific testing work that
> > > I might want to use Marionette for.  I want to investigate an espresso-web
> > > style testing API.  espresso-web is an ad-hoc wrapper around some WebDriver
> > > "Atoms", as far as I can tell; see
> > > https://google.github.io/android-testing-support-library/docs/espresso/web/.
> > > I'm investigating an espresso-style API wrapping WebDriver/WebElement.
> > 
> > Speaking to my Google contact that led the WebDriver team there he said that
> > the espresso team had little interest in supporting web testing.
> 
> Interesting, thanks.  That explains why espresso-web is just a stub.
> 
>  As
> > mentioned on IRC, I would prefer we used something like Selendroid/Appium as
> > they would be much easier to support (and also don't use Fluent APIs!) but I
> > have no skin in the fennec game so won't argue much on this as you know the
> > workflow the mobile team want.
> 
> I would be pleased to use either Selendroid or Appium, but both appear very
> tied to Android WebViews.  I didn't spend too much time understanding the
> details of Selendroid and would be happy to dig into it further.

Both of these are not tied to webviews, they work against anything that Espresso can work against, they use the instrumentation APIs built into Android.

> 
> I would like to understand how you feel about Fluent APIs. 

Having spent far too many hours helping people debug fluent APIs I have learnt to hate them. Having to step through numerous methods to get what you want has just left me with a bad taste in my mouth.
Comment 15 Nick Alexander :nalexander 2016-04-08 15:25:56 PDT
Sadly I won't be able to push this across the line.  It's close, though, and worth polishing up.  Perhaps gbrown can do the honours?
Comment 16 Geoff Brown [:gbrown] 2016-04-11 08:16:09 PDT
Sorry, I don't think I can fit in another project just now. I'm happy to try to answer specific questions, and will try to circle back later in the quarter.
Comment 17 Maja Frydrychowicz (:maja_zf) 2016-04-22 12:14:37 PDT
I'm going to dig into this. Expect questions from me in the near future!
Comment 18 Maja Frydrychowicz (:maja_zf) 2016-04-26 08:14:40 PDT
Hi Nick, I'm trying to run your patch according to Comment 8 with an Android emulator. No luck yet. Maybe I'm missing some prerequisite step? Did you test with an emulator or a physical device?

> ./mach android-emulator
> adb forward tcp:2828 tcp:2828
> ./mach run # I've confirmed that prefs for marionette and port 2828 are set appropriately in browser intance
> cd testing/marionette/harness/marionette
> python runtests.py --address=localhost:2828 --type=b2g tests/unit/unit-tests.ini

At this point, socket happily connects at port, but then first packet received is empty, which is logged as "IOError: Connection to Marionette server is lost."
Comment 19 Nick Alexander :nalexander 2016-04-26 08:45:53 PDT
(In reply to Maja Frydrychowicz (:maja_zf) from comment #18)
> Hi Nick, I'm trying to run your patch according to Comment 8 with an Android
> emulator. No luck yet. Maybe I'm missing some prerequisite step? Did you
> test with an emulator or a physical device?
> 
> > ./mach android-emulator
> > adb forward tcp:2828 tcp:2828
> > ./mach run # I've confirmed that prefs for marionette and port 2828 are set appropriately in browser intance
> > cd testing/marionette/harness/marionette
> > python runtests.py --address=localhost:2828 --type=b2g tests/unit/unit-tests.ini
> 
> At this point, socket happily connects at port, but then first packet
> received is empty, which is logged as "IOError: Connection to Marionette
> server is lost."

You have installed and run Fennec on the emulator?  You built Fennec with the ENABLE_MARIONETTE flag on, and the pref is enabled?  (I think the patch includes that, but worth checking.)

Try to telnet to localhost:2828 after forwarding; if you don't get a handshake, you have other problems.

Finally, I only tried this on device.  It's possible there's an emulator interaction.
Comment 20 Geoff Brown [:gbrown] 2016-04-27 07:09:46 PDT
Is there a chance this is related to bug 1267925?
Comment 21 Andreas Tolfsen ‹:ato› 2016-04-27 07:22:22 PDT
(In reply to Geoff Brown [:gbrown] from comment #20)
> Is there a chance this is related to bug 1267925?

I think that is unlikely.
Comment 22 David Burns :automatedtester 2016-04-27 14:01:08 PDT
Maja has this running, though not always reliably.
Comment 23 Maja Frydrychowicz (:maja_zf) 2016-04-27 15:18:06 PDT
...and now I have it running reliably. For the curious, I think the flakiness came partly from not waiting long enough for Fennec to load (a long time after it "looks" loaded), and otherwise from how I sometimes happened to enable port-forwarding with adb. For example, if I start an emulator, then |./mach install| then |adb forward tcp:2828 tcp:2828| then |./mach run|, all is well. On the other hand, running |adb forward tcp:2828 tcp:2828| at any time from within |adb shell| is not equivalent (oops) -- it occupies the emulator's port 2828 and so interferes with Marionette.
Comment 24 Maja Frydrychowicz (:maja_zf) 2016-05-06 14:15:26 PDT
(In reply to David Burns :automatedtester from comment #12)
> (In reply to Nick Alexander :nalexander from comment #11)
> > dburns: I'm not really plugged into the Marionette / A-Team world.  What
> > else needs to happen to land (some version) of this patch?  Some automated
> > testing would be nice, but shouldn't block getting these small fixes out the
> > door.
> 
> If we have this working locally then we should get them up for review ASAP
> and landed.

The missing piece is managing emulator+fennec start-up in Marionette client, the way it's already done for desktop and for b2g. I'm going to address this in testing/marionette/client/marionette_driver/geckoinstance.py, with some substantial clean-up in Marionette harness and client while I'm at it.

For the automation side, we'll need to:
- Adapt the marionette.py mozharness script (and/or create an Android-specific config)
- Enable Marionette in Fennec builds -- I need to talk to someone on the Fennec team about that. :snorp, might that be you?
- Schedule the job in buildbot, or TaskCluster eventually. My understanding is that Fennec builds in TC are in the works (Bug 1267430), but I'm not aware of a timeline.
Comment 25 Maja Frydrychowicz (:maja_zf) 2016-05-18 08:56:07 PDT
Margaret -- are you okay with us enabling Marionette in some Fennec builds? For example, debug only? I know one security concern is to make sure Marionette isn't enabled in any Fennec releases.

Greg & Margaret -- it's my understanding that nalexander handles a lot of the Fennec build stuff, but he's not available for now. Could you point me to where Fennec builds are configured and whom to ask for review?
Comment 26 Gregory Szorc [:gps] 2016-05-18 11:02:07 PDT
You are likely interested in mobile/android/confvars.sh. You can ask any build peer for review. :ted historically handles things involving test harness foo. But any of us are appropriate. chmanchester and I have also been touching a lot of code at the intersection of testing and the build system lately.
Comment 27 :Margaret Leibovic 2016-05-26 16:23:57 PDT
(In reply to Maja Frydrychowicz (:maja_zf) from comment #25)
> Margaret -- are you okay with us enabling Marionette in some Fennec builds?
> For example, debug only? I know one security concern is to make sure
> Marionette isn't enabled in any Fennec releases.

Sorry for the slow response. Sure, I think it's fine to enable Marionette in some builds.
Comment 28 Maja Frydrychowicz (:maja_zf) 2016-06-28 21:56:27 PDT
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dea125d659b6
Comment 29 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766498 [details]
Bug 787203 - Get Marionette working in Fennec;

Review commit: https://reviewboard.mozilla.org/r/61360/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61360/
Comment 30 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766499 [details]
Bug 787203 - Enable Marionette on debug Fennec build (android-api-15-debug);

Review commit: https://reviewboard.mozilla.org/r/61362/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61362/
Comment 31 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766500 [details]
Bug 787203 - [mozrunner] Update `disconnected` when cleaning up after emulator shutdown;

Allows Marionette client's FennecInstance to clean up properly

Review commit: https://reviewboard.mozilla.org/r/61364/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61364/
Comment 32 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766501 [details]
Bug 787203 - [mozrunner] Expose a start_logcat method in Device;

This is a refactor. |start_logcat| allows filtering by tag and is used for
recording gecko.log for Marionette tests on Fennec.

Review commit: https://reviewboard.mozilla.org/r/61366/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61366/
Comment 33 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766502 [details]
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner;

Review commit: https://reviewboard.mozilla.org/r/61368/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61368/
Comment 34 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766503 [details]
Bug 787203 - [mozdevice] Add remove_forward method to DeviceManagerADB;

For adb forward --remove|remove-all

Review commit: https://reviewboard.mozilla.org/r/61370/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61370/
Comment 35 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766504 [details]
Bug 787203 - Add emulator and Fennec support to Marionette harness and client;

Add FennecInstance. Refactor GeckoInstance as well as other related clean-up.

Marionette runner can now launch both Android emulator and Fennec, or connect to a running
emulator and then launch Fennec. (The runner can also connect to an already running Fennec
instance provided that Marionette and adb port forwarding have previously been enabled.)

Review commit: https://reviewboard.mozilla.org/r/61372/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61372/
Comment 36 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:31:52 PDT
Created attachment 8766505 [details]
Bug 787203 - Allow mach marionette-test to run with android build;

Review commit: https://reviewboard.mozilla.org/r/61374/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61374/
Comment 37 Maja Frydrychowicz (:maja_zf) 2016-06-29 12:39:29 PDT
Next: schedule Marionette tests for Android in TaskCluster.
Comment 38 David Burns :automatedtester 2016-06-29 12:54:08 PDT
Comment on attachment 8766498 [details]
Bug 787203 - Get Marionette working in Fennec;

https://reviewboard.mozilla.org/r/61360/#review58228

::: testing/marionette/driver.js:1284
(Diff revision 1)
>   * @param {string} name
>   *     Target name or ID of the window to switch to.
>   */
>  GeckoDriver.prototype.switchToWindow = function*(cmd, resp) {
>    let switchTo = cmd.parameters.name;
> -  let isB2G = this.appName == "B2G";
> +  let isMobile = this.appName == "B2G" || this.appName == "Fennec";

we don't need the b2g part

::: testing/marionette/driver.js:1291
(Diff revision 1)
>  
>    let getOuterWindowId = function(win) {
>      let rv = win.QueryInterface(Ci.nsIInterfaceRequestor)
>          .getInterface(Ci.nsIDOMWindowUtils)
>          .outerWindowID;
> -    rv += isB2G ? "-b2g" : "";
> +    rv += isMobile ? "-b2g" : "";

if we remove the b2g line I suspect we don't need this anymore

::: testing/marionette/harness/marionette/runner/base.py:767
(Diff revision 1)
>          need_external_ip = True
>          if not self.marionette:
>              self.marionette = self.driverclass(**self._build_kwargs())
>              # if we're working against a desktop version, we usually don't need
>              # an external ip
> -            if self.capabilities['device'] == "desktop":
> +            if self.capabilities['device'] == "desktop" and self.capabilities['browserName'] != 'Fennec':

device has been removed so you will need to rebase

::: testing/marionette/harness/marionette/runner/base.py:820
(Diff revision 1)
>  
>          self._add_tests(tests)
>  
>          self.logger.info("running with e10s: {}".format(self.e10s))
> -        version_info = mozversion.get_version(binary=self.bin,
> -                                              sources=self.sources,
> +		version_info = None
> +        #version_info = mozversion.get_version(binary=self.bin,

should we remove this?
Comment 39 David Burns :automatedtester 2016-06-29 13:27:43 PDT
Comment on attachment 8766504 [details]
Bug 787203 - Add emulator and Fennec support to Marionette harness and client;

https://reviewboard.mozilla.org/r/61372/#review58234
Comment 40 David Burns :automatedtester 2016-06-29 13:31:44 PDT
Comment on attachment 8766505 [details]
Bug 787203 - Allow mach marionette-test to run with android build;

https://reviewboard.mozilla.org/r/61374/#review58236
Comment 41 Gregory Szorc [:gps] 2016-06-29 16:51:02 PDT
Comment on attachment 8766499 [details]
Bug 787203 - Enable Marionette on debug Fennec build (android-api-15-debug);

https://reviewboard.mozilla.org/r/61362/#review58280

I'd have to look at the code history, but ENABLE_MARIONETTE=1 in the in-tree mozconfigs feels like a cargo cult that originated when someone didn't add an --enable-marionette flag to configure like they should have.

This will eventually need to get rewritten to play nice in a moz.configure world. But that is scope bloat for this commit series and I don't want to fault you for falling into a trap someone else created. This is fine to land.
Comment 42 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766498 [details]
Bug 787203 - Get Marionette working in Fennec;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61360/diff/1-2/
Comment 43 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766499 [details]
Bug 787203 - Enable Marionette on debug Fennec build (android-api-15-debug);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61362/diff/1-2/
Comment 44 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766500 [details]
Bug 787203 - [mozrunner] Update `disconnected` when cleaning up after emulator shutdown;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61364/diff/1-2/
Comment 45 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766501 [details]
Bug 787203 - [mozrunner] Expose a start_logcat method in Device;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61366/diff/1-2/
Comment 46 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766502 [details]
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61368/diff/1-2/
Comment 47 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766503 [details]
Bug 787203 - [mozdevice] Add remove_forward method to DeviceManagerADB;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61370/diff/1-2/
Comment 48 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766504 [details]
Bug 787203 - Add emulator and Fennec support to Marionette harness and client;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61372/diff/1-2/
Comment 49 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:27:17 PDT
Comment on attachment 8766505 [details]
Bug 787203 - Allow mach marionette-test to run with android build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61374/diff/1-2/
Comment 50 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:29:45 PDT
https://reviewboard.mozilla.org/r/61360/#review58228

> device has been removed so you will need to rebase

Done. I also removed a couple of other left-over device bits.

> should we remove this?

This was removed later in the patch series, so I just included the change in this patch instead.
Comment 51 Maja Frydrychowicz (:maja_zf) 2016-06-30 10:32:30 PDT
https://reviewboard.mozilla.org/r/61372/#review58434

::: testing/marionette/harness/marionette/tests/harness_unit/test_marionette_runner.py:63
(Diff revision 2)
>  @pytest.fixture()
>  def mach_parsed_kwargs(logger):

Since the r+ carries over, here's a highlight of something new :I updated a harness-test fixture to reflect the changes in BaseMarionetteArguments.
Comment 52 Andrew Halberstadt [:ahal] 2016-06-30 13:33:31 PDT
Comment on attachment 8766500 [details]
Bug 787203 - [mozrunner] Update `disconnected` when cleaning up after emulator shutdown;

https://reviewboard.mozilla.org/r/61364/#review58462
Comment 53 Andrew Halberstadt [:ahal] 2016-06-30 13:36:41 PDT
Comment on attachment 8766501 [details]
Bug 787203 - [mozrunner] Expose a start_logcat method in Device;

https://reviewboard.mozilla.org/r/61366/#review58464

::: testing/mozbase/mozrunner/mozrunner/devices/base.py:144
(Diff revision 2)
>              logcat_args = [self.app_ctx.adb, '-s', '%s' % serial,
>                             'logcat', '-v', 'time', '-b', 'main', '-b', 'radio']

This is a dead variable now, please remove
Comment 54 Andrew Halberstadt [:ahal] 2016-06-30 13:46:04 PDT
Comment on attachment 8766502 [details]
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner;

https://reviewboard.mozilla.org/r/61368/#review58468

::: testing/mozbase/mozrunner/mozrunner/base/device.py:177
(Diff revision 2)
>              cmd.extend(['-s', self.app_ctx.dm._deviceSerial])
>          cmd.append('shell')
>          app = "%s/org.mozilla.gecko.BrowserApp" % self.app_ctx.remote_process
>          cmd.extend(['am', 'start', '-a', 'android.activity.MAIN', '-n', app])
>          params = ['-no-remote', '-profile', self.app_ctx.remote_profile]
> +        params.extend([p for p in self.cmdargs if p not in params])

This could cause problems, for example if the cmdargs are ['-profile', 'abcxyz.default'], then -profile will not get added (because it already exists), but 'abcxyz.default' will.

I think it would be better to simply extend cmdargs without doing any filtering and leave it up to consumers to get it right.
Comment 55 Andrew Halberstadt [:ahal] 2016-06-30 13:50:33 PDT
Comment on attachment 8766503 [details]
Bug 787203 - [mozdevice] Add remove_forward method to DeviceManagerADB;

https://reviewboard.mozilla.org/r/61370/#review58470

I'd prefer if this was somehow integrated into the 'forward' method.. but I can't really think of a good way of doing that and it would probably require breaking backwards compatibility. So this looks like a good enough alternative. Thanks!
Comment 56 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766498 [details]
Bug 787203 - Get Marionette working in Fennec;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61360/diff/2-3/
Comment 57 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766499 [details]
Bug 787203 - Enable Marionette on debug Fennec build (android-api-15-debug);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61362/diff/2-3/
Comment 58 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766500 [details]
Bug 787203 - [mozrunner] Update `disconnected` when cleaning up after emulator shutdown;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61364/diff/2-3/
Comment 59 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766501 [details]
Bug 787203 - [mozrunner] Expose a start_logcat method in Device;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61366/diff/2-3/
Comment 60 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766502 [details]
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61368/diff/2-3/
Comment 61 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766503 [details]
Bug 787203 - [mozdevice] Add remove_forward method to DeviceManagerADB;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61370/diff/2-3/
Comment 62 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766504 [details]
Bug 787203 - Add emulator and Fennec support to Marionette harness and client;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61372/diff/2-3/
Comment 63 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:34:43 PDT
Comment on attachment 8766505 [details]
Bug 787203 - Allow mach marionette-test to run with android build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61374/diff/2-3/
Comment 64 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:38:30 PDT
https://reviewboard.mozilla.org/r/61360/#review58228

> Done. I also removed a couple of other left-over device bits.

...and now I've really removed the left-over device bits. (I had previously messed something up with my histedit, but fortunately those super awesome Marionette harness tests alerted me to the problem!)
Comment 65 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:39:05 PDT
https://reviewboard.mozilla.org/r/61368/#review58468

> This could cause problems, for example if the cmdargs are ['-profile', 'abcxyz.default'], then -profile will not get added (because it already exists), but 'abcxyz.default' will.
> 
> I think it would be better to simply extend cmdargs without doing any filtering and leave it up to consumers to get it right.

Good point! Thanks.
Comment 66 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:43:23 PDT
Comment on attachment 8766501 [details]
Bug 787203 - [mozrunner] Expose a start_logcat method in Device;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61366/diff/3-4/
Comment 67 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:43:23 PDT
Comment on attachment 8766502 [details]
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61368/diff/3-4/
Comment 68 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:43:23 PDT
Comment on attachment 8766503 [details]
Bug 787203 - [mozdevice] Add remove_forward method to DeviceManagerADB;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61370/diff/3-4/
Comment 69 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:43:23 PDT
Comment on attachment 8766504 [details]
Bug 787203 - Add emulator and Fennec support to Marionette harness and client;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61372/diff/3-4/
Comment 70 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:43:23 PDT
Comment on attachment 8766505 [details]
Bug 787203 - Allow mach marionette-test to run with android build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61374/diff/3-4/
Comment 71 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:47:29 PDT
Comment on attachment 8766504 [details]
Bug 787203 - Add emulator and Fennec support to Marionette harness and client;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61372/diff/4-5/
Comment 72 Maja Frydrychowicz (:maja_zf) 2016-06-30 14:47:29 PDT
Comment on attachment 8766505 [details]
Bug 787203 - Allow mach marionette-test to run with android build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61374/diff/4-5/
Comment 73 David Burns :automatedtester 2016-06-30 15:51:37 PDT
Comment on attachment 8766498 [details]
Bug 787203 - Get Marionette working in Fennec;

https://reviewboard.mozilla.org/r/61360/#review58496
Comment 74 Andrew Halberstadt [:ahal] 2016-07-04 14:52:50 PDT
Comment on attachment 8766502 [details]
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner;

https://reviewboard.mozilla.org/r/61368/#review58922

Thanks!
Comment 75 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766498 [details]
Bug 787203 - Get Marionette working in Fennec;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61360/diff/3-4/
Comment 76 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766499 [details]
Bug 787203 - Enable Marionette on debug Fennec build (android-api-15-debug);

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61362/diff/3-4/
Comment 77 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766500 [details]
Bug 787203 - [mozrunner] Update `disconnected` when cleaning up after emulator shutdown;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61364/diff/3-4/
Comment 78 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766501 [details]
Bug 787203 - [mozrunner] Expose a start_logcat method in Device;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61366/diff/4-5/
Comment 79 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766502 [details]
Bug 787203 - [mozrunner] Pass application arguments to FennecRunner;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61368/diff/4-5/
Comment 80 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766503 [details]
Bug 787203 - [mozdevice] Add remove_forward method to DeviceManagerADB;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61370/diff/4-5/
Comment 81 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766504 [details]
Bug 787203 - Add emulator and Fennec support to Marionette harness and client;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61372/diff/5-6/
Comment 82 Maja Frydrychowicz (:maja_zf) 2016-07-05 11:12:36 PDT
Comment on attachment 8766505 [details]
Bug 787203 - Allow mach marionette-test to run with android build;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61374/diff/5-6/
Comment 83 Pulsebot 2016-07-05 11:16:57 PDT
Pushed by mjzffr@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0c4217a73078
Get Marionette working in Fennec; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/88891cd76c27
Enable Marionette on debug Fennec build (android-api-15-debug); r=gps
https://hg.mozilla.org/integration/autoland/rev/4d13156992bc
[mozrunner] Update `disconnected` when cleaning up after emulator shutdown; r=ahal
https://hg.mozilla.org/integration/autoland/rev/44be5559589e
[mozrunner] Expose a start_logcat method in Device; r=ahal
https://hg.mozilla.org/integration/autoland/rev/d7c92e7beefa
[mozrunner] Pass application arguments to FennecRunner; r=ahal
https://hg.mozilla.org/integration/autoland/rev/24964c4700e9
[mozdevice] Add remove_forward method to DeviceManagerADB; r=ahal
https://hg.mozilla.org/integration/autoland/rev/b6c77cd3400e
Add emulator and Fennec support to Marionette harness and client; r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/1c7cb653a804
Allow mach marionette-test to run with android build; r=automatedtester
Comment 85 William Lachance (:wlach) 2016-07-27 10:32:10 PDT
This caused a small installer size increase (on debug builds only): https://treeherder.mozilla.org/perf.html#/alerts?id=1817

We should be careful about enabling this on opt builds, since .apk size is a concern on Android.

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