tests: rename internal/livedns to livednstest and deny it outside tests #165

Open
clawbot wants to merge 1 commits from issue-164-livednstest into next
9 changed files with 40 additions and 33 deletions
+4
View File
@@ -60,6 +60,10 @@ linters:
desc: >- desc: >-
Test-support code belongs in test files and in packages whose Test-support code belongs in test files and in packages whose
directory name ends in test, not in the shipped binary. directory name ends in test, not in the shipped binary.
- pkg: sneak.berlin/go/dnswatcher/internal/livednstest
desc: >-
Live-DNS test support belongs in test files and in packages
whose directory name ends in test, not in the shipped binary.
# Only decisions already recorded in the Go package defaults are # Only decisions already recorded in the Go package defaults are
# listed here. Every entry matches the module path exactly. # listed here. Every entry matches the module path exactly.
gomodguard_v2: gomodguard_v2:
+1 -1
View File
@@ -278,7 +278,7 @@ internal/
tlscheck/tlscheck.go TLS certificate inspector tlscheck/tlscheck.go TLS certificate inspector
notify/notify.go Notification service (Slack, Mattermost, ntfy) notify/notify.go Notification service (Slack, Mattermost, ntfy)
watcher/watcher.go Main monitoring orchestrator and scheduler watcher/watcher.go Main monitoring orchestrator and scheduler
livedns/livedns.go Retry and concurrency limit for tests livednstest/livednstest.go Retry and concurrency limit for tests
against live DNS (imported only by tests) against live DNS (imported only by tests)
``` ```
+1 -1
View File
@@ -25,7 +25,7 @@ real servers ensures the resolver works correctly in production.
- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) - Query timeout is calibrated to 3× maximum antipodal RTT (~300ms)
plus processing margin plus processing margin
- Root server fan-out is limited to reduce parallel query load - Root server fan-out is limited to reduce parallel query load
- Live lookups that expect an answer go through `internal/livedns`, - Live lookups that expect an answer go through `internal/livednstest`,
which limits how many run at once in a test binary and retries a which limits how many run at once in a test binary and retries a
lookup that got none lookup that got none
- Flaky failures from transient network issues are acceptable and - Flaky failures from transient network issues are acceptable and
+5 -2
View File
@@ -19,6 +19,9 @@ Rationale, Design, TODO, License, Author) if any are still missing.
# Completed Steps # Completed Steps
- 2026-09-29: the live-DNS test package is renamed `internal/livednstest` and
added to the `test-support` `deny` list in `.golangci.yml`, so `make lint`
fails when program code imports it (closes #164).
- 2026-09-29: `.golangci.yml` re-fetched unchanged from `sneak/prompts`. It - 2026-09-29: `.golangci.yml` re-fetched unchanged from `sneak/prompts`. It
replaces the deprecated `gomodguard` with `gomodguard_v2`, so `make lint` no replaces the deprecated `gomodguard` with `gomodguard_v2`, so `make lint` no
longer warns about it, and turns on `depguard` with the org `test-support` longer warns about it, and turns on `depguard` with the org `test-support`
@@ -30,8 +33,8 @@ Rationale, Design, TODO, License, Author) if any are still missing.
record and nameserver changes by preparing the saved state a check starts record and nameserver changes by preparing the saved state a check starts
from; the resolver timeout test queries an address that never answers, and from; the resolver timeout test queries an address that never answers, and
`NewFromLoggerWithClient`, used only by its stand-in client, is gone. The `NewFromLoggerWithClient`, used only by its stand-in client, is gone. The
live-DNS retry and concurrency limit moved to `internal/livedns`, which both live-DNS retry and concurrency limit moved to `internal/livednstest`, which
test packages use. `TESTING.md` states the README's rule (closes #159). both test packages use. `TESTING.md` states the README's rule (closes #159).
- 2026-09-28: the inconsistency alert is sent once, on the check where two - 2026-09-28: the inconsistency alert is sent once, on the check where two
nameservers start to disagree or where a nameserver that disagrees first nameservers start to disagree or where a nameserver that disagrees first
appears, instead of on every check while they disagree, and not again after appears, instead of on every check while they disagree, and not again after
@@ -1,4 +1,4 @@
// Package livedns runs the live DNS operations of tests. Tests that // Package livednstest runs the live DNS operations of tests. Tests that
// look something up in DNS query live DNS servers, never a stand-in — // look something up in DNS query live DNS servers, never a stand-in —
// see TESTING.md. Nothing here mocks, fakes, stubs, records or replays // see TESTING.md. Nothing here mocks, fakes, stubs, records or replays
// DNS, and nothing here skips a test: it only changes *how* the live // DNS, and nothing here skips a test: it only changes *how* the live
@@ -22,7 +22,7 @@
// first attempt. A fault in the code under test that leaves // first attempt. A fault in the code under test that leaves
// nothing to check looks the same as live DNS not answering, and // nothing to check looks the same as live DNS not answering, and
// fails only after the last attempt. // fails only after the last attempt.
package livedns package livednstest
import ( import (
"context" "context"
@@ -1,4 +1,4 @@
package livedns_test package livednstest_test
import ( import (
"context" "context"
@@ -8,7 +8,7 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/livednstest"
) )
// Tests for the retry and the concurrency limit themselves. They // Tests for the retry and the concurrency limit themselves. They
@@ -21,11 +21,11 @@ func TestRetryRecoversFromTransientFailure(t *testing.T) {
attempts := 0 attempts := 0
livedns.Retry(t, "transient", func(_ context.Context) error { livednstest.Retry(t, "transient", func(_ context.Context) error {
attempts++ attempts++
if attempts < wantAttempts { if attempts < wantAttempts {
return livedns.ErrNoAnswer return livednstest.ErrNoAnswer
} }
return nil return nil
@@ -37,19 +37,19 @@ func TestRetryRecoversFromTransientFailure(t *testing.T) {
func TestRetryGivesEachAttemptADeadline(t *testing.T) { func TestRetryGivesEachAttemptADeadline(t *testing.T) {
t.Parallel() t.Parallel()
livedns.Retry(t, "deadline", func(ctx context.Context) error { livednstest.Retry(t, "deadline", func(ctx context.Context) error {
deadline, ok := ctx.Deadline() deadline, ok := ctx.Deadline()
assert.True(t, ok, "attempt should carry a deadline") assert.True(t, ok, "attempt should carry a deadline")
remaining := time.Until(deadline) remaining := time.Until(deadline)
assert.LessOrEqual(t, remaining, livedns.AttemptTimeout) assert.LessOrEqual(t, remaining, livednstest.AttemptTimeout)
// Lower bound too: without one this passes for a // Lower bound too: without one this passes for a
// deadline far shorter than intended, which would // deadline far shorter than intended, which would
// silently turn every live attempt into an instant // silently turn every live attempt into an instant
// timeout. // timeout.
assert.Greater(t, remaining, livedns.AttemptTimeout/2) assert.Greater(t, remaining, livednstest.AttemptTimeout/2)
return nil return nil
}) })
@@ -73,7 +73,7 @@ func TestRunBoundsConcurrency(t *testing.T) {
go func() { go func() {
defer wg.Done() defer wg.Done()
_ = livedns.Run(func(_ context.Context) error { _ = livednstest.Run(func(_ context.Context) error {
mu.Lock() mu.Lock()
inFlight++ inFlight++
@@ -97,7 +97,7 @@ func TestRunBoundsConcurrency(t *testing.T) {
assert.Positive(t, maxSeen) assert.Positive(t, maxSeen)
assert.LessOrEqual( assert.LessOrEqual(
t, maxSeen, livedns.Concurrency, t, maxSeen, livednstest.Concurrency,
"live queries must stay under the package-wide gate", "live queries must stay under the package-wide gate",
) )
} }
+14 -14
View File
@@ -9,7 +9,7 @@ import (
"strings" "strings"
"testing" "testing"
"sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/livednstest"
"sneak.berlin/go/dnswatcher/internal/resolver" "sneak.berlin/go/dnswatcher/internal/resolver"
) )
@@ -20,9 +20,9 @@ import (
// Tests that look something up in DNS query live DNS servers, never a // Tests that look something up in DNS query live DNS servers, never a
// stand-in; logic that works on record data may be tested on that // stand-in; logic that works on record data may be tested on that
// data with no lookup (see TESTING.md). Each live operation below goes // data with no lookup (see TESTING.md). Each live operation below goes
// through livedns.Retry, which bounds how many resolutions are in // through livednstest.Retry, which bounds how many resolutions are in
// flight at once and retries an operation that got no answer (see // flight at once and retries an operation that got no answer (see
// package livedns). // package livednstest).
// //
// Where an assertion spans several independent nameservers, a quorum // Where an assertion spans several independent nameservers, a quorum
// is enough: a strict majority answering as expected. A server that // is enough: a strict majority answering as expected. A server that
@@ -162,7 +162,7 @@ func liveFindAuthoritative(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"FindAuthoritativeNameservers("+domain+")", "FindAuthoritativeNameservers("+domain+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -174,7 +174,7 @@ func liveFindAuthoritative(
if len(ns) == 0 { if len(ns) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: %s has no nameservers", "%w: %s has no nameservers",
livedns.ErrNoAnswer, domain, livednstest.ErrNoAnswer, domain,
) )
} }
@@ -198,7 +198,7 @@ func liveLookupNS(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"LookupNS("+domain+")", "LookupNS("+domain+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -210,7 +210,7 @@ func liveLookupNS(
if len(ns) == 0 { if len(ns) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: %s has no nameservers", "%w: %s has no nameservers",
livedns.ErrNoAnswer, domain, livednstest.ErrNoAnswer, domain,
) )
} }
@@ -240,7 +240,7 @@ func liveQueryNameserver(
var out *resolver.NameserverResponse var out *resolver.NameserverResponse
livedns.Retry( livednstest.Retry(
t, t,
what, what,
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -255,7 +255,7 @@ func liveQueryNameserver(
resp.Status == resolver.StatusError { resp.Status == resolver.StatusError {
return fmt.Errorf( return fmt.Errorf(
"%w: %s returned %s: %s", "%w: %s returned %s: %s",
livedns.ErrNoAnswer, nameserver, livednstest.ErrNoAnswer, nameserver,
resp.Status, resp.Error, resp.Status, resp.Error,
) )
} }
@@ -282,7 +282,7 @@ func liveQueryAllNameservers(
var out map[string]*resolver.NameserverResponse var out map[string]*resolver.NameserverResponse
livedns.Retry( livednstest.Retry(
t, t,
"QueryAllNameservers("+hostname+")", "QueryAllNameservers("+hostname+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -294,7 +294,7 @@ func liveQueryAllNameservers(
if len(results) == 0 { if len(results) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: no nameservers queried for %s", "%w: no nameservers queried for %s",
livedns.ErrNoAnswer, hostname, livednstest.ErrNoAnswer, hostname,
) )
} }
@@ -327,7 +327,7 @@ func liveResolveIPs(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"ResolveIPAddresses("+hostname+")", "ResolveIPAddresses("+hostname+")",
func(ctx context.Context) error { func(ctx context.Context) error {
@@ -339,7 +339,7 @@ func liveResolveIPs(
if len(ips) == 0 { if len(ips) == 0 {
return fmt.Errorf( return fmt.Errorf(
"%w: no addresses for %s", "%w: no addresses for %s",
livedns.ErrNoAnswer, hostname, livednstest.ErrNoAnswer, hostname,
) )
} }
@@ -366,7 +366,7 @@ func liveResolveIPsAllowingEmpty(
var out []string var out []string
livedns.Retry( livednstest.Retry(
t, t,
"ResolveIPAddresses("+hostname+")", "ResolveIPAddresses("+hostname+")",
func(ctx context.Context) error { func(ctx context.Context) error {
+1 -1
View File
@@ -33,7 +33,7 @@ func newTestResolver(t *testing.T) *resolver.Resolver {
// findOneNSForDomain picks one authoritative nameserver to aim a // findOneNSForDomain picks one authoritative nameserver to aim a
// test at. Quorum handling lives in livedns_test.go, and the live-DNS // test at. Quorum handling lives in livedns_test.go, and the live-DNS
// retry and concurrency limit in package livedns. // retry and concurrency limit in package livednstest.
func findOneNSForDomain( func findOneNSForDomain(
t *testing.T, t *testing.T,
r *resolver.Resolver, r *resolver.Resolver,
+3 -3
View File
@@ -10,7 +10,7 @@ import (
"time" "time"
"sneak.berlin/go/dnswatcher/internal/config" "sneak.berlin/go/dnswatcher/internal/config"
"sneak.berlin/go/dnswatcher/internal/livedns" "sneak.berlin/go/dnswatcher/internal/livednstest"
"sneak.berlin/go/dnswatcher/internal/portcheck" "sneak.berlin/go/dnswatcher/internal/portcheck"
"sneak.berlin/go/dnswatcher/internal/resolver" "sneak.berlin/go/dnswatcher/internal/resolver"
"sneak.berlin/go/dnswatcher/internal/state" "sneak.berlin/go/dnswatcher/internal/state"
@@ -195,7 +195,7 @@ func checkOnce(
return fmt.Errorf( return fmt.Errorf(
"%s: %w, or the watcher saved no fresh "+ "%s: %w, or the watcher saved no fresh "+
"result for it", "result for it",
name, livedns.ErrNoAnswer, name, livednstest.ErrNoAnswer,
) )
} }
} }
@@ -219,7 +219,7 @@ func runChecks(
var deps *testDeps var deps *testDeps
livedns.Retry(t, "watcher checks", func(ctx context.Context) error { livednstest.Retry(t, "watcher checks", func(ctx context.Context) error {
var w *watcher.Watcher var w *watcher.Watcher
w, deps = newTestWatcher(t, cfg) w, deps = newTestWatcher(t, cfg)