Skip to content

test(grpc): drive connected-state paths via mock_h2_server_peer (Part of #1063)#1077

Merged
kcenon merged 1 commit into
developfrom
test/issue-1063-grpc-client-coverage-phase2a
Apr 28, 2026
Merged

test(grpc): drive connected-state paths via mock_h2_server_peer (Part of #1063)#1077
kcenon merged 1 commit into
developfrom
test/issue-1063-grpc-client-coverage-phase2a

Conversation

@kcenon

@kcenon kcenon commented Apr 28, 2026

Copy link
Copy Markdown
Owner

What

Adds 12 new TEST_F cases under the existing GrpcClientHermeticTransportTest fixture in tests/unit/grpc_client_branch_test.cpp (+317 LOC, no new test files) that compose the Phase 2A mock_h2_server_peer shipped in PR #1075 to bring grpc_client into a fully SETTINGS-exchanged state, then exercise public methods that previously early-returned via is_connected() in the existing disconnected-state suites.

Newly reached paths in src/protocols/grpc/client.cpp

  • is_connected() compound condition after the inner http2 SETTINGS exchange completes (lines 1120-1122)
  • wait_for_connected() polling loop hitting the success branch (lines 1125-1139)
  • second connect() short-circuit through the already_connected branch (lines 1035-1042)
  • target() getter post-handshake (lines 1141-1144)
  • call_raw() connected path: header build, grpc-timeout header formatting, metadata copy loop, http2_client::post forwarding, future-timeout error branch (lines 1146-1245)
  • call_raw() post-connect deadline-exceeded branch (lines 1188-1202)
  • call_raw_async() connected submit_task path with callback delivery (lines 1340-1350)
  • server_stream_raw() / client_stream_raw() / bidi_stream_raw() connected paths: header build, start_stream forwarding, shared_*_holder allocation, ok() unique_ptr return (lines 1352-1611)
  • disconnect() second-call idempotent path after a real handshake (lines 1107-1118)

Why

Part of #1063 and Part of #953 (epic: 40% → 80% coverage).

grpc_client.cpp line coverage stood at 22.6% line / 9.5% branch as of the 2026-04-26 lcov run (workflow 24947193873). PR #1069 added 36 disconnected-state TEST_F / TEST cases, but the remaining gap was concentrated in the post-handshake code path that needed an in-process HTTP/2 peer to drive is_connected() == true in CI.

Because grpc_client::impl instantiates an internal http2::http2_client and delegates connect() to its connect(host, port), the Phase 2A mock_h2_server_peer shipped in PR #1075 is sufficient to push grpc_client past the connection gate without requiring a separate mock_grpc_server_peer to be built first. This PR consumes that infrastructure to actually expand grpc_client.cpp coverage.

The acceptance criteria of >= 80% line / >= 70% branch is not reached by this PR alone — it cannot be, until Phase 2A.2 of #1074 lands mock_h2_server_peer HEADERS+DATA reply support. Without server replies, the server_stream_reader_impl::on_data / on_headers / on_complete callback wiring, the client_stream_writer_impl round-trip, the bidi_stream_impl read/write/finish round-trip, the call_raw happy-path completion (HTTP/2 200 → trailer parse → status code extraction → ok(grpc_message)), and the official-grpcpp wrapper paths all remain unreachable from a hermetic test. This PR therefore uses Part of #1063, not Closes.

Where

Path Change
tests/unit/grpc_client_branch_test.cpp +317 LOC: anonymous-namespace helper make_connected_grpc_client(peer, default_timeout) plus 12 new TEST_F cases appended after the existing Phase 2A demo. One new include: mock_h2_server_peer.h.

No changes under src/, no new test files, no CMake change (the file is already at tests/CMakeLists.txt:4916-4918 linked to network::test_support, which already statically links mock_h2_server_peer.cpp from PR #1075).

How

Each new TEST_F:

  1. Constructs a mock_h2_server_peer peer(io()) on the fixture's shared io_context.
  2. Calls make_connected_grpc_client(peer, default_timeout) which spawns a worker thread that runs client->connect() synchronously against 127.0.0.1:peer.port().
  3. Waits on wait_for([&]() { return peer.settings_exchanged(); }, std::chrono::seconds(3)) — by which time the peer has read the preface, sent server SETTINGS, read client SETTINGS, and sent SETTINGS-ACK; the inner http2_client has flipped is_connected_ to true and grpc_client::is_connected() returns true.
  4. Drives the targeted post-connect public method, asserting the expected Result<T> outcome.
  5. Calls disconnect() which tears down the inner http2 client (emitting GOAWAY, drained by the peer), then joins the connector thread.

Tests reaching the call_raw post-timeout branch use a 150 ms default_timeout to bound execution time. Streaming methods use the default 2 s timeout for the connect path; their post-connect calls return immediately since they only forward to http2_client::start_stream which allocates a stream id without waiting on a future.

Coverage Scope (Honest Assessment)

Local C++ toolchain (cmake/g++/ninja) is not available in this dev environment; the coverage.yml workflow is the authoritative verification surface (PR #1071 / #1075 / #1076 precedent). Expected outcome:

Part of #1063
Part of #953

… of #1063)

Adds 12 TEST_F cases under the existing GrpcClientHermeticTransportTest
fixture that compose mock_h2_server_peer (PR #1075) to bring grpc_client
into a fully SETTINGS-exchanged state, then drive previously-unreachable
post-connect public methods.

Newly reached paths in src/protocols/grpc/client.cpp:
  - is_connected() compound condition after the http2 SETTINGS exchange
  - wait_for_connected() polling loop hitting the success branch
  - second connect() short-circuit through the already_connected branch
  - target() getter post-handshake
  - call_raw() connected path: header build, grpc-timeout header,
    metadata loop, http2_client::post forwarding, future timeout error
  - call_raw() post-connect deadline-exceeded branch
  - call_raw_async() connected submit_task + callback delivery
  - server_stream_raw / client_stream_raw / bidi_stream_raw connected
    paths: header build, start_stream forwarding, shared_*_holder
    allocation, ok() unique_ptr return
  - disconnect() second-call idempotent path after a real handshake

HEADERS+DATA reply paths still depend on Phase 2A.2 of #1074, so this
PR uses Part of #1063 / Part of #953, not Closes.
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Metric Value
Line Coverage 66.9%
Branch Coverage 33.1%
Target 80% lines / 70% branches
Coverage Details

Full HTML report is available as a build artifact.

@kcenon kcenon merged commit f672048 into develop Apr 28, 2026
9 checks passed
@kcenon kcenon deleted the test/issue-1063-grpc-client-coverage-phase2a branch April 28, 2026 21:56
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