[B2G] Intermittent test_mobile_voice_state.js | Emulator callback still pending when finish() called

RESOLVED FIXED in mozilla17

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jgriffin, Assigned: vicamo)

Tracking

({intermittent-failure})

Trunk
mozilla17
All
Gonk (Firefox OS)
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
On the B2G CI, test_mobile_voice_state.js sometimes fails with the error:

test_mobile_voice_state.js
TEST-UNEXPECTED-FAIL | Traceback (most recent call last):
File "/data/jenkins/workspace/webapi-marionette-test/testing/marionette/client/marionette/marionette_test.py", line 176, in runTest
results = self.marionette.execute_js_script(js, args, special_powers=True)
File "/data/jenkins/workspace/webapi-marionette-test/testing/marionette/client/marionette/marionette.py", line 369, in execute_js_script
specialPowers=special_powers)
File "/data/jenkins/workspace/webapi-marionette-test/testing/marionette/client/marionette/marionette.py", line 171, in _send_message
self._handle_error(response)
File "/data/jenkins/workspace/webapi-marionette-test/testing/marionette/client/marionette/marionette.py", line 228, in _handle_error
raise MarionetteException(message=message, status=status, stacktrace=stacktrace)
MarionetteException: Emulator callback still pending when finish() called

Seems like a possible timing issue.
(Assignee)

Updated

5 years ago
Assignee: nobody → vyang
Status: NEW → ASSIGNED
(Assignee)

Comment 1

5 years ago
Created attachment 651984 [details] [diff] [review]
V1
Attachment #651984 - Flags: review?(marshall)
(Assignee)

Updated

5 years ago
Blocks: 780558
Comment on attachment 651984 [details] [diff] [review]
V1

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

r+ since this will probably work -- but it feels a bit like a hack. See comments below.

::: dom/network/tests/marionette/test_mobile_voice_state.js
@@ +110,5 @@
>  function cleanUp() {
>    SpecialPowers.clearUserPref(WHITELIST_PREF);
> +
> +  // For 'Emulator callback still pending when finish() called'
> +  setTimeout(finish, 1000);

It would be better if we provided runEmulatorCmd a callback, and wait for that callback before moving on to the next test function (or in this case, cleanUp()). I currently do this in my voicemail tests here:

https://mxr.mozilla.org/mozilla-central/source/dom/telephony/test/marionette/test_voicemail_statuschanged.js#12
Attachment #651984 - Flags: review?(marshall) → review+
Comment on attachment 651984 [details] [diff] [review]
V1

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

::: dom/network/tests/marionette/test_mobile_voice_state.js
@@ +110,5 @@
>  function cleanUp() {
>    SpecialPowers.clearUserPref(WHITELIST_PREF);
> +
> +  // For 'Emulator callback still pending when finish() called'
> +  setTimeout(finish, 1000);

I agree with Marshall. I suggest something like this:

  let emulatorCmdPending = false;
  function setEmulatorVoiceState(state) {
    emulatorCmdPending = true;
    runEmulatorCmd("gsm voice " + state, function (result) {
      emulatorCmdPending = false;
      is(result[0], "OK");
    });
  }

and then do a tighter wait loop, e.g.:

  function cleanUp() {
    if (emulatorCmdPending) {
      setTimeout(cleanUp, 100);
      return;
    }
    SpecialPowers.clearUserPref(WHITELIST_PREF);
    finish();
  }
Attachment #651984 - Flags: review-
(Assignee)

Comment 4

5 years ago
Created attachment 652652 [details] [diff] [review]
V2

* rebase to include changes made in bug 780558
* address review comment #2 and #3. Thank you both ;)
Attachment #652652 - Flags: review?(philipp)
Attachment #652652 - Flags: review?(marshall)
(Assignee)

Updated

5 years ago
Attachment #651984 - Attachment is obsolete: true
Comment on attachment 652652 [details] [diff] [review]
V2

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

Looks good
Attachment #652652 - Flags: review?(marshall) → review+
Attachment #652652 - Flags: review?(philipp) → review+
(Assignee)

Comment 6

5 years ago
Created attachment 653245 [details] [diff] [review]
V3

rebase only
Attachment #652652 - Attachment is obsolete: true
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/785cd1cb9446

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/785cd1cb9446
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Keywords: intermittent-failure
Whiteboard: [orange]
Comment hidden (Treeherder Robot)
You need to log in before you can comment on or make changes to this bug.