Closed
Bug 1187555
Opened 9 years ago
Closed 9 years ago
Convert callbacks in worker_test.js into Promises
Categories
(Firefox OS Graveyard :: Gaia::Keyboard, defect)
Tracking
(b2g-master fixed)
RESOLVED
FIXED
FxOS-S5 (21Aug)
Tracking | Status | |
---|---|---|
b2g-master | --- | fixed |
People
(Reporter: timdream, Assigned: anubhav.worklinux, Mentored)
References
Details
(Whiteboard: [mentor-lang=zh][lang=js])
Attachments
(1 file)
+++ This bug was initially created as a clone of Bug #1044984 +++
We should use Promises and Promise chain to describe a series of async test operations, and worker_test.js do exactly that.
If you could like to take this bug, you would need to figure out how to run the test locally, and understand Promise and how Promise chain work with mocha (hint: |grep| other tests).
Please also read Julien's comment here:
https://github.com/mozilla-b2g/gaia/pull/31070.
Which is a pull request of bug 1044984. The comment on the bug should also help you understand what the test do to assert the output or worker.js.
Assignee | ||
Comment 1•9 years ago
|
||
Hi, i would like to work on this bug. i read Julien's comments on your bug to make the tests promise based. Would submit a patch soon.
thanks!
Comment 3•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8643505 -
Flags: review?(timdream)
Reporter | ||
Comment 4•9 years ago
|
||
Comment on attachment 8643505 [details] [review]
[gaia] anu7495:my-fix > mozilla-b2g:master
Nice! The patch need one more improvement:
1. Let's change the name of the function passed from mocha from |next| to |done| for consistency.
2. The prediction() function can also be a function that returns a promise, instead of take a callback.
I am not sure if prediction() is the only async function left in the file that takes callbacks. But the goal of this bug is to convert async function pattern to promises so we should convert all of them.
Thank you!
Attachment #8643505 -
Flags: review?(timdream) → feedback+
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8643505 [details] [review]
[gaia] anu7495:my-fix > mozilla-b2g:master
>https://github.com/mozilla-b2g/gaia/pull/31250
Attachment #8643505 -
Flags: review?(timdream)
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8643505 [details] [review]
[gaia] anu7495:my-fix > mozilla-b2g:master
I've left a comment on GitHub. Looks like a good progress but there is a common Promise pitfall on prediction(). I hope my explanation is clear enough there.
Thanks!
Attachment #8643505 -
Flags: review?(timdream)
Assignee | ||
Updated•9 years ago
|
Attachment #8643505 -
Flags: review?(timdream)
Reporter | ||
Comment 7•9 years ago
|
||
Comment on attachment 8643505 [details] [review]
[gaia] anu7495:my-fix > mozilla-b2g:master
Thanks for the patch. Please update the patch according to the two comments before asking for check-in.
Attachment #8643505 -
Flags: review?(timdream) → review+
Assignee | ||
Comment 8•9 years ago
|
||
Thanks!
Will make the changes and ask for check-in soon.
Thanks again.
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-b2g-master:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S5 (21Aug)
You need to log in
before you can comment on or make changes to this bug.
Description
•