Closed
Bug 756966
Opened 10 years ago
Closed 10 years ago
Fail hard when runEmulatorCmd callbacks still exist at the end of a test
Categories
(Testing :: Marionette, defect)
Testing
Marionette
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla15
People
(Reporter: philikon, Assigned: jgriffin)
References
Details
Attachments
(1 file, 1 obsolete file)
2.88 KB,
patch
|
philikon
:
review+
|
Details | Diff | Splinter Review |
If a test isn't written properly, it may call finish() before a runEmulatorCmd has returned. In that case we should just fail hard. Syncing a few async events together -- in the worst case -- shouldn't be too hard for the test author.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jgriffin
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #626614 -
Flags: review?(philipp)
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 626614 [details] [diff] [review] patch v0.1 Actually this causes a test failure, investigating.
Attachment #626614 -
Flags: review?(philipp)
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 626614 [details] [diff] [review] patch v0.1 Review of attachment 626614 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/marionette/marionette-listener.js @@ +290,5 @@ > > + // emulator callbacks > + sandbox._emu_cb_id = 0; > + sandbox._emu_cbs = {}; > + I don't understand why we have to leak these into the sandbox. sandbox.asyncComplete closes over the global scope and should be able to access them as global variables just fine. @@ +714,5 @@ > function emulatorCmdResult(msg) { > let message = msg.json; > + if (!sandbox || !sandbox._emu_cbs[message.id]) { > + // we fail silently since an error should have been generated in finish() > + return; Not a failure per se. It should be totally valid to not pass a callback at all. In fact, I think that's how we should write most runEmulatorCmd() invocations, after musing a bit about event racing. I'll post more in bug 756607.
Assignee | ||
Comment 4•10 years ago
|
||
Moved the callbacks back out of the sandbox. This version passes all tests.
Attachment #626614 -
Attachment is obsolete: true
Attachment #626636 -
Flags: review?(philipp)
Reporter | ||
Updated•10 years ago
|
Attachment #626636 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 5•10 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/646cbeae01a0
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in
before you can comment on or make changes to this bug.
Description
•