Skip to content

test(dns): remove flaky wall-clock upper bounds - #207

Merged
TMHSDigital merged 1 commit into
TMHSDigital:mainfrom
SatriaBagusNurhadianto:fix/remove-wall-clock-upper-bounds
Nov 9, 2026
Merged

TMHSDigital merged 1 commit into
TMHSDigital:mainfrom
SatriaBagusNurhadianto:fix/remove-wall-clock-upper-bounds

Conversation

@SatriaBagusNurhadianto

Copy link
Copy Markdown

What changed:

  • internal/dns/ratelimit_test.go: replaced tight upper bound with a hang-only ceiling (5x ideal)
  • internal/dns/resolver_test.go: replaced 500ms timing check with query-count assertion
  • internal/dnsserver/server_test.go: raised Close ceiling from 2s to 10s

Why:
Three tests asserted tight wall-clock upper bounds that flake under load on slow CI runners. Lower bounds and hang-only ceilings remain.

Tests:

  • go test ./internal/dns/ ./internal/dnsserver/ (pass)
  • go test -count=5 -shuffle=on (pass, GOMAXPROCS=2)
  • -race could not run locally (no cgo/gcc)

Related: #206

TMHSDigital added a commit that referenced this pull request Nov 9, 2026
#207 raised TestCloseWaitsForHandlers' ceiling from 2s to 10s. A parked
stream connection that Close fails to close is released by serveTCP's 5s
read deadline, so Close then returns in ~5s and passed the 10s ceiling:
the test no longer caught the regression it exists for (verified by
mutating Close to skip closing tracked connections). 4s still leaves ~20x
headroom over the normal ~200ms and fails on that mutation.

Refs #206

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01LpkeXSPwrzd7ZCuX3no7gD
@TMHSDigital
TMHSDigital merged commit 30d50f9 into TMHSDigital:main Nov 9, 2026
2 checks passed
@TMHSDigital

Copy link
Copy Markdown
Owner

Thanks for this! It's merged as 30d50f9. The rate-limiter and cancelled-context changes went in as you wrote them; asserting srv.Queries() == 0 is a better check than the timing it replaced.

I made one follow-up in 3a30307. The Close ceiling is now 4s instead of 10s. A parked stream connection that Close fails to close is still released by serveTCP's 5s read deadline (server.go:287). If that regression came back, Close would return in about 5s and pass a 10s ceiling. I checked this by mutating Close to skip closing tracked connections: the test passed at 10s and failed at 4s. 4s still gives about 20x headroom over the normal ~200ms.

go test -race -count=5 -shuffle=on ./internal/dns/ ./internal/dnsserver/ with GOMAXPROCS=2 passes, and CI on main is green.

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