Extend Test#add_cleanup to honor Promise-returning callbacks

[wpt-issue-mover]

Originally posted as w3c/testharness.js#261 by @jugglinmike on 10 Apr 2017, 16:41 UTC:

The documentation on add_cleanup reads:

Occasionally tests may create state that will persist beyond the test itself.
In order to ensure that tests are independent, such state should be cleaned
up once the test has a result. This can be achieved by adding cleanup
callbacks to the test. Such callbacks are registered using the add_cleanup
function on the test object. All registered callbacks will be run as soon as
the test result is known

The current implementation assumes that the "cleanup" operation is synchronous.
This is not always the case. For instance, tests for Service Workers may need
to unregister one or more workers after completion. Using the current
implementation of add_cleanup for this purpose will cause the operation to be
triggered at the correct moment, but it will not ensure that the operation is
complete before the test is signaled as "finished." This exposes a race
condition: the test runner may navigate away from the test document prior to
unregistration.

To support use cases like this, functions supplied to the add_cleanup method
should be able to signal completion. This should be communicated through a
Promise instance; testharness.js should not consider a test "complete" until
all Promises have settled.

[wpt-issue-mover]

Originally posted as w3c/testharness.js#261 (comment) by @foolip on 11 Apr 2017, 04:48 UTC:

If the promise is never settled, would that turn into a test timeout, a harness error, or something else?

[wpt-issue-mover]

Originally posted as w3c/testharness.js#261 (comment) by @jugglinmike on 11 Apr 2017, 14:21 UTC:

Currently, if a function provided to add_cleanup throws an error, the test
status is not affected, but the harness status is set to an error. I think that
if the returned promise is not settled within the timeout (or if it is rejected
within the timeout), then we should do the same.

[wpt-issue-mover]

Originally posted as w3c/testharness.js#261 (comment) by @foolip on 11 Apr 2017, 14:24 UTC:

Yep, that makes sense.

[jakearchibald]

This would be really useful for the service worker tests. In some cases we're meaningless promise_tests to do this.

[jdm]

Hmm, I guess this is the same as #6075 too. I thought I had seen someone actually do the work necessary, but it appears not.

[foolip]

Closing in favor of #6075. I too thought that this was already done.