815441 - Errors should pre-empt emulator callback warning

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Jonathan Griffin (:jgriffin)]

Currently, if an async call fails due to some JS error that occurs during an emulator callback, the test will fail with "emulator callback still pending".  This obscures the real source of the error, which is the JS error.

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: Make errors pre-empt emulator callback warning,

Comment 2

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 685401 [details] [diff] [review]
Make errors pre-empt emulator callback warning,

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

::: testing/marionette/marionette-listener.js
@@ +304,5 @@
> +      value = "Emulator callback still pending when finish() called";
> +      status = 500;
> +    }
> +    _emu_cbs = {};
> +

How about

if (status == 0) {
   if (Object ....) {
   }
   else {
   }
}

instead of checking the status again on line 309, so it's less repetitive?

Comment 3

[Jonathan Griffin (:jgriffin)]

(In reply to Malini Das [:mdas] from comment #2)
> Comment on attachment 685401 [details] [diff] [review]
> Make errors pre-empt emulator callback warning,
> 
> Review of attachment 685401 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/marionette/marionette-listener.js
> @@ +304,5 @@
> > +      value = "Emulator callback still pending when finish() called";
> > +      status = 500;
> > +    }
> > +    _emu_cbs = {};
> > +
> 
> How about
> 
> if (status == 0) {
>    if (Object ....) {
>    }
>    else {
>    }
> }
> 
> instead of checking the status again on line 309, so it's less repetitive?

If we do that, then I'd have to duplicate the sendError line at http://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js#329 in the case where emulator callbacks are still pending.

Comment 4

[Jonathan Griffin (:jgriffin)]

Attachment: Make errors pre-empt emulator callback warning,

Addressed review comments

Comment 5

[Jonathan Griffin (:jgriffin)]

Attachment: Make errors pre-empt emulator callback warning,

Really address review comments (forgot to hit save previously)...

Comment 6

[Jonathan Griffin (:jgriffin)]

Comment on attachment 685828 [details] [diff] [review]
Make errors pre-empt emulator callback warning,

Carry r+ forward.

Comment 7

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/14c1eb8471f6

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/14c1eb8471f6

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/36c874449af9
https://hg.mozilla.org/releases/mozilla-beta/rev/c0f2bca4b970