-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 tests for RequestInit.duplex #34496
Conversation
This is for whatwg/fetch#1457.
@annevk @lucacasonato PTAL |
async function assert_request(test, input, init) { | ||
assert_throws_js(TypeError, () => new Request(input, init), "new Request()"); | ||
await promise_rejects_js(test, TypeError, fetch(input, init), "fetch()"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove this?
(If there's a good reason to remove this, presumably we don't need to use promise_test
below?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If new Request(input, init)
throws, then fetch(input, init)
should be rejected. This is not at all streaming upload specific and I don't think we need to check that here.
Moreover, this is an HTTP/1.1 fetch, which is one more reason for fetch(input, init)
to be rejected, which makes this assertion more pointless.
I turned some promise_test
s into test
s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Require a new duplex member to be set to "half" when request's body is a stream. This makes the semantics clearer for the caller and would allow for a better default in the future. Tests: web-platform-tests/wpt#34496. Closes #1438.
This is for whatwg/fetch#1457.