Convert callbacks in worker_test.js into Promises

RESOLVED FIXED in Firefox OS master

Status

Firefox OS
Gaia::Keyboard
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: timdream, Assigned: anubhav.worklinux, Mentored)

Tracking

unspecified
FxOS-S5 (21Aug)
x86
Linux

Firefox Tracking Flags

(b2g-master fixed)

Details

(Whiteboard: [mentor-lang=zh][lang=js])

Attachments

(1 attachment)

+++ 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

2 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!
Thanks!
Assignee: nobody → anubhav.worklinux
Status: NEW → ASSIGNED

Comment 3

2 years ago
Created attachment 8643505 [details] [review]
[gaia] anu7495:my-fix > mozilla-b2g:master
(Assignee)

Updated

2 years ago
Attachment #8643505 - Flags: review?(timdream)
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

2 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)
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

2 years ago
Attachment #8643505 - Flags: review?(timdream)
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

2 years ago
Thanks!
Will make the changes and ask for check-in soon.
Thanks again.
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/e3b15bd8607686438f394d491d5ab0372fc09d70
Status: ASSIGNED → RESOLVED
Last Resolved: 2 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.