Closed
Bug 1316500
Opened 8 years ago
Closed 8 years ago
Upgrade from Task.jsm and Promise.jsm to async/await in Sync tests
Categories
(Firefox :: Sync, defect, P1)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
Firefox 53
| Tracking | Status | |
|---|---|---|
| firefox53 | --- | fixed |
People
(Reporter: markh, Assigned: markh)
References
Details
Attachments
(1 file)
Inspired by bug 1313956, it seems that changing the sync test suite to use async functions might get us some experience and should be largely immune from unexpected bad things happening for our users. Driven by a little procrastination, I did this and also removed all uses of Promise.jsm (using PromiseUtils.defer() for the few places where the change didn't seem worthwhile) and also started using other promise-based helpers to make the tests easier to read.
| Comment hidden (mozreview-request) |
|
||
Comment 2•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8809270 [details] Bug 1316500 - remove use of tasks and promise.jsm promises from Sync tests. https://reviewboard.mozilla.org/r/91862/#review91978 I've just asked in #fx-team, and we're not supposed to land code (even test code) using it for another cycle due to concerns that async/await may be turned off for web compat reasons (... that I don't fully understand, but apparently it's a request from the spidermonkey team?). Other than that it looks fine, I didn't look *that* closely, but it seems mechanical and as long as the tests pass I'm content with landing this once we get the go-ahead on using async/await in firefox.
Attachment #8809270 -
Flags: review?(tchiovoloni)
|
Assignee | |
Updated•8 years ago
|
Flags: needinfo?(markh)
|
|
Assignee | |
Comment 3•8 years ago
|
||
Comment on attachment 8809270 [details] Bug 1316500 - remove use of tasks and promise.jsm promises from Sync tests. I just asked in #jsapi and doing test only changes for 53 is fine and probably helpful. Even production code in 53 is probably OK with some caveats (there's still a tiny risk of backout, there are some questions about performance, but even then it's almost certainly not worse than Task.jsm) - but for now, let's get these tests in.
Flags: needinfo?(markh)
Attachment #8809270 -
Flags: review?(tchiovoloni)
|
|
||
Comment 4•8 years ago
|
||
| mozreview-review | ||
Comment on attachment 8809270 [details] Bug 1316500 - remove use of tasks and promise.jsm promises from Sync tests. https://reviewboard.mozilla.org/r/91862/#review93130 In that case, ship it!
Attachment #8809270 -
Flags: review?(tchiovoloni) → review+
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
Pushed by mhammond@skippinet.com.au: https://hg.mozilla.org/integration/autoland/rev/ca02f30bc3a0 remove use of tasks and promise.jsm promises from Sync tests. r=tcsc
Backout by philringnalda@gmail.com: https://hg.mozilla.org/integration/autoland/rev/6239169c1eff Backed out changeset ca02f30bc3a0 for eslint failures
|
||
Comment 9•8 years ago
|
||
If we add a services/sync/.eslintrc.js file (like http://searchfox.org/mozilla-central/source/.eslintrc.js), and set the `"parserOptions": { "ecmaVersion": 8 }` option, I think that'll squelch the failures.
|
|
||
Updated•8 years ago
|
Priority: -- → P1
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 13•8 years ago
|
||
Pushed by mhammond@skippinet.com.au: https://hg.mozilla.org/integration/autoland/rev/843b7449f803 remove use of tasks and promise.jsm promises from Sync tests. r=tcsc
|
||
Comment 14•8 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/843b7449f803
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
You need to log in
before you can comment on or make changes to this bug.
Description
•