test(quic): expand quic_socket.cpp coverage#1071
Merged
Merged
Conversation
Add 35 unit tests targeting public surface of quic_socket reachable without an active QUIC peer: - Construction / destruction lifetime (IPv6 socket, destructor with active receive, RFC 9000 connection ID length bounds) - connect() validation guards: server-role rejection, empty/explicit server name, port-0 endpoint, post-close re-connect rejection - accept() validation guards: client-role rejection, missing/empty cert paths, post-close re-accept rejection - close() variations: non-zero error code, empty reason, long reason string, default arguments - send_stream_data() validation: empty payload, oversized payload, fin flag, max stream id (all rejected when not connected) - create_stream() / close_stream() validation guards - Receive loop idempotency: stop without start, start twice, start-stop-start cycle - State queries: state remains idle after failed connect, remote_endpoint defaults to zero, is_connected/is_handshake_complete consistency in idle state - Multi-instance isolation: independent connection IDs, close on one instance does not affect another - Move semantics: move-assignment transfers state, self-move-assignment is a no-op - Callback edge cases: null callbacks accepted, overwrite replaces - Enum sanity: quic_role and quic_connection_state numeric values - const socket accessor Acceptance criteria of #1065 (>= 80% line / >= 70% branch) cannot be fully met without an in-process QUIC loopback fixture. These tests expand coverage of reachable validation and state-machine paths only. Part of #1065 Part of #953
Contributor
Coverage Report
Coverage DetailsFull HTML report is available as a build artifact. |
…ic-socket-coverage
This was referenced Apr 27, 2026
kcenon
added a commit
that referenced
this pull request
Apr 28, 2026
…eer (#1079) Add 8 new TEST_F cases under the existing QuicSocketHermeticTransportTest fixture in tests/unit/quic_socket_branch_test.cpp. They use make_loopback_udp_pair + mock_udp_peer to exercise paths that PR #1071 left uncovered because that PR only validated idle/disconnected state. New paths driven: - connect() success path past TLS init / derive_initial_secrets / start_handshake / queue_crypto_data / send_pending_packets / send_packet, with the loopback peer capturing the long-header Initial datagram and verifying the 0xC0+ first byte and a parsed src connection id. - connect() state transition from idle to handshake_start/handshake. - close() at the initial encryption level building a CONNECTION_CLOSE frame and dispatching it as a second datagram on the same socket. - do_receive() lambda handing a real datagram to handle_packet, with the silent-drop branches taken when packet_parser::parse_header fails (1-byte input) or when get_read_keys reports keys-not-yet- derived (long-header Initial stub on a fresh client). - stop_receive() correctness under an active peer-side sender. - Distinct connection-id generation across two independent client sockets, asserted via SCID extraction from the captured Initial packets. - connect() with an explicit non-empty SNI string still emitting a long-header Initial. No new test_support files; no CMake changes (the file is already linked to network::test_support at tests/CMakeLists.txt:4962). The acceptance criteria of issue #1065 (>= 80% line / >= 70% branch on src/internal/quic_socket.cpp) remain unreachable until Phase 2C of #1074 lands mock_quic_peer_loop with TLS 1.3 key derivation, so this PR ships incremental progress only. Part of #1065 Part of #953
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
2 participants
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
Expands unit-test coverage of
src/internal/quic_socket.cppby adding 35 new tests targeting validation guards, public-API input variations, multi-instance state isolation, and struct/result edge cases.Change Type
Affected Components
tests/test_quic_socket.cpp(test additions only; no production code changed)Why
Problem Solved
quic_socket.cppwas identified as the #4 worst-coverage target in the 2026-04-26 lcov re-measurement (43.7% line / 21.3% branch). This PR expands coverage of the surface reachable without an active QUIC peer.Honest Scope Note
The acceptance criteria of #1065 (>= 80% line / >= 70% branch) cannot be fully met by these tests alone. The majority of
quic_socket.cpp(handshake, frame processing, packet protection, retransmission) requires an in-process QUIC loopback fixture, which does not yet exist in this repo. This PR therefore usesPart of(notCloses).The new tests still meaningfully exercise:
connect()/accept()/close()send_stream_data()andcreate_stream()/close_stream()early-return pathsRelated Issues
Who
Reviewers
Required Approvals
When
Urgency
Target Release
Next minor (
v2.x.x)Where
Files Changed
tests/test_quic_socket.cppAPI / Database Changes
None.
How
Implementation Details
QuicSocketTestfixtureTesting Done
Test Plan for Reviewers
src/internal/quic_socket.cpp(expect partial, not full, improvement)Breaking Changes
None.
Rollback Plan
Revert this commit; no schema or API changes.