Upstream service wrkr `importScripts` tests to WPT
In order to contribute this test logic to the Web Platform Tests
project, the invalid assertion it includes needed to be corrected.
Because the test was previously formatted as a single sub-test, the
resultant failure tended to obscure the results of assertions for
independent conditions. Re-factor the test to support more fine-grained
reporting, and white-list the specific failure as "allowed" in the
Chromium build infrastructure.
BUG=688116, 719052R=mek@chromium.org
Review-Url: https://codereview.chromium.org/2864783002
Cr-Commit-Position: refs/heads/master@{#470105}
Committed: https://chromium.googlesource.com/chromium/src/+/e9f34ed893dd0ab15ac4834becee26c4886620d7
Hi Mek, I created bug 719052 to track the spec violation, but I can't find ...
3 years, 7 months ago
(2017-05-05 22:18:49 UTC)
#1
Hi Mek,
I created bug 719052 to track the spec violation, but I can't find any way to
reference that in the new `-expected.txt` file. Is that alright?
Marijn Kruisselbrink
On 2017/05/05 at 22:18:49, mike wrote: > I created bug 719052 to track the spec ...
3 years, 7 months ago
(2017-05-06 00:08:50 UTC)
#2
On 2017/05/05 at 22:18:49, mike wrote:
> I created bug 719052 to track the spec violation, but I can't find any way to
reference that in the new `-expected.txt` file. Is that alright?
Yeah, that's the downside of expected files, now way to clearly link them to
bugs. Not really anything you can about it. At least with the bug in the CL
description you can always figure out what's going on with the expectation with
very minimal digging.
lgtm with two small nits
https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html
(right):
https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html:9:
function post_and_wait_for_reply(worker, message) {
this function doesn't seem to be used?
https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html:27:
.then(r => { add_completion_callback(() => r.unregister()); });
this should probably just be r => r.unregister(), right? To make sure the test
actually waits for unregister to complete.
mike3
Thanks! https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html File third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html (right): https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html#newcode9 third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html:9: function post_and_wait_for_reply(worker, message) { On 2017/05/06 00:08:49, Marijn ...
3 years, 7 months ago
(2017-05-08 18:49:42 UTC)
#3
Thanks!
https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html
(right):
https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html:9:
function post_and_wait_for_reply(worker, message) {
On 2017/05/06 00:08:49, Marijn Kruisselbrink wrote:
> this function doesn't seem to be used?
Done.
https://codereview.chromium.org/2864783002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html:27:
.then(r => { add_completion_callback(() => r.unregister()); });
On 2017/05/06 00:08:49, Marijn Kruisselbrink wrote:
> this should probably just be r => r.unregister(), right? To make sure the test
> actually waits for unregister to complete.
Done.
mike3
The CQ bit was checked by mike@mikepennisi.com
3 years, 7 months ago
(2017-05-08 18:49:45 UTC)
#4
CQ is committing da patch. Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494269385977750, "parent_rev": "ebd2fa73e3046f3d6397545285a41fa1d78c4be8", "commit_rev": "e9f34ed893dd0ab15ac4834becee26c4886620d7"}
3 years, 7 months ago
(2017-05-08 20:25:53 UTC)
#7
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1494269385977750,
"parent_rev": "ebd2fa73e3046f3d6397545285a41fa1d78c4be8", "commit_rev":
"e9f34ed893dd0ab15ac4834becee26c4886620d7"}
commit-bot: I haz the power
Description was changed from ========== Upstream service wrkr `importScripts` tests to WPT In order to ...
3 years, 7 months ago
(2017-05-08 20:26:03 UTC)
#8
Message was sent while issue was closed.
Description was changed from
==========
Upstream service wrkr `importScripts` tests to WPT
In order to contribute this test logic to the Web Platform Tests
project, the invalid assertion it includes needed to be corrected.
Because the test was previously formatted as a single sub-test, the
resultant failure tended to obscure the results of assertions for
independent conditions. Re-factor the test to support more fine-grained
reporting, and white-list the specific failure as "allowed" in the
Chromium build infrastructure.
BUG=688116, 719052
R=mek@chromium.org
==========
to
==========
Upstream service wrkr `importScripts` tests to WPT
In order to contribute this test logic to the Web Platform Tests
project, the invalid assertion it includes needed to be corrected.
Because the test was previously formatted as a single sub-test, the
resultant failure tended to obscure the results of assertions for
independent conditions. Re-factor the test to support more fine-grained
reporting, and white-list the specific failure as "allowed" in the
Chromium build infrastructure.
BUG=688116, 719052
R=mek@chromium.org
Review-Url: https://codereview.chromium.org/2864783002
Cr-Commit-Position: refs/heads/master@{#470105}
Committed:
https://chromium.googlesource.com/chromium/src/+/e9f34ed893dd0ab15ac4834becee...
==========
commit-bot: I haz the power
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/e9f34ed893dd0ab15ac4834becee26c4886620d7
3 years, 7 months ago
(2017-05-08 20:26:05 UTC)
#9
Issue 2864783002: Upstream service wrkr `importScripts` tests to WPT
(Closed)
Created 3 years, 7 months ago by mike3
Modified 3 years, 7 months ago
Reviewers: Marijn Kruisselbrink
Base URL:
Comments: 4