We should restart b2g before each app tests This is what b2gperf does. See bug 1020557 comment 7.

We restart b2g between each run Note: this is the quick solution, I have one involving the device host but there is no way I can get it checked in before this evening.

Comment on attachment 8443458 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20806 Gareth do you mind reviewing this? I will have a follow up when I come back to do thing properly in the runner instead.

Quick note: There should be a second part to that bug that involve: 1. the restart code should be in the device host module and not the shell script 2. we should have an option to restart / not restart. We the current patch it is harder bug 1027649 is much harder to reproduce. I propose that restart be optout. NO_B2G_RESTART=1 as env should be enough.

The marionette-device-host changes are on github. See the diff https://github.com/hfiguiere/marionette-device-host/compare/bug1027232 I need to test it further, release a new npm module, update gaia-node-modules and set the new revision in gaia. Lot of moving pieces, lots of review. Hence the [leave-open] whiteboard tag - fixing automation is a priority.

Eli, if you can watch this. If it has a r+ make sure it is checked in. And then that bug 1020557 is no longer happening. Don't close the bug though I have a definite followup for when I come back. Thanks.

Comment on attachment 8443458 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/20806 Comments on GH. Main concern is the sleeping to wait for the device b2g to come up. Also I would like for all of this code to move to marionette-device-host. Then after that I just have some js nits. Thanks hub and eli :)

Comment on attachment 8449537 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21295 I want the adb shell stuff moved into the host so that we can really know for everything to be ready when we invoke the MarionetteDeviceHost#start callback. Flag for review again when we have that bit. Thanks!

Comment on attachment 8449537 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21295 ADB code has been moved into marionette-device-host along with a tracking flag to ensure it only restarts B2G on the first run.

Just a note that this patch should take care of point 1 from comment 3, but point 2 remains.

The original goal what to get this quickly to resume automation - a 4 step landing would have been impossible before I left. #fail. Comment 3 and comment 4 said that. But at that point I'll just cancel the review and do it properly when I come back. I'm supposedly on vacation.

taking it back from PTO. Also tempted to actually merge bug 1033402 into this as it is needed anyway for that sole reason.

Comment on attachment 8451646 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21450 We should get that part in Gaia TestPerf first. I think Gareth comments have been addressed. This wait for lockscreen to be properly loaded before attempting anything. This actually improve the robustness of the tests. Thanks. The device-host part will be coming up next.

Comment on attachment 8451646 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21450 Other than a couple nits, this LGTM. Thanks!

Gareth: pondering is waitForSocket() shouldn't be in marionette-js-client. Otherwise this does work.

And a further comment this waitForSocket() logic is lifted from the Python b2gperf, ie it is proven to be working in the long run.

Merged the Gaia PerfTest part: https://github.com/mozilla-b2g/gaia/commit/260a371cd07895f051bf96365342072902e7df1f

Comment on attachment 8453178 [details] Link to Github pull-request: https://github.com/mozilla-b2g/marionette-device-host/pull/2 I think it is safe to keep this localised to the device-host for now. Switching to r?

Comment on attachment 8453178 [details] Link to Github pull-request: https://github.com/mozilla-b2g/marionette-device-host/pull/2 Cancelling the review request. I will split the PRs moving the resolution for bug 1033402 to the js-client. Will update the pull request shortly and request review, but the blocker will need to be resolved first.

Comment on attachment 8453178 [details] Link to Github pull-request: https://github.com/mozilla-b2g/marionette-device-host/pull/2 This requires bug 1033402 to be landed. This is in the device-host module. It will require: a new module release. the usual update of the gaia-node-module

Still need to figure out how we can tell that b2g is up without sleeping. If waiting for the tcp socket didn't work then perhaps waiting for the tcp socket and then trying to do things with it like executing a script in content and making sure that works suffices.

(Or looking at the bootwatcher thing from marionette-apps)

That setTimeout in the patch was no longer meant to be here. Really.

(In reply to Gareth Aye [:gaye] from comment #22) > Still need to figure out how we can tell that b2g is up without sleeping. If > waiting for the tcp socket didn't work then perhaps waiting for the tcp > socket and then trying to do things with it like executing a script in > content and making sure that works suffices. Do you mean waiting for B2G or waiting for Gaia? Waiting for gecko/marionette server to be ready via the tcp socket is pretty well tested, I don't think you'll have problems with that (except when b2g has crashed). Waiting for Gaia, there are various options there but you'll need to handle at least 3 ready scenarios: lockscreen, ftu and no lockscreen or ftu, for various states of profiles.

bug 1033402 is covering this. the homescreen it is already done (see first patch).

Also after stopping you can poll `ps` for the b2g process and check that it is no longer running before starting it again. (We do this on Python framework after the a bug caused b2g process to be un-stoppable).

FYI, the pull request has been updated. The timeout is "gone". Actually set to 0 for an async call to the callback. (In reply to Zac C (:zac) from comment #27) > Also after stopping you can poll `ps` for the b2g process and check that it > is no longer running before starting it again. (We do this on Python > framework after the a bug caused b2g process to be un-stoppable). I haven't had that issue though. I'll keep that in mind, the experience is valuable.

Gareth, this is a critical blocking issue. Please prioritize doing this review. We need this resolved asap.

Hey Mike! I've been working incrementally with :hub to get this work in and I think we're making progress. I've always gotten feedback in within a day of his updates. I'm happy to keep providing feedback, but no amount of managerial compulsion is going to make me settle for a patches that aren't ready for prime time. This has been a longer than usual process because :hub is still in the process of learning how all of the bits are wired together. If you're hoping for a less strict reviewer, James Lal is also a marionette js peer.

Oh! I just realized there was another pull request to marionette-device-host. That one looks okay. In general, I prefer one patch per bug since every time I've looked at this bug I've seen that I've already OK'd it. Sorry for the delay!

Comment on attachment 8453178 [details] Link to Github pull-request: https://github.com/mozilla-b2g/marionette-device-host/pull/2 A couple of nits on GH but mostly looks good. Fix and merge when ready!

(In reply to Gareth Aye [:gaye] from comment #31) > Oh! I just realized there was another pull request to > marionette-device-host. That one looks okay. In general, I prefer one patch > per bug since every time I've looked at this bug I've seen that I've already > OK'd it. Sorry for the delay! Yes there was two commit in that other PR because I had no choice due to the dependency. Sorry about that. I thought I mentioned it. Thanks !

Merge https://github.com/mozilla-b2g/marionette-device-host/commit/3c116cd509221f7e940e29b9f6932c9d6cb4dbb4 (technically it is fixed, but I'll need to do a new module release for that for it to be available. Will happen with the blocking bug)

Actually keeping open until the released module.

version 0.1.0 uploaded to npm. The modules will be pulled into gaia-node-modules when we fix bug 1033402

All in See https://bugzilla.mozilla.org/show_bug.cgi?id=1033402#c32

Comment on attachment 8451646 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21450 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] [Bug caused by] (feature/regressing bug #): itself. we forgot to uplift [User impact] if declined: perf test less reliable [Testing completed]: [Risk to taking this patch] (and alternatives if risky): npotb. [String changes made]: none

and by "perf test less reliable" I really mean "broken" testing all done in master. backporting the rest of the feature require this.

Agreed, perf tests cannot be run against v2.0 without this patch.

Merged v2.0 https://github.com/mozilla-b2g/gaia/commit/8cc28fd31905a0ea2b2e15d13e80a0eab2feb1ba