Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(309)

Issue 2889153004: Upstream service worker "request" tests to WPT (Closed)

Created:
3 years, 7 months ago by mike3
Modified:
3 years, 7 months ago
Reviewers:
falken
CC:
chromium-reviews, michaeln, jsbell+serviceworker_chromium.org, kenjibaheux+watch_chromium.org, shimazu+serviceworker_chromium.org, serviceworker-reviews, blink-reviews-w3ctests_chromium.org, nhiroki, kinuko+serviceworker, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, tzik
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Upstream service worker "request" tests to WPT **request-body-blob** Re-locate test file to Web Platform Test directory for eventual automated upstreaming. Update resource URLs to suitable values for that project. Schedule frame removal to occur following test completion. Introduce a "use strict" directive. **request-end-to-end** This test exists in both WPT and the Chromium project tree, although the two implementations differ in the following ways: - Test structure - the WPT version uses `async_test` while the Chromium version uses `promise_test` - Client-worker communication - the WPT version mediates communication between the client and the service worker via a dedicated MessageChannel. The Chromium version implements this communication via the "Handle Fetch" mechanism. - "User-Agent" header assertion - the WPT version asserts that the "User-Agent" header is not set while the Chromium version asserts that it is set to the value of the client document's `navigator.userAgent` string (the former is correct) - "Request construction assertion" - the Chromium version asserts that the expected exception is thrown when attempting to construct a new Request instance from the intercepted instance. The WPT version makes no such assertion. Modify the WPT version to use the test structure from the Chromium version, as this improves test readability. Adopt the communication solution from the Chromium version, as this improves maintainability and makes the test less susceptible to failure from unrelated bugs. Persist the "User-Agent" assertion from the WPT version and incorporate the "Request construction" assertion from the Chromium version. Add "use strict" directives to the test files. Because Chromium continues to fail the WPT version of the test, the Chromium version cannot be removed without negatively effecting test coverage in that project. Persist the Chromium version of the test, but re-name the file, remove the faulty assertion, and add a comment documenting its deprecated status. BUG=688116, 595993 R=falken@chromium.org Review-Url: https://codereview.chromium.org/2889153004 Cr-Commit-Position: refs/heads/master@{#473587} Committed: https://chromium.googlesource.com/chromium/src/+/b7d76092f05699bab9d8d9c3fd5433f04dfd3334

Patch Set 1 #

Patch Set 2 : Upstream service worker "request" tests to WPT #

Total comments: 2

Patch Set 3 : Re-instate faulty assertion #

Patch Set 4 : Add "use strict" directive #

Unified diffs Side-by-side diffs Delta from patch set Stats (+77 lines, -200 lines) Patch
A + third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/request-body-blob.https.html View 2 chunks +10 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/request-end-to-end.https.html View 1 1 chunk +32 lines, -67 lines 0 comments Download
A + third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/request-body-blob-iframe.html View 0 chunks +-1 lines, --1 lines 0 comments Download
A + third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/request-body-blob-worker.js View 0 chunks +-1 lines, --1 lines 0 comments Download
M third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/request-end-to-end-worker.js View 1 chunk +23 lines, -32 lines 0 comments Download
A + third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.request-end-to-end.html View 1 2 2 chunks +12 lines, -0 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/serviceworker/request-body-blob.html View 1 chunk +0 lines, -28 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/serviceworker/request-end-to-end.html View 1 chunk +0 lines, -37 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/request-body-blob-iframe.html View 1 chunk +0 lines, -20 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/request-body-blob-worker.js View 1 chunk +0 lines, -11 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/request-end-to-end-worker.js View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (5 generated)
falken
I think this looks good. Would you be able to provide a diff of current ...
3 years, 7 months ago (2017-05-19 03:46:21 UTC) #3
mike3
> I think this looks good. Would you be able to provide a diff of ...
3 years, 7 months ago (2017-05-19 16:12:12 UTC) #4
falken
thanks, lgtm
3 years, 7 months ago (2017-05-22 04:44:44 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2889153004/60001
3 years, 7 months ago (2017-05-22 14:48:13 UTC) #7
commit-bot: I haz the power
3 years, 7 months ago (2017-05-22 16:11:45 UTC) #10
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b7d76092f05699bab9d8d9c3fd54...

Powered by Google App Engine
This is Rietveld 408576698