Fixed todos in test h2 client #241
Conversation
TingDaoK
left a comment
TingDaoK
left a comment
There was a problem hiding this comment.
Github is just too clever, those I marked as "old test" are not new code.
| } | ||
|
|
||
| /* Receiving malformed headers should result in a "Stream Error", not a "Connection Error". */ | ||
| TEST_CASE(h2_client_stream_err_malformed_header) { |
There was a problem hiding this comment.
old test, you can skip when you review this pr.
| return s_tester_clean_up(); | ||
| } | ||
|
|
||
| TEST_CASE(h2_client_conn_err_stream_frames_received_for_idle_stream) { |
There was a problem hiding this comment.
old test, you can skip when you review
|
|
||
| /* Peer may have sent certain frames (WINDOW_UPDATE and RST_STREAM) before realizing | ||
| * that we have closed the stream. These frames should be ignored. */ | ||
| TEST_CASE(h2_client_stream_ignores_some_frames_received_soon_after_closing) { |
There was a problem hiding this comment.
old test, you can skip when you review
| /* A server MAY request that the client abort transmission of a request without error by sending a | ||
| * RST_STREAM with an error code of NO_ERROR after sending a complete response. */ | ||
| TEST_CASE(h2_client_stream_receive_rst_stream_after_complete_response_ok) { | ||
| TEST_CASE(h2_client_stream_receive_end_stream_and_rst_before_done_sending) { |
There was a problem hiding this comment.
Actually an old test, just rename it, and made one more check, you can skip when you review.
TingDaoK
changed the title
| #TODO add_test_case(h2_client_stream_err_receive_info_headers_after_main) | ||
| #TODO add_test_case(h2_client_stream_receive_trailing_headers) | ||
| #TODO add_test_case(h2_client_stream_err_receive_trailing_before_main) | ||
| add_test_case(h2_client_conn_err_stream_frames_received_after_rst_stream_received) |
There was a problem hiding this comment.
Oh, should this be a stream error instead of connection error? "An endpoint that receives any frame other than PRIORITY after receiving a RST_STREAM MUST treat that as a stream error (Section 5.4.2) of type STREAM_CLOSED" stream_closed_state
Updated here, upcoming PR
There was a problem hiding this comment.
Good catch, you're right!
Guess this should be something like
h2_client_stream_err_frames_received_after_rst_stream_received
| #TODO add_test_case(h2_client_stream_err_receive_info_headers_after_main) | ||
| #TODO add_test_case(h2_client_stream_receive_trailing_headers) | ||
| #TODO add_test_case(h2_client_stream_err_receive_trailing_before_main) | ||
| add_test_case(h2_client_conn_err_stream_frames_received_after_rst_stream_received) |
There was a problem hiding this comment.
Good catch, you're right!
Guess this should be something like
h2_client_stream_err_frames_received_after_rst_stream_received
| } | ||
|
|
||
| /* Receiving malformed headers should result in a "Stream Error", not a "Connection Error". */ | ||
| TEST_CASE(h2_client_stream_err_malformed_header) { |
| /* a stream error should not affect the connection */ | ||
| ASSERT_TRUE(aws_http_connection_is_open(s_tester.connection)); | ||
|
|
||
| /* validate that stream sent RST_STREAM */ |
Remaining TODOs in client_test:
h2_client_conn_err_stream_frames_received_long_after_closing: upcoming PR for removing entries from closed_streams_where_frames_might_trickle_in
h2_client_stream/conn_err_flow_control: After we get APIs for user to change setting.
h2_client_send_goaway_with_push_promises: After we fully support PUSH_PROMISE
2 participants
Issue #, if available:
Description of changes:
Remaining TODOs in client_test:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.