Import the fetch-request-redirect.https.html test from Blink

RESOLVED FIXED in Firefox 45

Status

()

Core
DOM: Service Workers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Ehsan, Assigned: bkelly)

Tracking

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Reporter)

Comment 1

2 years ago
Created attachment 8679761 [details] [diff] [review]
Import the fetch-request-redirect.https.html test from Blink
Attachment #8679761 - Flags: review?(josh)
(Reporter)

Updated

2 years ago
Blocks: 1200677

Comment 2

2 years ago
Comment on attachment 8679761 [details] [diff] [review]
Import the fetch-request-redirect.https.html test from Blink

Review of attachment 8679761 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/web-platform/mozilla/tests/service-workers/service-worker/resources/fetch-rewrite-worker.js
@@ +16,5 @@
>    var init = {};
>    init['method'] = params['method'] || base['method'];
>    init['mode'] = params['mode'] || base['mode'];
>    init['credentials'] = params['credentials'] || base['credentials'];
> +  init['redirect'] = params['redirect-mode'] || base['redirect'];

This seems like we should be rewriting the test, rather than changing this.
Attachment #8679761 - Flags: review?(josh) → review+
(Reporter)

Updated

2 years ago
Depends on: 1219469
(Reporter)

Comment 5

2 years ago
I can reproduce the timeout in an opt build but not on a debug build.  This test sets up 2 iframes and waits for 10 seconds to wait for an asynchronous operation.  RyanVM said on IRC that the default timeout for these tests is 20s, but eyeballing the test run in an opt build, the test times out in clearly less than 20 seconds.  Furthermore, if I run it with --timeout-multiplier 2, it passes, which seems to suggest to me that the default timeout values in debug vs opt runs is actually different!

James, what is the default timeout value?  And what's the equivalent of SimpleTest.requestLongerTimeout() in WPT?  Thanks!
Flags: needinfo?(james)
(Reporter)

Comment 6

2 years ago
I also tried adding |<meta name="timeout" content="long">| to the test to no avail.
Flags: needinfo?(ehsan)
The default timeout is 10s in opt builds. Debug builds have a default timeout multiplier of three, chosen because tests seems to be mostly stable with that value, but not with 2.

<meta name="timeout" content="long"> before <script src=/resources/testharness.js> is the right way to update the timeout value, but because the harness also sets a timeout you need to make sure that the metadata is updated after you add this to the file; run with --manifest-update to do the update.
Flags: needinfo?(james)
(Reporter)

Comment 8

2 years ago
Thanks a lot, James!
(Assignee)

Updated

2 years ago
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
(Assignee)

Comment 9

2 years ago
Created attachment 8688718 [details] [diff] [review]
Bug-1219085---Import-the-fetch-request-redirecthtt.patch

Updated with long timeout in manifest.
Attachment #8679761 - Attachment is obsolete: true
Attachment #8688718 - Flags: review+
(Assignee)

Comment 10

2 years ago
The e10s path is quite racy and we're unlikely to fix it in the short term.  Lets see if we can land this test for non-e10s only.

  https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5dc82e18882
(Assignee)

Comment 11

2 years ago
I forgot to hg add the wpt .ini file in that last try push, but the test clearly passes in non-e10s.  I'm just going to land with the ini.
(Assignee)

Updated

2 years ago
No longer depends on: 1219469

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6d2641e05d2b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.