test(grpc): drive connected-state paths via mock_h2_server_peer (Part of #1063)#1077
Merged
Merged
Conversation
… 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.
5 tasks
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.
What
Adds 12 new
TEST_Fcases under the existingGrpcClientHermeticTransportTestfixture intests/unit/grpc_client_branch_test.cpp(+317 LOC, no new test files) that compose the Phase 2Amock_h2_server_peershipped in PR #1075 to bringgrpc_clientinto a fully SETTINGS-exchanged state, then exercise public methods that previously early-returned viais_connected()in the existing disconnected-state suites.Newly reached paths in
src/protocols/grpc/client.cppis_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)connect()short-circuit through thealready_connectedbranch (lines 1035-1042)target()getter post-handshake (lines 1141-1144)call_raw()connected path: header build,grpc-timeoutheader formatting, metadata copy loop,http2_client::postforwarding, 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_streamforwarding,shared_*_holderallocation,ok()unique_ptr return (lines 1352-1611)disconnect()second-call idempotent path after a real handshake (lines 1107-1118)Why
Part of #1063andPart of #953(epic: 40% → 80% coverage).grpc_client.cppline coverage stood at 22.6% line / 9.5% branch as of the 2026-04-26 lcov run (workflow 24947193873). PR #1069 added 36 disconnected-stateTEST_F/TESTcases, but the remaining gap was concentrated in the post-handshake code path that needed an in-process HTTP/2 peer to driveis_connected() == truein CI.Because
grpc_client::implinstantiates an internalhttp2::http2_clientand delegatesconnect()to itsconnect(host, port), the Phase 2Amock_h2_server_peershipped in PR #1075 is sufficient to pushgrpc_clientpast the connection gate without requiring a separatemock_grpc_server_peerto be built first. This PR consumes that infrastructure to actually expandgrpc_client.cppcoverage.The acceptance criteria of
>= 80% line / >= 70% branchis not reached by this PR alone — it cannot be, until Phase 2A.2 of #1074 landsmock_h2_server_peerHEADERS+DATA reply support. Without server replies, theserver_stream_reader_impl::on_data/on_headers/on_completecallback wiring, theclient_stream_writer_implround-trip, thebidi_stream_implread/write/finish round-trip, thecall_rawhappy-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 usesPart of #1063, notCloses.Where
tests/unit/grpc_client_branch_test.cppmake_connected_grpc_client(peer, default_timeout)plus 12 newTEST_Fcases 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 attests/CMakeLists.txt:4916-4918linked tonetwork::test_support, which already statically linksmock_h2_server_peer.cppfrom PR #1075).How
Each new
TEST_F:mock_h2_server_peer peer(io())on the fixture's sharedio_context.make_connected_grpc_client(peer, default_timeout)which spawns a worker thread that runsclient->connect()synchronously against127.0.0.1:peer.port().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 innerhttp2_clienthas flippedis_connected_to true andgrpc_client::is_connected()returns true.Result<T>outcome.disconnect()which tears down the inner http2 client (emitting GOAWAY, drained by the peer), then joins the connector thread.Tests reaching the
call_rawpost-timeout branch use a 150 msdefault_timeoutto 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 tohttp2_client::start_streamwhich 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.ymlworkflow is the authoritative verification surface (PR #1071 / #1075 / #1076 precedent). Expected outcome:grpc_client.cppline coverage strictly increases above the 22.6% baseline.Part of, notCloses); the issue's>= 80% line / >= 70% branchacceptance criteria becomes reachable only after Phase 2A.2 lands and a final follow-up test PR drives the success paths.Part of #1063
Part of #953