Skip to content

Push test coverage from ~94% to ~98% - #3

Merged
defcron merged 1 commit into
binda-scaffold-and-featuresfrom
binda-coverage-100
Sep 19, 2026
Merged

defcron merged 1 commit into
binda-scaffold-and-featuresfrom
binda-coverage-100

Conversation

@defcron

@defcron defcron commented Sep 19, 2026

Copy link
Copy Markdown
Member

Summary

Stacked on #2 (still open) — this only adds the coverage-completion work on top of it, so the diff here is just the new commit.

Closes nearly all remaining coverage gaps identified via cargo llvm-cov --show-missing-lines after #2:

  • api.rs: converted match { ... => {} other => panic!(...) } test assertions to assert!(matches!(...)) — the panic! 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: 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 — all via the same local fake-server pattern already established in Behavioral gossip conformance testing, CI/coverage tooling, and ~94% test coverage #2.
  • 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 on its own. 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. Overall coverage: ~94% → ~98%.

What's left, and why

The remaining ~2% is essentially irreducible without contrived tests:

  • A handful of node.rs lines 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.
  • A couple of fcrdns/ntp lines 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.
  • A few are error variants (like a JSON encode failure) with no reachable trigger through this project's own types without constructing a deliberately-broken value.

Notes for reviewers

  • #[coverage(off)] requires #![feature(coverage_attribute)], which is fine since the project already pins Rust nightly.
  • All new tests verified stable across repeated runs (both --test-threads=1 and default parallelism).

🤖 Generated with Claude Code

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>
@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@defcron
defcron merged commit e6480cf into binda-scaffold-and-features Sep 19, 2026
3 checks passed
defcron added a commit that referenced this pull request Sep 19, 2026
Merge pull request #3 from gptenv/binda-coverage-100
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.

2 participants