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

HTTP/2fy request-upload test around streaming #28236

Merged
merged 2 commits into from
Apr 20, 2021

Conversation

yoichio
Copy link
Contributor

@yoichio yoichio commented Mar 25, 2021

That's because wpt h1 server doesn't support chunked encoding but h2 supports it natively.
Related thread: whatwg/fetch#966

@foolip
Copy link
Member

foolip commented Mar 25, 2021

The wpt-chrome-dev-* CI failures here were likely due to #19360. A workaround was landed in #28238, so I'm closing and reopening to re-trigger the CI checks.

@foolip foolip closed this Mar 25, 2021
@foolip foolip reopened this Mar 25, 2021
@annevk
Copy link
Member

annevk commented Mar 25, 2021

I guess we haven't quite resolved whether to support H/1, but ideally we have some kind of test for that as well.

@yoichio
Copy link
Contributor Author

yoichio commented Mar 29, 2021

I guess we haven't quite resolved whether to support H/1,

Yes but supporting H/2 has been decided. Should we also have a wpt that confirms the feature is not supported over H/1?

@yoichio
Copy link
Contributor Author

yoichio commented Mar 29, 2021

Undid confirming failure tests, which required wptserve/wptserve/server.py change. Will do later-on.

@yoichio
Copy link
Contributor Author

yoichio commented Apr 5, 2021

Ping?

Copy link
Contributor

@yutakahirano yutakahirano left a comment

Choose a reason for hiding this comment

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

LGTM.

@annevk
Copy link
Member

annevk commented Apr 14, 2021

@yoichio I'm not sure I understand your comment above. Has there been a decision to only support H/2 and up? If so, a test or a few tests for non-support for H/1 would be great.

(To be clear, that shouldn't be a blocker to landing this PR.)

@yoichio
Copy link
Contributor Author

yoichio commented Apr 14, 2021

Has there been a decision to only support H/2 and up?

No to "only" but yes to supporting at least H/2.

If so, a test or a few tests for non-support for H/1 would be great.

That's bit confusing. Should we put a wpt test that points an API doesn't work? This is different from how an API raises an exception.

@yoichio
Copy link
Contributor Author

yoichio commented Apr 14, 2021

I'm O.K to just put same tests both in request-upload.any.js and request-upload.h2.any.js

@annevk
Copy link
Member

annevk commented Apr 14, 2021

All I'm saying is that once we make a decision on what to do for H/1 we should have test coverage for that decision.

@yoichio
Copy link
Contributor Author

yoichio commented Apr 14, 2021

Sure.

@yoichio
Copy link
Contributor Author

yoichio commented Apr 19, 2021

Ping?

@annevk
Copy link
Member

annevk commented Apr 19, 2021

@yoichio you should feel free to merge this. Or do you want another review?

@yoichio
Copy link
Contributor Author

yoichio commented Apr 20, 2021

I don't have a write access to this repository.
Could you give me the right?

@yutakahirano yutakahirano merged commit 0573c1b into web-platform-tests:master Apr 20, 2021
@annevk
Copy link
Member

annevk commented Apr 20, 2021

(I've asked on W3C IRC #testing for someone to give you access.)

@yoichio yoichio deleted the request-upload-h2 branch April 21, 2021 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants