Closed Bug 815441 Opened 8 years ago Closed 8 years ago

Errors should pre-empt emulator callback warning

Categories

(Testing :: Marionette, defect)

defect
Not set
normal

Tracking

(firefox18 fixed, firefox19 fixed, firefox20 fixed)

RESOLVED FIXED
mozilla20
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: jgriffin, Assigned: jgriffin)

Details

Attachments

(1 file, 2 obsolete files)

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.
Attachment #685401 - Flags: review?(mdas)
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?
Attachment #685401 - Flags: review?(mdas) → review+
(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.
Addressed review comments
Attachment #685401 - Attachment is obsolete: true
Really address review comments (forgot to hit save previously)...
Attachment #685827 - Attachment is obsolete: true
Comment on attachment 685828 [details] [diff] [review]
Make errors pre-empt emulator callback warning,

Carry r+ forward.
Attachment #685828 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/14c1eb8471f6
Whiteboard: [automation-needed-in-aurora][automation-needed-in-beta]
Target Milestone: --- → mozilla20
https://hg.mozilla.org/mozilla-central/rev/14c1eb8471f6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.