Skip to content

Commit

Permalink
Change Tests.prototype.all_done to require either tests or pending …
Browse files Browse the repository at this point in the history
…remotes

`Tests.prototype.all_done` used to treat the presence of at least one
test as a necessary condition for all tests to be done. This would be
false in cases where an exception is thrown inside the `setup` function,
but the code path that runs in that case doesn't use the `all_done()`
function.

However, if an exception is thrown inside the `setup` function in a
worker test, `tests.all_done()` is called in that code path, which
results in the completion message from the worker being ignored. This
change fixes this by changing the condition to require the presence of
either tests or remote contexts.

For service worker tests, however, that is not enough, since service
worker registration is asynchronous. This change lets the
`fetch_tests_from_worker` function take a promise or an async function
that returns the worker, and it guarantees that the test won't be
considered complete until the promise is resolved. The HTML boilerplate
for service worker tests is also changed in turn.

Closes #29777.
  • Loading branch information
Andreu Botella committed Aug 11, 2021
1 parent 5998702 commit 8b3feba
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 16 deletions.
30 changes: 23 additions & 7 deletions resources/testharness.js
Original file line number Diff line number Diff line change
Expand Up @@ -2942,7 +2942,8 @@
};

Tests.prototype.all_done = function() {
return this.tests.length > 0 && test_environment.all_loaded &&
return (this.tests.length > 0 || this.pending_remotes.length > 0) &&
test_environment.all_loaded &&
(this.num_pending === 0 || this.is_aborted) && !this.wait_for_finish &&
!this.processing_callbacks &&
!this.pending_remotes.some(function(w) { return w.running; });
Expand Down Expand Up @@ -3168,18 +3169,33 @@
);
};

Tests.prototype.fetch_tests_from_worker = function(worker) {
Tests.prototype.fetch_tests_from_worker = function(promiseOrWorker) {
if (this.phase >= this.phases.COMPLETE) {
return;
}

var remoteContext = this.create_remote_worker(worker);
this.pending_remotes.push(remoteContext);
return remoteContext.done;
if (typeof promiseOrWorker === "function") {
promiseOrWorker = promiseOrWorker();
}

if (self.Promise && promiseOrWorker instanceof self.Promise) {
const dummy_remote = { running: true };
this.pending_remotes.push(dummy_remote);
return promiseOrWorker.then((worker) => {
dummy_remote.running = false;
var remoteContext = this.create_remote_worker(worker);
this.pending_remotes.push(remoteContext);
return remoteContext.done;
});
} else {
var remoteContext = this.create_remote_worker(promiseOrWorker);
this.pending_remotes.push(remoteContext);
return remoteContext.done;
}
};

function fetch_tests_from_worker(port) {
return tests.fetch_tests_from_worker(port);
function fetch_tests_from_worker(promiseOrPort) {
return tests.fetch_tests_from_worker(promiseOrPort);
}
expose(fetch_tests_from_worker, 'fetch_tests_from_worker');

Expand Down
12 changes: 6 additions & 6 deletions tools/serve/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,13 @@ class ServiceWorkersHandler(HtmlWrapperHandler):
<script src="/resources/testharnessreport.js"></script>
<div id=log></div>
<script>
(async function() {
fetch_tests_from_worker(async function() {
const scope = 'does/not/exist';
let reg = await navigator.serviceWorker.getRegistration(scope);
if (reg) await reg.unregister();
reg = await navigator.serviceWorker.register("%(path)s%(query)s", {scope});
fetch_tests_from_worker(reg.installing);
})();
return reg.installing;
});
</script>
"""

Expand All @@ -333,16 +333,16 @@ class ServiceWorkerModulesHandler(HtmlWrapperHandler):
<script src="/resources/testharnessreport.js"></script>
<div id=log></div>
<script>
(async function() {
fetch_tests_from_worker(async function() {
const scope = 'does/not/exist';
let reg = await navigator.serviceWorker.getRegistration(scope);
if (reg) await reg.unregister();
reg = await navigator.serviceWorker.register(
"%(path)s%(query)s",
{ scope, type: 'module' },
);
fetch_tests_from_worker(reg.installing);
})();
return reg.installing;
});
</script>
"""

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
<script src="/resources/testharnessreport.js"></script>
<div id=log></div>
<script>
(async function() {
fetch_tests_from_worker(async function() {
const scope = 'does/not/exist';
let reg = await navigator.serviceWorker.getRegistration(scope);
if (reg) await reg.unregister();
reg = await navigator.serviceWorker.register("/foo.any.worker.js", {scope});
fetch_tests_from_worker(reg.installing);
})();
return reg.installing;
});
</script>

0 comments on commit 8b3feba

Please sign in to comment.