Skip to content

Commit

Permalink
Bug 1597785 - fix review nits from bug 1594521, r=kmag
Browse files Browse the repository at this point in the history
Differential Revision: https://phabricator.services.mozilla.com/D53861

--HG--
extra : moz-landing-system : lando
  • Loading branch information
gijsk committed Nov 19, 2019
1 parent b9540d5 commit 734a4ad
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 13 deletions.
12 changes: 4 additions & 8 deletions services/settings/RemoteSettingsWorker.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ let gShutdownResolver = null;
class Worker {
constructor(source) {
if (gShutdown) {
Cu.reportError(
new Error("Can't create worker once shutdown has started")
);
Cu.reportError("Can't create worker once shutdown has started");
}
this.source = source;
this.worker = null;
Expand All @@ -52,7 +50,7 @@ class Worker {

async _execute(method, args = []) {
if (gShutdown) {
return Promise.reject(new Error("Remote Settings has shut down."));
throw new Error("Remote Settings has shut down.");
}
// (Re)instantiate the worker if it was terminated.
if (!this.worker) {
Expand Down Expand Up @@ -136,7 +134,7 @@ try {
!RemoteSettingsWorker.worker ||
!RemoteSettingsWorker.callbacks.size
) {
return Promise.resolve();
return null;
}
// Otherwise, return a promise that the worker will resolve.
return new Promise(resolve => {
Expand All @@ -145,9 +143,7 @@ try {
},
{
fetchState() {
return (
"Remaining: " + RemoteSettingsWorker.callbacks.size + " callbacks."
);
return `Remaining: ${RemoteSettingsWorker.callbacks.size} callbacks.`;
},
}
);
Expand Down
10 changes: 5 additions & 5 deletions services/settings/test/unit/test_remote_settings_worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ add_task(async function test_canonicaljson_merges_remote_into_local() {
);

Assert.equal(
'{"data":[{"id":"1","title":"title 1"},{"id":"2","title":"title b"}],"last_modified":"42"}',
serialized
serialized,
'{"data":[{"id":"1","title":"title 1"},{"id":"2","title":"title b"}],"last_modified":"42"}'
);
});

Expand Down Expand Up @@ -67,7 +67,7 @@ add_task(async function test_throws_error_if_worker_fails() {
} catch (e) {
error = e;
}
Assert.equal("TypeError: localRecords is null", error.message);
Assert.equal(error.message, "TypeError: localRecords is null");
});

add_task(async function test_stops_worker_after_timeout() {
Expand All @@ -78,7 +78,7 @@ add_task(async function test_stops_worker_after_timeout() {
);
// Run a task:
let serialized = await RemoteSettingsWorker.canonicalStringify([], [], 42);
Assert.equal('{"data":[],"last_modified":"42"}', serialized, "API works.");
Assert.equal(serialized, '{"data":[],"last_modified":"42"}', "API works.");
// Check that the worker gets stopped now the task is done:
await TestUtils.waitForCondition(() => !RemoteSettingsWorker.worker);
// Ensure the worker stays alive for 10 minutes instead:
Expand All @@ -89,8 +89,8 @@ add_task(async function test_stops_worker_after_timeout() {
// Run another task:
serialized = await RemoteSettingsWorker.canonicalStringify([], [], 42);
Assert.equal(
'{"data":[],"last_modified":"42"}',
serialized,
'{"data":[],"last_modified":"42"}',
"API still works."
);
Assert.ok(RemoteSettingsWorker.worker, "Worker should stay alive a bit.");
Expand Down

0 comments on commit 734a4ad

Please sign in to comment.