Repository navigation
Push test coverage from ~94% to ~98% - #4
Merged
Merged
Conversation
Closes nearly all remaining coverage gaps identified via `cargo llvm-cov --show-missing-lines`: - api.rs: converted match+panic!() test assertions to assert!(matches!(...)) (the panic arm was itself an uncoverable dead branch on a passing run), and added the one real gap this revealed: Register with a bad signature was never exercised. - collision.rs, dns.rs, store.rs, gossip.rs: covered the remaining branch of LowerWins collision resolution, NS records, a sixth registration attempt while still live (not just after going stale), and made the ConformanceChallenge mismatch test deterministic instead of depending on which way a single random challenge happened to fall. - fcrdns.rs, ntp.rs: added tests for is_forward_confirmed's two failure-propagation branches, AAAA forward lookups, a wrong-record-type filter, multi-resolver fallback, a compression pointer loop, malformed/short responses, and a short NTP reply. - main.rs: extracted spawn_listener_tasks out of main() so the listener-spawning logic (which address goes to which listener, a --dns flag producing a fourth task, and each task's bind-failure logging) is unit-testable; excluded main() itself from coverage accounting with the nightly #[coverage(off)] attribute, since it's pure wiring around already-covered pieces that, by design, never returns in a real run. - node.rs: added a smoke test for the previously-never-called spawn_rate_limiter_maintenance. Workspace test count: 147 -> 167. Remaining gaps are essentially irreducible: infinite-loop listener tasks' async completion state inside tests that intentionally never let them finish, test-helper error arms that only fire on a background thread's own socket failure, and error variants (like a JSON encode failure) with no reachable trigger through this project's own types. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
This branch was originally opened as PR #3 stacked on the (then-open) PR #2. #2 has since merged into
main, but #3 was merged into the now-gonebinda-scaffold-and-featuresbranch rather thanmaindirectly, so this coverage commit never made it intomain. This PR re-targets that same work atmain.Closes nearly all remaining coverage gaps identified via
cargo llvm-cov --show-missing-linesafter #2:match { ... => {} other => panic!(...) }test assertions toassert!(matches!(...))— thepanic!arm was itself an uncoverable dead branch on a passing run, which was inflating the apparent gap. Also added the one real gap this revealed:Registerwith a bad signature was never exercised.LowerWinscollision resolution, NS records, a sixth registration attempt while still live (not just after going stale), and made theConformanceChallengemismatch test deterministic instead of depending on which way a single random challenge happened to fall.is_forward_confirmed's two failure-propagation branches, AAAA forward lookups, a wrong-record-type filter, multi-resolver fallback, a compression pointer loop, malformed/short responses, and a short NTP reply — via the same local fake-server pattern already established in Behavioral gossip conformance testing, CI/coverage tooling, and ~94% test coverage #2.spawn_listener_tasksout ofmain()so the listener-spawning logic (which address goes to which listener, a--dnsflag producing a fourth task, and each task's bind-failure logging) is unit-testable on its own. Excludedmain()itself from coverage accounting with the nightly#[coverage(off)]attribute, since it's pure wiring around already-covered pieces that, by design, never returns in a real run.spawn_rate_limiter_maintenance.Workspace test count: 147 → 167. Overall coverage: ~94% → ~98%.
What's left, and why
The remaining ~2% is essentially irreducible without contrived tests:
node.rslines are the "kick off an infinite listener loop" statements inside tests that intentionally never let that loop finish, so the async state machine's completion path is never counted — even though the loop's actual behavior is thoroughly exercised via real socket round-trips elsewhere in the same test.fcrdns/ntplines are a test-helper's own defensive error arm that only fires if a background thread's socket read fails after the test already got what it needed.Notes for reviewers
#[coverage(off)]requires#![feature(coverage_attribute)], which is fine since the project already pins Rust nightly.--test-threads=1and default parallelism).🤖 Generated with Claude Code