Closed Bug 1230847 Opened 4 years ago Closed 4 years ago

Callback of runEmulatorCmd/runEmulatorShell doesn't be triggered correctly when running two requests in parallel

Categories

(Testing :: Marionette, defect)

defect
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed

People

(Reporter: edgar, Assigned: edgar)

References

Details

(Keywords: pi-marionette-server)

Attachments

(1 file, 1 obsolete file)

Bug 1211489 allows running requests in parallel, but when test script execute two runEmulatorCmd in parallel, only callback will be triggered correctly. After checking the log, marionette did receive both response, but only one callback is triggered.
Blocks: 1211489
Attached patch Patch, v1 (obsolete) — Splinter Review
I found Bug 1211489 changed to use |asyncTestCommandId| as id for every runEmulator* message which causes problem on handling parallel request. In this patch, I just rollback to use increasing |_emu_cb_id| as id, not sure if this is a good way to fix this. :ato, could you take a look at the patch? Thank you.
Attachment #8696365 - Flags: feedback?(ato)
This patch looks fine to me.  Can you please submit it to mozreview?
Assignee: nobody → echen
Status: NEW → ASSIGNED
Attachment #8696365 - Flags: feedback?(ato) → feedback+
Bug 1230847 - Use increasing value as id for runEmulatorCmd and runEmulatorShell. r?ato
Attachment #8696968 - Flags: review?(ato)
Attachment #8696365 - Attachment is obsolete: true
Comment on attachment 8696968 [details]
MozReview Request: Bug 1230847 - Use increasing value as id for runEmulatorCmd and runEmulatorShell. r?ato

https://reviewboard.mozilla.org/r/27443/#review24755
Attachment #8696968 - Flags: review?(ato) → review+
Please do a try run before integrating.
(In reply to Andreas Tolfsen (:ato) from comment #5)
> Please do a try run before integrating.

Sure, will do this. Thank you.
https://hg.mozilla.org/mozilla-central/rev/bfb273191c68
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.