Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split fetch CORP tests as HTTP and HTTPS. #14907

Merged
merged 2 commits into from
Jan 18, 2019

Conversation

youennf
Copy link
Contributor

@youennf youennf commented Jan 16, 2019

Add fetch.https.any.js in addition to fetch.any.js.
Make fetch.any.js not run in service worker.
Make fetch.https.any.js run also in service worker.
Update a test in fetch.any.js for the "same-site HTTP -> HTTPS" case.

Run service worker tests for HTTPS only
fetch.https.any.js contains a subset of tests from fetch.any.js.
@anforowicz
Copy link
Contributor

Drive-by comment... feel free to ignore :-)

Thank you very much for putting together the PR. I can confirm that the tests pass against my Chromium WIP CL @ https://chromium-review.googlesource.com/c/chromium/src/+/1416813.

And separate thanks for having the luxury of implementing the feature with already existing test coverage . One benefit is that it prevent me from forgetting to support redirects (the last testcase in fetch.any.js fails). Thanks!

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, two minor nits.

@gsnedders gsnedders removed their assignment Jan 17, 2019
@youennf
Copy link
Contributor Author

youennf commented Jan 17, 2019

Taskcluster is failing for both Firefox and Chrome.
Looking at the logs, there does not seem to be anything suspicious related to the PR.
@foolip, any idea what this might be?

@annevk
Copy link
Member

annevk commented Jan 18, 2019

I think the problem is the change to common/get-host-info.sub.js which presumably triggers runs of lots of tests, causing the whole thing to time out.

@sideshowbarker sideshowbarker merged commit f15f912 into web-platform-tests:master Jan 18, 2019
@youennf youennf deleted the update-corp-tests branch January 18, 2019 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants