Skip to content

Commit 3e2af2c

Browse files
author
Andreu Botella
committed
Change Tests.prototype.all_done to require either tests or pending 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.
1 parent 7df27fb commit 3e2af2c

File tree

3 files changed

+32
-16
lines changed

3 files changed

+32
-16
lines changed

resources/testharness.js

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2942,7 +2942,8 @@
29422942
};
29432943

29442944
Tests.prototype.all_done = function() {
2945-
return this.tests.length > 0 && test_environment.all_loaded &&
2945+
return (this.tests.length > 0 || this.pending_remotes.length > 0) &&
2946+
test_environment.all_loaded &&
29462947
(this.num_pending === 0 || this.is_aborted) && !this.wait_for_finish &&
29472948
!this.processing_callbacks &&
29482949
!this.pending_remotes.some(function(w) { return w.running; });
@@ -3168,18 +3169,33 @@
31683169
);
31693170
};
31703171

3171-
Tests.prototype.fetch_tests_from_worker = function(worker) {
3172+
Tests.prototype.fetch_tests_from_worker = function(promiseOrWorker) {
31723173
if (this.phase >= this.phases.COMPLETE) {
31733174
return;
31743175
}
31753176

3176-
var remoteContext = this.create_remote_worker(worker);
3177-
this.pending_remotes.push(remoteContext);
3178-
return remoteContext.done;
3177+
if (typeof promiseOrWorker === "function") {
3178+
promiseOrWorker = promiseOrWorker();
3179+
}
3180+
3181+
if (self.Promise && promiseOrWorker instanceof self.Promise) {
3182+
const dummy_remote = { running: true };
3183+
this.pending_remotes.push(dummy_remote);
3184+
return promiseOrWorker.then((worker) => {
3185+
dummy_remote.running = false;
3186+
var remoteContext = this.create_remote_worker(worker);
3187+
this.pending_remotes.push(remoteContext);
3188+
return remoteContext.done;
3189+
});
3190+
} else {
3191+
var remoteContext = this.create_remote_worker(promiseOrWorker);
3192+
this.pending_remotes.push(remoteContext);
3193+
return remoteContext.done;
3194+
}
31793195
};
31803196

3181-
function fetch_tests_from_worker(port) {
3182-
return tests.fetch_tests_from_worker(port);
3197+
function fetch_tests_from_worker(promiseOrPort) {
3198+
return tests.fetch_tests_from_worker(promiseOrPort);
31833199
}
31843200
expose(fetch_tests_from_worker, 'fetch_tests_from_worker');
31853201

tools/serve/serve.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,13 @@ class ServiceWorkersHandler(HtmlWrapperHandler):
311311
<script src="/resources/testharnessreport.js"></script>
312312
<div id=log></div>
313313
<script>
314-
(async function() {
314+
fetch_tests_from_worker(async function() {
315315
const scope = 'does/not/exist';
316316
let reg = await navigator.serviceWorker.getRegistration(scope);
317317
if (reg) await reg.unregister();
318318
reg = await navigator.serviceWorker.register("%(path)s%(query)s", {scope});
319-
fetch_tests_from_worker(reg.installing);
320-
})();
319+
return reg.installing;
320+
});
321321
</script>
322322
"""
323323

@@ -333,16 +333,16 @@ class ServiceWorkerModulesHandler(HtmlWrapperHandler):
333333
<script src="/resources/testharnessreport.js"></script>
334334
<div id=log></div>
335335
<script>
336-
(async function() {
336+
fetch_tests_from_worker(async function() {
337337
const scope = 'does/not/exist';
338338
let reg = await navigator.serviceWorker.getRegistration(scope);
339339
if (reg) await reg.unregister();
340340
reg = await navigator.serviceWorker.register(
341341
"%(path)s%(query)s",
342342
{ scope, type: 'module' },
343343
);
344-
fetch_tests_from_worker(reg.installing);
345-
})();
344+
return reg.installing;
345+
});
346346
</script>
347347
"""
348348

tools/wptserve/tests/functional/docroot/foo.any.serviceworker.html

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
<script src="/resources/testharnessreport.js"></script>
66
<div id=log></div>
77
<script>
8-
(async function() {
8+
fetch_tests_from_worker(async function() {
99
const scope = 'does/not/exist';
1010
let reg = await navigator.serviceWorker.getRegistration(scope);
1111
if (reg) await reg.unregister();
1212
reg = await navigator.serviceWorker.register("/foo.any.worker.js", {scope});
13-
fetch_tests_from_worker(reg.installing);
14-
})();
13+
return reg.installing;
14+
});
1515
</script>

0 commit comments

Comments
 (0)