infra(test): add mock_h2_server_peer for HTTP/2 loopback (Phase 2A of #1074)#1075
Merged
Merged
Conversation
Server-side HTTP/2 framing peer composed on top of tls_loopback_listener. A dedicated worker thread reads the 24-byte connection preface, sends an empty server SETTINGS frame, reads the client SETTINGS frame, and sends a SETTINGS-ACK. After the exchange completes (settings_exchanged() flag), the worker drains any subsequent client frames (e.g. the GOAWAY emitted by disconnect()) until EOF. Reuses settings_frame and frame_header::parse from the production code, so no manual byte-level encoding lives in tests/support/. Hermetic: the listener binds 127.0.0.1:0 and certs are regenerated per construction. Phase 2A of #1074. Phases 2A.2 (HEADERS+DATA reply), 2B (gRPC framing), 2C (QUIC peer), 2D (friend injection points), and 2E (frame_injector) remain as follow-up work. Part of #1074 Part of #953
Adds ConnectCompletesSettingsExchangeWithMockPeer to the existing Http2ClientHermeticTransportTest fixture. The test composes the new mock_h2_server_peer with the http2_client and asserts: - peer.settings_exchanged() reaches true within the 3s budget - peer.io_failed() stays false (preface, framing, and ACK all valid) - client.is_connected() reports true - client.get_settings() returns sane values (initial_window_size > 0) - client.disconnect() drives the GOAWAY-emit path and reports is_connected() == false afterwards Prior to mock_h2_server_peer this code path was reachable only against the public-internet DISABLED_ConnectToHttpbin test, which the coverage workflow does not run. The new test gives http2_client.cpp line coverage a strictly positive delta against a hermetic peer. Phase 2A of #1074. Part of #1074 Part of #953
5 tasks
The header doc and README promised that make_self_signed_ssl_context is 'configured for TLS 1.2+ with HTTP/2 ALPN h2 and HTTP/1.1 fallback'. The implementation only loaded the cert and key; the ALPN selection callback was never installed, so OpenSSL ignored the client ALPN extension and the negotiated application protocol came back empty. http2_client::connect strictly verifies that the negotiated ALPN is exactly 'h2' (see ALPN check after SSL_get0_alpn_selected in src/protocols/http2/http2_client.cpp). Without the server-side callback the verification fails and connect returns an error before the post-handshake code paths are reached, defeating the purpose of the mock_h2_server_peer added in the same PR. This commit installs an SSL_CTX_set_alpn_select_cb that prefers h2 and falls back to http/1.1, matching the documented behavior. No other behavior changes; existing tests that only assert listener.accepted() remain green because they did not depend on the ALPN result. Part of #1074 Part of #953
Contributor
Coverage Report
Coverage DetailsFull HTML report is available as a build artifact. |
This was referenced Apr 28, 2026
kcenon
added a commit
that referenced
this pull request
Apr 28, 2026
…t of #1062) (#1076) * test(http2): drive connected-state paths via mock_h2_server_peer Adds 12 new TEST_F cases under Http2ClientHermeticTransportTest in tests/unit/http2_client_branch_test.cpp that compose the Phase 2A mock_h2_server_peer (PR #1075) to bring http2_client into a fully SETTINGS-exchanged state, then exercise public methods that early- returned via is_connected() in the existing disconnected-state suite. Newly reached paths in src/protocols/http2/http2_client.cpp: - connect() second-call short-circuit through already_exists branch - send_request() / get() / post() create_stream + build_headers + encoder_.encode + send_frame on the connected path, terminating in the future timeout branch (peer sends no HEADERS+DATA in 2A) - start_stream() / write_stream() / cancel_stream() / close_stream_writer() connected paths plus their not_found error branches with an unallocated stream id - close_stream_writer() idempotent early-return on a half_closed_local stream - set_settings() post-handshake call site updating encoder / decoder table sizes - disconnect() GOAWAY emit + stop_io path followed by the second-call early-return on a torn-down handshaken client HEADERS+DATA reply paths remain unreachable until Phase 2A.2 of #1074 lands; this PR therefore uses Part of #1062, not Closes. Part of #1062 Part of #953 * test(http2): qualify http_header in new lambda signatures The four lambdas added in the prior commit used the bare identifier http_header in their parameter list, which macOS clang flags as 'use of undeclared identifier' because the namespace alias 'namespace http2 = kcenon::network::protocols::http2;' inside the test file does not implicitly bring symbols of that namespace into unqualified lookup at the function signature site. Switch all four occurrences to http2::http_header to match the existing qualified usage at line 60 of the file.
This was referenced Apr 28, 2026
kcenon
added a commit
that referenced
this pull request
Apr 28, 2026
… of #1063) (#1077) 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.
This was referenced Apr 28, 2026
Merged
kcenon
added a commit
that referenced
this pull request
May 5, 2026
…2C of #1074) (#1103) Add server-side QUIC Initial echo peer to tests/support/. The peer receives one client Initial datagram, derives QUIC v1 initial keys from the original DCID (RFC 9001 Section 5.2), and replies with a header-protected server Initial carrying a stub crypto_frame (type 0x06). This makes quic_socket::process_crypto_frame reachable from a hermetic test for the first time, unblocking coverage expansion under #1062 / #1065. Phase 2A (#1075) and 2B (#1102) shipped HTTP/2 and gRPC peers; this follows the same RAII shape: shared io_context, dedicated worker thread, atomic state flags polled via wait_for. Phase 2D (friend- test injection) and 2E (frame_injector) remain open under #1074. Files: - tests/support/mock_quic_peer_loop.{h,cpp} (new) - tests/support/CMakeLists.txt (+1) - tests/support/README.md (+1) - tests/unit/quic_socket_branch_test.cpp (+46) -- one demo TEST_F Part of #1074
Merged
3 tasks
kcenon
added a commit
that referenced
this pull request
May 6, 2026
…#1104) Add friend-test injection so unit tests can drive two private server-side methods previously unreachable from public APIs: - messaging_quic_server::handle_packet (src/internal/experimental/quic_server.h:432) - messaging_ws_server::handle_new_connection (src/internal/http/websocket_server.h:415) Mechanism: new compile definition NETWORK_ENABLE_TEST_INJECTION gated behind BUILD_TESTS in cmake/network_system_targets.cmake. The two production headers gain a forward declaration block and a single 'friend class' line, all wrapped in '#if defined(NETWORK_ENABLE_TEST_INJECTION)'. When BUILD_TESTS=OFF the production binary is byte-identical to the previous develop tip. Probe types live entirely in tests/support/: - network_test_friends.h forward-declares the probes - quic_server_probe.{h,cpp} forwards to handle_packet - ws_server_probe.{h,cpp} forwards to handle_new_connection Demo tests verify end-to-end wiring: - QuicServerProbeTest.HandlePacket{Empty,Garbage}BufferDoesNotCrash - WsServerProbeTest.HandleNewConnectionEarlyReturnNoSessionManager - WsServerProbeTest.HandleNewConnectionEmptySocketEarlyReturn Per #1074 scope: no behavioral changes to src/. Phases 2A (#1075), 2A.2 (#1082), 2B (#1102), 2C (#1103) merged; 2E remains. Part of #1074
This was referenced May 6, 2026
Merged
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
Summary
Adds
mock_h2_server_peertotests/support/and one demoTEST_Fthat driveshttp2_client::connect()past the SETTINGS-exchange gate, reaching post-connect public methods (get_settings,disconnect) inside a hermetic CI environment.Change Type
Affected Components
tests/support/mock_h2_server_peer.h(new) -- peer class declarationtests/support/mock_h2_server_peer.cpp(new) -- worker-thread implementationtests/support/CMakeLists.txt-- add new source tonetwork_test_supporttests/support/README.md-- document the peer + Phase-2 progress tabletests/unit/http2_client_branch_test.cpp-- append oneTEST_F(~54 LOC)Why
Part of #1074(Phase 2 of #1060). The six in-flight coverage sub-issues #1062, #1063, #1064, #1065, #1066, #1067 (children of epic #953) all share one root cause: most uncovered code in their target files sits behind a connected protocol peer that the existingtls_loopback_listenerdoes not provide. PRs #1068-#1073 honestly usedPart ofrather thanClosesbecause the >=80% line / >=70% branch acceptance criteria are unreachable without the framing this issue ships.This PR is the first phase (2A) of #1074. Subsequent phases (2A.2 for HEADERS+DATA reply, 2B for gRPC, 2C for QUIC, 2D for friend injection points, 2E for frame_injector) ship as separate PRs.
Where
tests/support/mock_h2_server_peer.htests/support/mock_h2_server_peer.cpptests/support/CMakeLists.txttests/support/README.mdtests/unit/http2_client_branch_test.cppNo
src/changes. No public-API additions. No build-system changes beyond adding the new translation unit to the support library.How
Implementation Approach
mock_h2_server_peercomposes the existingtls_loopback_listener(RSA-2048 self-signed cert, ALPN "h2", TCP accept, TLS handshake) and adds a worker thread that performs the application-layer exchange:PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n).Frame encoding/decoding reuses production
settings_frameandframe_header::parse-- no manual byte-level work in the test fixture.Demo TEST_F
Http2ClientHermeticTransportTest, ConnectCompletesSettingsExchangeWithMockPeerasserts that with the new peer:peer.settings_exchanged()reaches true (vs the prior tls-listener-only test that only reachedlistener.accepted()and stalled there)peer.io_failed()stays falseclient.is_connected()is trueclient.get_settings()returns sane values (initial_window_size > 0, max_frame_size > 0)client.disconnect()drives the GOAWAY emit pathclient.is_connected()is false after disconnectCoverage Scope (Honest Assessment)
Local toolchain (cmake/g++/ninja) is not available in this dev environment -- relying on CI for build, sanitizer, and coverage verification (PR #1071 precedent). The
coverage.ymlworkflow should report a strictly positive line-coverage delta onsrc/protocols/http2/http2_client.cppafter merge.Issue #1074 stays open (
Part of, notCloses); the remaining phases (2A.2, 2B, 2C, 2D, 2E) are tracked there. Issues #1062-#1067 also stay open until each relevant phase lands and a follow-up PR can push their target files past the >=80% / >=70% threshold.Test Plan for Reviewers
mock_h2_server_peer.{h,cpp}for RFC 7540 frame correctness.tests/support/CMakeLists.txtcorrectly adds the new source tonetwork_test_support.coverage.ymlPR comment -- line coverage onhttp2_client.cppshould strictly increase.Breaking Changes
None -- test-only addition.
Rollback Plan
Revert the two commits; no schema, ABI, or build surface affected.
Part of #1074
Part of #953