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 Response test for propagating underlying ReadableStream error #9928

Merged

Conversation

Projects
None yet
5 participants
@AnthumChris
Copy link
Member

AnthumChris commented Mar 8, 2018

When a Response is constructed with ReadableStream, errors originating in that
ReadableStream are not propagated to the Response during a Promise rejection.
Tests throw errors in start() and pull() methods.

whatwg/fetch#676

add Response test for propagating underlying ReadableStream error
When a Response is constructed with ReadableStream, errors originating in that
ReadableStream are not propagated to the Response during a Promise rejection.
Tests throw errors in start() and pull() methods.

whatwg/fetch#676

@wpt-pr-bot wpt-pr-bot added the fetch label Mar 8, 2018

@wpt-pr-bot wpt-pr-bot requested review from annevk, jdm, mnot, youennf and yutakahirano Mar 8, 2018

@AnthumChris

This comment has been minimized.

Copy link
Member Author

AnthumChris commented Mar 8, 2018

This is my first experience writing WPT. Please let me know if this should be moved/renamed/changed

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 8, 2018

Build PASSED

Started: 2018-03-10 02:13:04
Finished: 2018-03-10 02:20:21

View more information about this build on:

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 9, 2018

Thanks for these! Much appreciated. Would you mind updating them to use the return promise_rejects pattern documented at http://web-platform-tests.org/writing-tests/testharness-api.html? That would make them a little easier to read. There's also no need to document the issue in the test itself. We can do that as part of the commit message.

@AnthumChris

This comment has been minimized.

Copy link
Member Author

AnthumChris commented Mar 10, 2018

@annevk All set. Thanks for the suggestion.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 11, 2018

@yutakahirano could you have a final look at these?

@annevk

annevk approved these changes Mar 11, 2018

@yutakahirano

This comment has been minimized.

Copy link
Contributor

yutakahirano commented Mar 12, 2018

lgtm

@annevk annevk merged commit e4f8dad into web-platform-tests:master Mar 12, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 12, 2018

@AnthumChris did you happen to file browser bugs on the browsers that fail these tests?

@AnthumChris

This comment has been minimized.

Copy link
Member Author

AnthumChris commented Mar 12, 2018

@annevk I have not filed browser bugs for these and could do that if needed. Please let me know if there's an official workflow to follow too

@AnthumChris AnthumChris deleted the AnthumChris:response-error-from-stream branch Mar 12, 2018

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 12, 2018

@AnthumChris there's no templates at the moment though maybe I should create some guidance somewhere. Basically file a bug and say http://w3c-test.org/fetch/api/response/response-error-from-stream.html fails. I think at this point only Chrome and Safari need a bug because the other browsers haven't shipped ReadableStream yet.

@AnthumChris

This comment has been minimized.

Copy link
Member Author

AnthumChris commented Mar 12, 2018

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 12, 2018

Great, thanks! For Safari you can file public bugs over at https://bugs.webkit.org/enter_bug.cgi?product=WebKit&component=HTML%20DOM btw.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 12, 2018

It seems we have advice for this over at https://github.com/whatwg/meta/blob/master/MAINTAINERS.md#handling-pull-requests. I'll try to remember linking that going forward.

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.