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

Add some tests for no-cors and navigate requests for Blob URLs. #9320

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@mkruisselbrink
Copy link
Contributor

mkruisselbrink commented Jan 31, 2018

Verifying behavior as discussed in whatwg/fetch#666. Don't land yet, since these tests currently don't match the spec.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Jan 31, 2018

Build PASSED

Started: 2018-02-08 21:47:30
Finished: 2018-02-08 21:58:37

Failing Jobs

  • chrome:dev

Unstable Results

Browser: "Chrome Dev" (failures allowed)

View in: WPT PR Status | TravisCI

Test Subtest Results Messages
/FileAPI/url/fetch-navigate.window.html   OK: 10
  Can not load a cross-origin blob URL in an iframe. PASS: 10
  Can not load a cross-origin blob URL in a no-opener window. PASS: 4
FAIL: 6
assert_unreached: Loading should have failed Reached unreachable code
  Can not load a cross-origin blob URL in a window. PASS: 10
  Can not navigate an iframe to a cross-origin blob URL. PASS: 10
  Can not navigate a window to a cross-origin blob URL. PASS: 10
@mkruisselbrink

This comment has been minimized.

Copy link
Contributor Author

mkruisselbrink commented Jan 31, 2018

Any suggestions on how to make these tests less flaky are welcome... not sure how to reliable detect cross-origin loading failure, other than the timeout on loading success I'm currently doing (and as it shows, apparently a 1 second timeout isn't long enough to not be flaky...)

@annevk

annevk approved these changes Feb 1, 2018

Copy link
Member

annevk left a comment

Tests look great, though I wonder if they should be in the /fetch directory somewhere instead, given that we'll probably define it there?

I'm not sure how to deal with the flaky behavior, I guess you can expand the timeout to 2s and see if that goes better?

@mkruisselbrink

This comment has been minimized.

Copy link
Contributor Author

mkruisselbrink commented Feb 8, 2018

Tests look great, though I wonder if they should be in the /fetch directory somewhere instead, given that we'll probably define it there?

I'm not sure. We could indeed try to split the various blob URL loading related tests between FileAPI, fetch, url and XHR directories, but it isn't always clear where a test belongs, so it seemed easiest to just keep all the blob URL related tests together. But yeah, you're right that the spec changes to make these tests match the spec would be in the fetch spec.

I'm not sure how to deal with the flaky behavior, I guess you can expand the timeout to 2s and see if that goes better?

I increased the timeout and changed the tests to async_test rather than promise_test to at least let the timeouts of all the tests overlap... Lets see how that goes.

@mkruisselbrink

This comment has been minimized.

Copy link
Contributor Author

mkruisselbrink commented Feb 8, 2018

Interestingly that's still flaky in chrome... I wonder if maybe our implementation is just flaky, although I haven't been able to reproduce that test passing locally...

@youennf

This comment has been minimized.

Copy link
Contributor

youennf commented Feb 8, 2018

I usually prefer promise_test over async_test as they are run sequentially one after the other.
That makes the tests much more predictable. WebKit CI is checking JS console log messages for instance and the order might make tests fail.

I don't have a good answer for the iframe loading issue.
Maybe there could be some internals/webdriver API that could help there.

As of where it should be, fetch API already has some blob tests, as well as service worker.
I would tend to put these in fetch folder

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Sep 3, 2018

@mkruisselbrink did you ever get to the bottom of the flakiness?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.