Skip to content

test(http2): expand http2_client.cpp coverage to 70%/50% (Round 3)#1131

Merged
kcenon merged 2 commits into
developfrom
test/issue-1119-http2-client-coverage-70
May 9, 2026
Merged

test(http2): expand http2_client.cpp coverage to 70%/50% (Round 3)#1131
kcenon merged 2 commits into
developfrom
test/issue-1119-http2-client-coverage-70

Conversation

@kcenon

@kcenon kcenon commented May 9, 2026

Copy link
Copy Markdown
Owner

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 gcov instrumentation still exceeds the 15s wait budget on shared CI runners, so the only viable strategy for raising http2_client.cpp coverage remains direct-dispatch via http2_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)

Criterion Coverage
Line coverage >=70% on http2_client.cpp New tests target the bulk of the success-path body across all 7 handlers; CI Coverage Analysis confirms
Branch coverage >=50% Each handler now has both not-found and success-path branches measured
All new tests pass on Ubuntu/macOS/Windows CI Local Apple Clang Debug: 38/38 PASS in 1 ms
Sanitizer builds (ASAN, TSAN, UBSAN) pass Tests are pure in-memory dispatch, no socket/thread state
Coverage workflow re-measurement attached Will be commented post-merge per established convention

New TEST cases (38 total in 3 suites)

Http2ClientDispatcherKnownStream (24 tests)

Test Branch driven
RstStreamOnKnownStreamClosesAndFulfillsPromise handle_rst_stream_frame close + promise.set_value (CANCEL)
RstStreamWithRefusedStreamErrorClosesKnownStream Same arm with REFUSED_STREAM error code
GoawayClosesStreamsAboveLastStreamId Loop body — stream id > last_stream_id closed
GoawayLeavesAlreadyClosedStreamUntouched Loop body — already-closed stream skipped
WindowUpdateOnKnownStreamIncrementsPerStreamWindow Per-stream window update (else branch)
WindowUpdateOnKnownStreamWithMaxIncrementSaturates Boundary value INT32_MAX
DataFrameOnKnownStreamBuffersBodyForNonStreaming Non-streaming buffer append
DataFrameOnKnownStreamWithEndStreamFulfillsPromise end_stream + status 200 success
DataFrameWithMalformedStatusFallsThroughCatchArm std::stoi catch arm
DataFrameInvokesOnDataCallbackForStreamingStream Streaming on_data callback
DataFrameWithEndStreamFiresOnCompleteForStreaming Streaming on_complete with status 404
DataFrameDrivesPerStreamWindowUpdateOnLargePayload Per-stream WINDOW_UPDATE branch
DataFrameDrivesConnectionLevelWindowUpdateOnLargePayload Connection-level WINDOW_UPDATE branch
DataFrameWithMaxStreamIdSucceeds RFC 7540 §5.1.1 boundary (2^31 - 1)
DataFrameWithLargePayloadFitsBuffer 16384-byte (max_frame_size) payload
HeadersFrameOnKnownStreamAccumulatesHeaders end_headers=false, end_stream=false (accumulate)
HeadersFrameWithEndStreamFulfillsPromiseWithStatus HPACK static-index 8 → :status: 200
HeadersFrameWithEndStreamAndError404FulfillsWithErrorStatus HPACK static-index 13 → :status: 404
HeadersFrameWithMalformedStatusFallsThroughCatchArm std::stoi catch arm via pre-staged headers
HeadersFrameInvokesOnHeadersForStreamingStream Streaming on_headers callback
HeadersFrameWithEndStreamInvokesOnCompleteForStreaming Streaming on_complete with status 500
ServerSettingsWithMaxHeaderTableSizeAppliesToEncoder header_table_size = UINT32_MAX
ServerSettingsWithEnablePushTrueSetsRemoteFlag enable_push = 1
ServerSettingsWithUnknownIdentifierIsIgnored Unknown identifier (no-op fall-through)

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 frame instance 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 Branch driven
BuildHeadersWithEmptyPathSubstitutesRoot http2_client.cpp:857 empty-path-to-/
BuildHeadersIncludesPseudoHeadersAndUserAgent Pseudo-header sequencing + user-agent prepend
BuildHeadersSkipsAdditionalPseudoHeaders Leading-colon name skip
AllocateStreamIdReturnsOddIdentifiers RFC 7540 §5.1.1 odd-id contract

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_test non-network suites (34 tests across 8 suites) continue to pass after the access-header extension.

Out of scope

  • No src/ changes; test-only PR per issue scope
  • Production header (src/internal/protocols/http2/http2_client.h) compiles byte-identical when BUILD_TESTS=OFF because all friend declarations remain inside the existing #if defined(NETWORK_ENABLE_TEST_INJECTION) block
  • The 14 connect-state public-API TEST_F (StartStream*, 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 scaffold

Notes

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 Analysis workflow against the PR #1117 baseline.

kcenon added 2 commits May 9, 2026 15:37
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.
Adds CHANGELOG entries for Issue #1119 (Round 3 of #953):
known-stream dispatcher branch tests for
src/protocols/http2/http2_client.cpp.

Updates both the root CHANGELOG.md (compatibility) and the
canonical docs/CHANGELOG.md (SSOT).
@github-actions

github-actions Bot commented May 9, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Metric Value
Line Coverage 69.8%
Branch Coverage 34.7%
Target 80% lines / 70% branches
Coverage Details

Full HTML report is available as a build artifact.

@kcenon kcenon merged commit 2cbda41 into develop May 9, 2026
9 checks passed
@kcenon kcenon deleted the test/issue-1119-http2-client-coverage-70 branch May 9, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant