Is WebSocketHttp2HandshakeStream tested well? |
||
Issue descriptionWebSocketBasicHandshakeStream is tested in websocket_stream_test.cc as WebSocket.*StreamCreate.*Test. Some of them such as WebSocketMultiProtocolStreamCreateTest cover both BASIC_HANDSHAKE_STREAM and HTTP2_HANDSHAKE_STREAM. Others such as WebSocketStreamCreateTest don't. Is WebSocketHttp2HandshakeStream tested well? Is it tested in other places?
,
Apr 13 2018
I'm adding a WebRequest API support for WebSocket with Network Service (WIP: https://chromium-review.googlesource.com/c/chromium/src/+/1009266), and I found I cannot test part of the features because WebSocketStreamCreateTest::OnHeadersReceived I'm introducing in the CL uses CreateAndConnectCustomResponse.
,
Apr 13 2018
,
Apr 13 2018
The semantics of the response headers is very different for HTTP/1 and HTTP/2 based WebSockets. For example, 101 and 200 status codes mean success, respectively. There is also a difference is some other response headers. Not to mention that the syntax of response headers is different for the different HTTP versions. CreateAndConnectCustomResponse is only used for HTTP/1-specific tests for this reason. For this exact reason, are you sure it is a good idea to expose response headers to developers? They would then need to deal with different semantics depending on HTTP protocol version, and I foresee a lot of applications hardwired not to support HTTP/2. Otherwise, for the OnHeadersReceived test, you can call SetHttp2ResponseStatus(404) before calling CreateAndConnectStandard() to generate mock read data with 404 error.
,
Apr 17 2018
Fair enough. I don't know how they should be exposed either. |
||
►
Sign in to add a comment |
||
Comment 1 by b...@chromium.org
, Apr 13 2018