Skip to content

test: whole test-tree audit — tests that could not fail now do, no wall-clock windows or sleep-to-sync, duplicates and dead fixtures cut - #543

Closed
CMGS wants to merge 7 commits into
mainfrom
review/test-cleanup
Closed

CMGS wants to merge 7 commits into
mainfrom
review/test-cleanup

Conversation

@CMGS

@CMGS CMGS commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

A whole-file audit of every test file in the repo: about 60k lines of Go tests, 5k of Rust and 3k of Python. Each file was read in full against the test rules (layout, zero comments, modern Go) and for test value. Every finding was checked against the source before it was fixed.

Tests that could not catch their regression, now strengthened

Each was proven by temporarily reverting the production behavior: the old test passed against the revert, the new one fails.

Test Gap before Mutant used to prove it
TestIdleOnceSkipsArchived, TestIdleOnceSkipsVolumeClaim hibernateLocked refuses before any engine call, so removing the skip changed nothing observable. Now the claim's Transition is held across the sweep, and the test asserts the sweep did not pick the claim. remove the skip condition
assertRestartFailsUntilBudget restart == exit-1 also holds when no attempt is billed drop the restart billing
TestAnAdoptionConfirmsTheClaimsAddressBeforeTakingItsRow checked that a confirmation happened, not its order confirm after Take
TestTheTunnelTargetPrefersIPv4 resolved the host's real localhost, which Ubuntu lists IPv4 first. The selection is now a pure tunnelAddr with a byte-identical result, driven with [::1, 127.0.0.1]. take the first address
TestCloseBoxesWaitsForEveryReleaseOnce a 4th release panicked inside the handler, and net/http swallowed the panic remove sync.Once
test_volume_timeout_bounds_the_call the 2 s client timeout raised the same error when the per-call timeout was ignored ignore the per-call timeout
Tenant delete fan-out, broadcast delete, e2e hibernate-with-volume checked a total, or only that an error occurred fan out twice, send everything to one peer, map 409 to 500

Flakes removed

  • Wall-clock upper bounds and throughput windows:
    • egress limiter, unthrottled download, store probe;
    • engine dials, tenants/pg;
    • Python stream/hardening/client, silkd forward.
  • Sleep-to-sync, replaced by signals or synctest:
    • tunnet router, now 17 tests in synctest;
    • reader fence, move retry, limiter sink, s3 populate;
    • silkd tun sink, exec/pty readiness.
  • Concurrency and harness fixes:
    • an unlocked refillSem swap;
    • a process-wide runtime.NumGoroutine and TotalAlloc;
    • unbounded receives and reads;
    • a main_test that could shut down before serving;
    • a 300 ms lease checked as live under -race.

Cut

  • Duplicate tests: checkpoint bad ids, hibernate wake failure, reader off, failover adoption, server tenant rows, move budget row, redirect tests, silkd tree push, Python redirect/volume/checkpoint (now parametrized).
  • Dead code:
    • unread fixtures: the engine COCOON_TEST_ATTACHED, sdk deletes, volume.img;
    • unreachable Python info arms;
    • the silkd throughput "test", which asserted nothing and was skipped in CI, plus its helpers and the --skip throughput in silkd.yml.

Not changed, on purpose

  • tun_test "no tun_down" sleep and fork_test sleep: making them observable needs a production hook.
  • Env-gated suites (tun exit, real S3, TLS/caddy): unchanged.
  • storetest.WaitStacks stack-text coupling: kept.
  • // SAFETY: comments in Rust tests: required by undocumented_unsafe_blocks.

Gates (this branch, on main)

  • make go-lint (fresh cache): 10× 0 issues.
  • asl -forwarder=false: clean for 4 modules on both GOOS.
  • go test -race: all 5 modules ok. Changed tests also passed at -race -count=20, and the PG-gated suites passed against a live Postgres.
  • silkd: fmt and clippy -D warnings clean, all test binaries ok, and the CI root-only list passed as root in a Linux container.
  • Python: ruff clean; pytest 305 / 11 / 15 passed.
  • Comment lines: 0 added.

Merge order

Written against main plus the open #530 and #533–#542, then rebased onto main. Only test files overlap with those PRs:

Merge it after them; I'll rebase it once they land.

CMGS added 7 commits November 10, 2026 00:18
… drives with an IPv6-first list

The test resolved the host's real localhost, which Ubuntu lists IPv4 first,
so a take-the-first regression passed on Linux CI.
…ns; no unlocked writes, wall-clock windows or duplicates

TestIdleOnceSkipsArchived and TestIdleOnceSkipsVolumeClaim passed with their
skip removed, and the restart-budget helper passed with no attempt billed.
The reload test swapped refillSem without m.mu, claims counted every
goroutine in the process, and several tests slept to sync or to prove a
negative. The warmup-argument test moves to config, where the check lives.
…s and signals, not windows

The limit test asserted upper bounds on throughput after sleeps, the splice
test counted every allocation in the process, the router tests polled in real
time where synctest applies, and main_test could shut down before serving.
…hroughput bench and info arms removed, fixture counts pinned

The per-call timeout test passed with the per-call timeout ignored, because
the client timeout raised the same error. The throughput test asserted
nothing and CI skipped it.
… lookup's first-success is gated, not timed

The process-wide recommit count could include a retrier an earlier test left
in backoff. Dropping the lookup stopwatch also dropped what it checked: a gated
peer now proves lookup returns from the owner while a slow peer still holds,
with the probe timeout raised past the guard so a serial lookup fails.
@CMGS

CMGS commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Replaced by #544: the same commits rebased onto main after #530 and #533–#542 merged.

@CMGS CMGS closed this Nov 9, 2026
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