Closed Bug 1493416 Opened 6 years ago Closed 6 years ago

[wpt-sync] Sync PR 13165 - [service-workers] Refactor to use async cleanup

Categories

(Core :: DOM: Service Workers, enhancement, P4)

enhancement

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

()

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 13165 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/13165
Details from upstream follow.

Mike Pennisi <mike@mikepennisi.com> wrote:
>  [service-workers] Refactor to use async cleanup
>  
>  Previously, many tests un-registered service workers only after all
>  assertions had been satisfied. This meant that failing tests would *not*
>  un-register workers. In order to account for workers that persisted from
>  previous test failures, the tests included "setup" code to defensively
>  un-register such workers.
>  
>  The `Test#add_cleanup` method was recently extended to support
>  asynchronous "clean up" operations [1], but the functionality is only
>  available to tests declared using `promise_test`. Update tests which
>  were previously declared with `async_test` and use the new "async
>  cleanup" API to schedule service worker un-registration so that it
>  occurs regardless of the result of each test.
>  
>  [1] https://github.com/web-platform-tests/wpt/pull/8748
>  
>  ---
>  
>  This patch is not intended to influence test results. To verify that, I used
>  the WPT CLI to run the affected tests in Chromium and Firefox, comparing the
>  summary it produced both on `master` and on this branch.
>  
>  Chromium on `master`:
>  
>      web-platform-test
>      ~~~~~~~~~~~~~~~~~
>      Ran 96 checks (32 tests, 64 subtests)
>      Expected results: 93
>      Unexpected results: 3
>        subtest: 3 (3 fail)
>  
>  Chromium with patch applied:
>  
>      web-platform-test
>      ~~~~~~~~~~~~~~~~~
>      Ran 96 checks (32 tests, 64 subtests)
>      Expected results: 93
>      Unexpected results: 3
>        subtest: 3 (3 fail)
>  
>  Firefox on `master`:
>  
>      web-platform-test
>      ~~~~~~~~~~~~~~~~~
>      Ran 96 checks (32 tests, 64 subtests)
>      Expected results: 89
>      Unexpected results: 7
>        test: 1 (1 timeout)
>        subtest: 6 (4 fail, 2 timeout)
>  
>  Firefox with patch applied:
>  
>      web-platform-test
>      ~~~~~~~~~~~~~~~~~
>      Ran 96 checks (32 tests, 64 subtests)
>      Expected results: 89
>      Unexpected results: 7
>        test: 1 (1 timeout)
>        subtest: 6 (4 fail, 1 notrun, 1 timeout)
>  
>  The discrepancy in Firefox is expected. In `master`, two of the four
>  `async_tests` in `fetch-request-css-images.https.html` time out in Firefox.
>  This branch refactors all four into `promise_test`s. `promise_test`s run in
>  series (unlike the parallel `async_test`s), so the harness stops running the
>  tests following the first timeout and reports the final test as "NOT RUN".
>  
>  I've attempted to ease the review process by splitting the work across a few
>  pull requests, but this patch is still pretty onerous. I'd be happy to break it
>  up further if desired.
Component: web-platform-tests → DOM: Service Workers
Product: Testing → Core
Failed to get results from try push
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/de13d03cc138
[wpt PR 13165] - [service-workers] Refactor to use async cleanup, a=testonly
https://hg.mozilla.org/integration/mozilla-inbound/rev/2980304187d7
[wpt PR 13165] - Update wpt metadata, a=testonly
https://hg.mozilla.org/mozilla-central/rev/de13d03cc138
https://hg.mozilla.org/mozilla-central/rev/2980304187d7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.