812521 - [B2G] 'kill all' fails if no apps are running

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

The killAll method was introduced in bug 800017, however it fails when no applications are currently running.

I fixed this locally by adding the following to the end of the function:

if (originsToClose.length === 0) {
  marionetteScriptFinished(true);
}

Comment 1

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I've submitted a pull request [1] for mozilla/gaia-ui-tests that demonstrates this issue.

[1] https://github.com/mozilla/gaia-ui-tests/pull/2

Comment 2

[Jonathan Griffin (:jgriffin)]

(In reply to Dave Hunt (:davehunt) from comment #0)
> The killAll method was introduced in bug 800017, however it fails when no
> applications are currently running.
> 
> I fixed this locally by adding the following to the end of the function:
> 
> if (originsToClose.length === 0) {
>   marionetteScriptFinished(true);
> }

Can you add just this bit as a pull request?

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

If this is the right approach then I certainly can. I thought I had a couple of issues with it locally but will test again. Do you think this should be submitted to mozilla-b2g/gaia or mozilla/gaia-ui-tests? I suspect the former is the correct approach but the latter would unblock the UI tests sooner.

Comment 4

[Jonathan Griffin (:jgriffin)]

Everything should be submitted to mozilla/gaia-ui-tests; I'll take care of merging changes upstream on occasion.

Comment 5

[Jonathan Griffin (:jgriffin)]

(In reply to Dave Hunt (:davehunt) from comment #3)
> If this is the right approach then I certainly can. I thought I had a couple
> of issues with it locally but will test again. Do you think this should be
> submitted to mozilla-b2g/gaia or mozilla/gaia-ui-tests? I suspect the former
> is the correct approach but the latter would unblock the UI tests sooner.

Yes, I discovered this isn't the right fix.  I'll post a PR with a better fix tomorrow.

Comment 6

[Jonathan Griffin (:jgriffin)]

Attachment: Pointer to pull request

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 683651 [details]
Pointer to pull request

-          function() { marionetteScriptFinished(true); },
+          function() { marionetteScriptFinished("foo"); },

Why "foo"?

Could you run the tests from https://github.com/mozilla/gaia-ui-tests/pull/2 with this fix? Once my emulator build has finished I'll try running them, but you might be quicker. Also, seeing as we're changing this here, does it make sense to merge in the additional tests into this pull? I previously closed it as there was a question over adding the tests here or via mozilla-b2g/gaia.

Comment 8

[Jonathan Griffin (:jgriffin)]

Comment on attachment 683651 [details]
Pointer to pull request

PR updated.  I've added the test from your PR and verified it passes.  Also fixed the silly "foo" mistake, which was something I left in for debugging.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 683651 [details]
Pointer to pull request

Looks good and passes! Landed in:
https://github.com/mozilla/gaia-ui-tests/commit/c6d9a746ef79fbced59a6e5ab617bb608e5177a3