Skip to content

infra(test): add mock_h2_server_peer for HTTP/2 loopback (Phase 2A of #1074)#1075

Merged
kcenon merged 3 commits into
developfrom
feat/issue-1074-mock-h2-server-peer-phase-2a
Apr 28, 2026
Merged

infra(test): add mock_h2_server_peer for HTTP/2 loopback (Phase 2A of #1074)#1075
kcenon merged 3 commits into
developfrom
feat/issue-1074-mock-h2-server-peer-phase-2a

Conversation

@kcenon

@kcenon kcenon commented Apr 28, 2026

Copy link
Copy Markdown
Owner

What

Summary

Adds mock_h2_server_peer to tests/support/ and one demo TEST_F that drives http2_client::connect() past the SETTINGS-exchange gate, reaching post-connect public methods (get_settings, disconnect) inside a hermetic CI environment.

Change Type

  • Test infrastructure (no production behavior change)

Affected Components

  • tests/support/mock_h2_server_peer.h (new) -- peer class declaration
  • tests/support/mock_h2_server_peer.cpp (new) -- worker-thread implementation
  • tests/support/CMakeLists.txt -- add new source to network_test_support
  • tests/support/README.md -- document the peer + Phase-2 progress table
  • tests/unit/http2_client_branch_test.cpp -- append one TEST_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 existing tls_loopback_listener does not provide. PRs #1068-#1073 honestly used Part of rather than Closes because 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

File Action
tests/support/mock_h2_server_peer.h new (~155 LOC)
tests/support/mock_h2_server_peer.cpp new (~190 LOC)
tests/support/CMakeLists.txt modify (+1)
tests/support/README.md modify (+51)
tests/unit/http2_client_branch_test.cpp modify (+54)

No 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_peer composes the existing tls_loopback_listener (RSA-2048 self-signed cert, ALPN "h2", TCP accept, TLS handshake) and adds a worker thread that performs the application-layer exchange:

  1. Wait for the listener to deliver the post-handshake SSL stream.
  2. Read the 24-byte client connection preface (PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n).
  3. Send an empty server SETTINGS frame (length=0, type=0x4, flags=0).
  4. Read the client's SETTINGS frame; verify type=SETTINGS and ack=0.
  5. Send SETTINGS-ACK (length=0, type=0x4, flags=0x1).
  6. Drain subsequent client frames (e.g. GOAWAY) until EOF or stop.

Frame encoding/decoding reuses production settings_frame and frame_header::parse -- no manual byte-level work in the test fixture.

Demo TEST_F

Http2ClientHermeticTransportTest, ConnectCompletesSettingsExchangeWithMockPeer asserts that with the new peer:

  • peer.settings_exchanged() reaches true (vs the prior tls-listener-only test that only reached listener.accepted() and stalled there)
  • peer.io_failed() stays false
  • client.is_connected() is true
  • client.get_settings() returns sane values (initial_window_size > 0, max_frame_size > 0)
  • client.disconnect() drives the GOAWAY emit path
  • client.is_connected() is false after disconnect

Coverage 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.yml workflow should report a strictly positive line-coverage delta on src/protocols/http2/http2_client.cpp after merge.

Issue #1074 stays open (Part of, not Closes); 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

  1. Read mock_h2_server_peer.{h,cpp} for RFC 7540 frame correctness.
  2. Verify the demo TEST_F's assertion sequence is hermetic and non-flaky (every socket binds 127.0.0.1:0; no DNS, no external network).
  3. Confirm tests/support/CMakeLists.txt correctly adds the new source to network_test_support.
  4. Wait for the coverage.yml PR comment -- line coverage on http2_client.cpp should 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

kcenon added 2 commits April 28, 2026 20:31
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
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
@github-actions

github-actions Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Metric Value
Line Coverage 66.5%
Branch Coverage 32.9%
Target 80% lines / 70% branches
Coverage Details

Full HTML report is available as a build artifact.

@kcenon kcenon merged commit c2c96df into develop Apr 28, 2026
14 checks passed
@kcenon kcenon deleted the feat/issue-1074-mock-h2-server-peer-phase-2a branch April 28, 2026 12:02
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.
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.
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
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
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