test(http2): expand http2_client.cpp coverage to 70%/50% (Round 3)#1131
Merged
Conversation
Issue #1119 (Round 3 of #953): expand http2_client.cpp coverage to >=70% line / >=50% branch. Round 6 (#1115 / PR f3c289f) and Round 2 (#1117 / PR 7af3bc5) raised http2_client.cpp coverage by direct-dispatching server-originated frames through http2_client_test_access::process_frame. Both rounds targeted handlers whose entry arms are reachable without an entry in streams_; anything that runs after the stream lookup succeeds remained unmeasured. This commit extends the friend access class with stream-seeding helpers (seed_stream, set_stream_callbacks, stream_state_of, stream_window_size_of, append_response_header, response_body_of, build_headers, allocate_stream_id, next_stream_id) and adds 38 TEST cases driving the known-stream success-path branches: - handle_rst_stream_frame: close-and-fulfill arm with both CANCEL and REFUSED_STREAM error codes - handle_goaway_frame: populated streams_ loop body covering streams above last_stream_id (closed and promise.set_value), at-or-below (kept open), and already-closed (skipped) - handle_window_update_frame: known-stream non-zero stream_id branch including INT32_MAX boundary - handle_data_frame: non-streaming buffer-append, end_stream success with status-code extraction (200), malformed-status std::stoi catch arm, streaming on_data fan-out, streaming end_stream on_complete, per-stream WINDOW_UPDATE branch, connection-level WINDOW_UPDATE branch, max-uint31 stream id, and 16384-byte payload round-trip - handle_headers_frame: accumulate (end_headers without end_stream), end_stream success with HPACK static-index decode for status 200 and 404, malformed-status catch arm, streaming on_headers, streaming end_stream on_complete with status 500 - process_frame: dynamic_cast-fails / break / fall-through arm for all seven dispatched frame types, plus default arm for priority, push_promise, and continuation - Private helpers: build_headers empty-path-to-/ substitution, pseudo-header sequencing, additional-header pseudo-header skip; allocate_stream_id RFC 7540 5.1.1 odd-id contract Production header compiles byte-identical when BUILD_TESTS=OFF; all friend declarations remain inside the existing NETWORK_ENABLE_TEST_INJECTION gate. No new friend declarations needed because the new accessors are member functions of the existing http2_client_test_access class. Local verification: 38/38 PASSED in 1 ms wall-time on Apple Clang Debug build; existing network_http2_client_branch_test non-network suites (34 tests across 8 suites) continue to pass. Part of #953 coverage expansion.
Contributor
Coverage Report
Coverage DetailsFull HTML report is available as a build artifact. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1119
Summary
Round 3 of #953: extends Round 6 (#1115 / PR f3c289f) and Round 2 (#1117 / PR 7af3bc5) direct-dispatch coverage with 38 new TEST cases that drive the known-stream success-path branches the prior rounds could not reach.
The SETTINGS handshake under
gcovinstrumentation still exceeds the 15s wait budget on shared CI runners, so the only viable strategy for raisinghttp2_client.cppcoverage remains direct-dispatch viahttp2_client_test_access::process_frame(the friend gate proven in Round 6). Extending the access class with stream-seeding helpers (seed_stream,set_stream_callbacks,stream_state_of,stream_window_size_of,append_response_header,response_body_of,build_headers,allocate_stream_id,next_stream_id) lets the dispatcher tests drive the success-path body of every handler, not just the entry / not-found arms.Acceptance criteria mapping (#1119)
New TEST cases (38 total in 3 suites)
Http2ClientDispatcherKnownStream(24 tests)Http2ClientDispatcherDefaultArm(10 tests)For each of the seven dispatched frame types (settings, headers, data, rst_stream, goaway, window_update, ping), drive the dynamic_cast-fails / break / fall-through arm by passing a base-class
frameinstance whose header().type matches the case but runtime type does not (this branch is unreachable from the wire path because frame::parse always returns a matching concrete instance). Plus the default arm for priority, push_promise, and continuation types (no handler implemented).Http2ClientPrivateHelpers(4 tests)Test plan
cmake --build build --target network_http2_client_dispatcher_branch_test -j 4 ./build/bin/network_http2_client_dispatcher_branch_test # Expect: [ PASSED ] 38 tests.Local verification: 38/38 PASS in 1 ms wall-time on Apple Clang Debug build. Existing
network_http2_client_branch_testnon-network suites (34 tests across 8 suites) continue to pass after the access-header extension.Out of scope
src/changes; test-only PR per issue scopesrc/internal/protocols/http2/http2_client.h) compiles byte-identical whenBUILD_TESTS=OFFbecause all friend declarations remain inside the existing#if defined(NETWORK_ENABLE_TEST_INJECTION)blockStartStream*,WriteStream*,CancelStream*, etc.) genuinely require the connected-state path and remain on the PR fix(test): apply NETWORK_COVERAGE_TIMEOUT_MULTIPLIER at explicit wait_for call sites (#1112) #1114 multiplier scaffoldNotes
Chains from PR #1115 Round 6 (friend-injected dispatcher pivot) → PR #1117 Round 2 (error-branch dispatcher TEST_F) → this PR Round 3 (known-stream success-path branches). Acceptance gate confirmation will rely on the post-merge
Coverage Analysisworkflow against the PR #1117 baseline.