tests: the review-002 coverage residue — stage exits, splits, spent arms (N-8)
Eighteen new tests, zero production-code changes; every line the review attributed in §N-8 is now covered (verified by a `cargo llvm-cov --show-missing-lines` before/after diff). - producer: the datagram stage's failure exits pinned as direct unit tests on `run_datagram_stage` (the serve layer discards StageExit, so the unit shape is the only place the exact matrix is assertable) — `ReplyFrameRejected` (a joint-bound-violating reply), `EgressEnded` via `next_reply`'s Err, `Io` via a failing write and a failing read; the `send`-failure arm got the serve-layer drop-witness treatment (full ASSOCIATE conversation, EOF after the refusal, the drop flag flips — teardown-at-birth on the failure path) - producer: the quiet wait's `Err(_)` arm (a read *error* ends the wait `Ok(())`, the accept future drops with its owned state — the client-gone contract's error-shaped twin; driven directly on `bind_quiet_wait_and_serve`) and the BIND reply#2 Domain-peer fallback (`TargetAddr::Domain(_, port)` → `0.0.0.0:port`, the data plane live end to end) - producer: the establisher's `None`-witness arm (the registration's optional witness seam) added to the existing unit test - consumer: the two front-door-bridge `split`s tested on live `Socks5Stream<BiStream>` streams via `build_skip_auth_bi_stream` (two duplexes joined the `open_and_negotiate` shape) — raw framed bytes through both halves; the BIND split leaves the take-guard untouched (`await_inbound` still reads the reply#2 phase) - consumer: the single-arm residue — spent `send_datagram`'s `Io` arm (+ the spent-`recv` `Ok(None)` shape asserted alongside), the mid-batch later-frame `Err` dropping the pending `Ok`s (the fail-loud-drops-pending semantics), ASSOCIATE `into_inner`'s `Ok` pairing, `BindDataPlane`'s `poll_flush`/`poll_shutdown` passthroughs (the BIND door's half-close), `read_reply2`'s version pin (a foreign producer's non-5 reply#2), the ASSOCIATE/BIND session `Debug` bodies, the spent-`close` no-op - egress/error: the three seam-handle `Debug` renders (TargetHandle / BindTargetHandle / BindAccept) and `SocksEstablishmentReason`'s `Display` (each variant renders as its canonical wire code) Coverage: 96.31% regions / 97.22% lines; no new accepted-uncovered entries (the residual uncovered lines are the pre-existing out-of-scope set + producer.rs 188-189's untestable `accept_bi` substrate-failure log arm). One test-only select-shape discovery recorded in the task Notes: `tokio::select!`'s random branch order can drop a not-yet-polled accept future before its owned state exists — the quiet-wait test parks once before erroring so both branches are guaranteed started. Verification: `cargo test` + `--all-features` green (375 tests), clippy `-D warnings` clean (native + wasm), fmt clean, `cargo doc` clean, both wasm checks clean.
This commit is contained in:
1 parent
986bd13ce7
commit
8dbe73d49b
5 files changed
+1009
-8
No files matched your search
@@ -1,7 +1,7 @@
|
||||
---
|
||||
id: implementation/fix-coverage-residue
|
||||
name: "Tests — the review-002 coverage residue (N-8: stage exits, splits, spent arms)"
|
||||
status: pending
|
||||
status: completed
|
||||
depends_on: [implementation/fix-bind-reply2-reentry, implementation/fix-producer-gate-policy, implementation/fix-rfc1928-posture]
|
||||
scope: moderate
|
||||
risk: low
|
||||
@@ -77,19 +77,19 @@ through an artificial shape.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [ ] The stage's failure exits tested (`ReplyFrameRejected`,
|
||||
- [x] The stage's failure exits tested (`ReplyFrameRejected`,
|
||||
`EgressEnded` both arms, `Io`) — the `send`-failure arm with
|
||||
the drop-witness assertion
|
||||
- [ ] `AssociateSession::split` + `BindSession::split` tested on a
|
||||
- [x] `AssociateSession::split` + `BindSession::split` tested on a
|
||||
live stream (or the batch-2-door note recorded per arm)
|
||||
- [ ] The single-arm residue covered: spent-`send`, mid-batch
|
||||
- [x] The single-arm residue covered: spent-`send`, mid-batch
|
||||
later-frame `Err`, ASSOCIATE `into_inner`'s `Ok`,
|
||||
`BindDataPlane` shutdown, `read_reply2` version mismatch, the
|
||||
quiet-wait `Err(_)` arm, the Domain-peer fallback, the
|
||||
`Debug`/`Display` renders
|
||||
- [ ] `cargo llvm-cov` re-run: the named lines covered or recorded
|
||||
- [x] `cargo llvm-cov` re-run: the named lines covered or recorded
|
||||
as accepted-uncovered with a reason in the task Notes
|
||||
- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean
|
||||
- [x] `cargo test` green; clippy/fmt clean; both wasm checks clean
|
||||
|
||||
## References
|
||||
|
||||
@@ -103,8 +103,137 @@ through an artificial shape.
|
||||
|
||||
## Notes
|
||||
|
||||
> Agent fills this during implementation.
|
||||
> Filled by the implementation agent (2026-09-24):
|
||||
|
||||
- **All N-8-named lines covered; no accepted-uncovered entries
|
||||
added.** The `cargo llvm-cov --show-missing-lines` diff before →
|
||||
after: every line the review attributed (consumer.rs 356-359/
|
||||
419/437/453-456/636-642/771-774/843 pre-fix numbering; producer.rs
|
||||
374-386/405-414/560/568; egress.rs 72-74/226-230/275-279;
|
||||
error.rs 113-115) is now covered. The suite grew 357 → 375 tests
|
||||
(145 lib unit / 38 associate / 41 bind / 40 connect / 41
|
||||
integration / 30 harness / 37 differential / 3 doc-tests);
|
||||
TOTAL 96.31% regions / 97.22% lines.
|
||||
- **The stage's failure exits were pinned as direct unit tests on
|
||||
`run_datagram_stage`** (the serve layer discards `StageExit`, so
|
||||
the unit shape is the only place the exact exit matrix is
|
||||
assertable): `datagram_stage_reply_frame_rejected_exit` (an
|
||||
egress reply whose inner bytes violate the u16 joint bound →
|
||||
`ReplyFrameRejected`), `datagram_stage_egress_relay_ended_exit_
|
||||
via_next_reply` (`next_reply`'s `Err` → `EgressEnded`),
|
||||
`datagram_stage_write_failure_is_the_io_exit` + `datagram_stage_
|
||||
read_failure_is_the_io_exit` (a failing channel write/read →
|
||||
`Io`). The `send`-failure arm got the serve-layer drop-witness
|
||||
treatment: `udp_egress_send_failure_ends_the_association_and_
|
||||
drops_the_egress` (a full ASSOCIATE conversation, one datagram in,
|
||||
the egress refuses it, EOF reaches the client, the stage exits,
|
||||
and the drop flag flips — teardown-at-birth on the failure path).
|
||||
The tests ride `FailStream` (a poll-failing stream) and
|
||||
`OneReplyEgress`/`RelayEndedEgress`/`ParkedEgress` (stage-shaped
|
||||
egress fakes), all inside the producer test module.
|
||||
- **The two splits tested on live streams.** `associate_split_
|
||||
yields_live_halves_and_survives_the_round_trip` and `bind_split_
|
||||
yields_live_halves_pre_reply2` exercise `tokio::io::split` on a
|
||||
live `Socks5Stream<BiStream>` built by a new test helper
|
||||
(`build_skip_auth_bi_stream` — two duplexes joined via
|
||||
`BiStream::from_joined`, the same shape `open_and_negotiate`
|
||||
assembles): raw framed bytes pass through both directions on the
|
||||
borrowed halves, and (BIND) the split leaves the take-guard
|
||||
untouched — `await_inbound` still reads the reply#2 phase after
|
||||
the halves are dropped. No "batch-2 door exercises it" note
|
||||
needed; the tests stand alone.
|
||||
- **The single-arm residue, test by test:**
|
||||
`send_datagram_on_a_spent_stream_is_the_io_arm` (the spent-stream
|
||||
`Io` arm — and the spent-`recv` shape asserted in the same test:
|
||||
`Ok(None)`, the family's read-side posture),
|
||||
`recv_datagram_mid_batch_later_frame_err_drops_the_pending_oks`
|
||||
(first frame `Ok`, a later one garbage → the pending `Ok`s dropped
|
||||
with the session, the fail-loud-drops-pending semantics pinned),
|
||||
`associate_into_inner_pairs_the_stream_with_the_teardown_owner`
|
||||
(the `TakenAssociateInner` Ok arm — the CONNECT
|
||||
`into_inner_pairs_the_pipe_with_the_teardown_owner` shape's
|
||||
ASSOCIATE mirror),
|
||||
`bind_data_plane_poll_flush_and_poll_shutdown_pass_through`
|
||||
(write → `poll_flush` → `poll_shutdown` — the bytes reach the peer
|
||||
and the EOF sentinel follows),
|
||||
`read_reply2_rejects_a_foreign_producers_version` (a non-5
|
||||
version reply#2 → `UnsupportedSocksVersion(0x04)`, loud),
|
||||
`bind_quiet_wait_read_error_ends_the_wait_cleanly` (the quiet
|
||||
wait's `Err(_)` arm driven directly on `bind_quiet_wait_and_serve`
|
||||
— a read *error* ends the wait `Ok(())` and the accept future
|
||||
drops with its owned state, the client-gone contract's
|
||||
error-shaped twin), `bind_domain_peer_falls_back_to_the_
|
||||
unspecified_address` (a foreign accept handle carrying
|
||||
`TargetAddr::Domain(_, port)` peer → reply#2 carries
|
||||
`0.0.0.0:port`, data plane live),
|
||||
`debug_impls_render_the_handle_surfaces` (the three egress.rs
|
||||
`Debug` bodies — TargetHandle non-exhaustive, BindTargetHandle
|
||||
names the peer, BindAccept names the bind address),
|
||||
`establishment_reason_display_renders_the_wire_code` (the
|
||||
`SocksEstablishmentReason` `Display` — each variant renders as its
|
||||
canonical wire code), `associate_debug_renders_channel_liveness_
|
||||
and_pending` (the ASSOCIATE + BIND session `Debug` bodies),
|
||||
`spent_associate_close_reaps_a_no_op` (the `close`-on-spent arm),
|
||||
and `establisher_records_identity_and_defaults_the_establishment`
|
||||
extended with the `None`-witness arm.
|
||||
- **A quiet-wait select-shape discovery (test-only note):**
|
||||
`tokio::select!` polls branches in random order — a read branch
|
||||
that errors on its *first* poll can win before the accept branch
|
||||
is ever polled, dropping the accept future at zero polls (the
|
||||
owned state never constructed; a drop flag would never fire — the
|
||||
first cut of the test caught this flake). The test's `FailReadStream`
|
||||
parks once (with a wake) then errors on the second poll so both
|
||||
branches are guaranteed started; the assertion then holds
|
||||
deterministically. Production semantics unaffected (either exit
|
||||
drops the accept future; the contract is exit-shaped, not
|
||||
poll-count-shaped).
|
||||
- **One review-item line numbers shifted post-fix-tasks**: the
|
||||
review's `consumer.rs:396-398` (the covered spent-`recv` arm) is
|
||||
now `418` (the `inner.as_mut()` else arm after the C-1/D-1
|
||||
changes); it is covered by the same test that pins the spent-send
|
||||
arm. `consumer.rs:937` (the spent-bind test's `err().map()`
|
||||
diagnostic closure) stays uncovered — it only evaluates on `Err`,
|
||||
and the spent shape is `Ok(None)` by design (the C-1 posture);
|
||||
cosmetic, not chased.
|
||||
- **Reachability check for the arm-became-unreachable clause**: no
|
||||
N-8 arm was made unreachable by the three code-fix tasks — every
|
||||
named arm still exists at its (shifted) location and is now
|
||||
tested. The explicit out-of-scope items stayed out of scope:
|
||||
`rfc1928/client.rs:217-219` remains accepted-uncovered (with its
|
||||
translated-error note), as do the fork's other accepted-uncovered
|
||||
blocks.
|
||||
- **Residual uncovered lines after this task (none new)**: producer
|
||||
.rs 188-189 (the pump handler's `accept_bi` substrate-failure log
|
||||
arm — pre-existing, not in N-8's list; untestable without a
|
||||
broken `Connection` outside alkcall's test surface),
|
||||
rfc1928/client.rs 217-219 + the fork internals (accepted-
|
||||
uncovered per review-001/002), wire.rs/mod.rs/target_addr.rs
|
||||
internals (pre-existing, not in N-8's list), and the test modules'
|
||||
own panic-guard/unreachable lines (coverage-of-tests noise).
|
||||
|
||||
## Summary
|
||||
|
||||
> Agent fills this on completion.
|
||||
Closed review 002's N-8 coverage residue with 18 new tests and zero
|
||||
production-code changes: the datagram stage's three failure exits
|
||||
pinned as direct unit tests on `run_datagram_stage`
|
||||
(`ReplyFrameRejected`, `EgressEnded` via `next_reply`, `Io` via a
|
||||
failing write and a failing read) plus the serve-layer drop-witness
|
||||
test for the `send`-failure arm (the egress drops with the stage on
|
||||
the failure path too); the two front-door-bridge `split`s tested on
|
||||
live `Socks5Stream<BiStream>` streams (raw bytes through both
|
||||
halves; the BIND split leaves the take-guard untouched); and the
|
||||
single-arm residue (spent `send_datagram`'s `Io` arm + the spent
|
||||
`recv` shape, the mid-batch later-frame `Err` dropping pending `Ok`s,
|
||||
ASSOCIATE `into_inner`'s `Ok` pairing, `BindDataPlane`'s
|
||||
`poll_flush`/`poll_shutdown` passthroughs, `read_reply2`'s version
|
||||
pin, the quiet wait's `Err(_)` arm, the BIND reply#2 Domain-peer
|
||||
fallback `0.0.0.0:port`, the three egress `Debug` renders, the
|
||||
`SocksEstablishmentReason` `Display`, the ASSOCIATE/BIND session
|
||||
`Debug` bodies, the spent-`close` no-op, and the establisher's
|
||||
`None`-witness arm). `cargo llvm-cov` re-run: every N-8-named line
|
||||
covered, no new accepted-uncovered entries (the residual uncovered
|
||||
lines are the pre-existing out-of-scope set + producer.rs 188-189's
|
||||
untestable `accept_bi` log arm). Verification: `cargo test` +
|
||||
`--all-features` green (375 tests), clippy `-D warnings` clean
|
||||
(native + wasm), fmt clean, `cargo doc` clean, both wasm checks
|
||||
clean; coverage 96.31% regions / 97.22% lines.
|
||||
Reference in new issue
Block a user