diff --git a/.golangci.yml b/.golangci.yml index a7a74c2..cb84cff 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -60,6 +60,8 @@ linters: desc: >- Test-support code belongs in test files and in packages whose 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 only. # Only decisions already recorded in the Go package defaults are # listed here. Every entry matches the module path exactly. gomodguard_v2: diff --git a/README.md b/README.md index 4704b23..1e6294b 100644 --- a/README.md +++ b/README.md @@ -278,7 +278,7 @@ internal/ tlscheck/tlscheck.go TLS certificate inspector notify/notify.go Notification service (Slack, Mattermost, ntfy) 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) ``` diff --git a/TESTING.md b/TESTING.md index 70e3899..6b4f3d8 100644 --- a/TESTING.md +++ b/TESTING.md @@ -25,7 +25,7 @@ real servers ensures the resolver works correctly in production. - Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus processing margin - 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 lookup that got none - Flaky failures from transient network issues are acceptable and diff --git a/TODO.md b/TODO.md index 1c50752..5f654fe 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,9 @@ Rationale, Design, TODO, License, Author) if any are still missing. # 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 replaces the deprecated `gomodguard` with `gomodguard_v2`, so `make lint` no 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 from; the resolver timeout test queries an address that never answers, and `NewFromLoggerWithClient`, used only by its stand-in client, is gone. The - live-DNS retry and concurrency limit moved to `internal/livedns`, which both - test packages use. `TESTING.md` states the README's rule (closes #159). + live-DNS retry and concurrency limit moved to `internal/livednstest`, which + 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 nameservers start to disagree or where a nameserver that disagrees first appears, instead of on every check while they disagree, and not again after diff --git a/internal/livedns/livedns.go b/internal/livednstest/livednstest.go similarity index 97% rename from internal/livedns/livedns.go rename to internal/livednstest/livednstest.go index b86794b..1554596 100644 --- a/internal/livedns/livedns.go +++ b/internal/livednstest/livednstest.go @@ -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 — // see TESTING.md. Nothing here mocks, fakes, stubs, records or replays // 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 // nothing to check looks the same as live DNS not answering, and // fails only after the last attempt. -package livedns +package livednstest import ( "context" diff --git a/internal/livedns/livedns_test.go b/internal/livednstest/livednstest_test.go similarity index 75% rename from internal/livedns/livedns_test.go rename to internal/livednstest/livednstest_test.go index 1c0f5fd..d7f2376 100644 --- a/internal/livedns/livedns_test.go +++ b/internal/livednstest/livednstest_test.go @@ -1,4 +1,4 @@ -package livedns_test +package livednstest_test import ( "context" @@ -8,7 +8,7 @@ import ( "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 @@ -21,11 +21,11 @@ func TestRetryRecoversFromTransientFailure(t *testing.T) { attempts := 0 - livedns.Retry(t, "transient", func(_ context.Context) error { + livednstest.Retry(t, "transient", func(_ context.Context) error { attempts++ if attempts < wantAttempts { - return livedns.ErrNoAnswer + return livednstest.ErrNoAnswer } return nil @@ -37,19 +37,19 @@ func TestRetryRecoversFromTransientFailure(t *testing.T) { func TestRetryGivesEachAttemptADeadline(t *testing.T) { t.Parallel() - livedns.Retry(t, "deadline", func(ctx context.Context) error { + livednstest.Retry(t, "deadline", func(ctx context.Context) error { deadline, ok := ctx.Deadline() assert.True(t, ok, "attempt should carry a 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 // deadline far shorter than intended, which would // silently turn every live attempt into an instant // timeout. - assert.Greater(t, remaining, livedns.AttemptTimeout/2) + assert.Greater(t, remaining, livednstest.AttemptTimeout/2) return nil }) @@ -73,7 +73,7 @@ func TestRunBoundsConcurrency(t *testing.T) { go func() { defer wg.Done() - _ = livedns.Run(func(_ context.Context) error { + _ = livednstest.Run(func(_ context.Context) error { mu.Lock() inFlight++ @@ -97,7 +97,7 @@ func TestRunBoundsConcurrency(t *testing.T) { assert.Positive(t, maxSeen) assert.LessOrEqual( - t, maxSeen, livedns.Concurrency, + t, maxSeen, livednstest.Concurrency, "live queries must stay under the package-wide gate", ) } diff --git a/internal/resolver/livedns_test.go b/internal/resolver/livedns_test.go index b7d1be2..fb425a8 100644 --- a/internal/resolver/livedns_test.go +++ b/internal/resolver/livedns_test.go @@ -9,7 +9,7 @@ import ( "strings" "testing" - "sneak.berlin/go/dnswatcher/internal/livedns" + "sneak.berlin/go/dnswatcher/internal/livednstest" "sneak.berlin/go/dnswatcher/internal/resolver" ) @@ -20,9 +20,9 @@ import ( // 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 // 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 -// package livedns). +// package livednstest). // // Where an assertion spans several independent nameservers, a quorum // is enough: a strict majority answering as expected. A server that @@ -162,7 +162,7 @@ func liveFindAuthoritative( var out []string - livedns.Retry( + livednstest.Retry( t, "FindAuthoritativeNameservers("+domain+")", func(ctx context.Context) error { @@ -174,7 +174,7 @@ func liveFindAuthoritative( if len(ns) == 0 { return fmt.Errorf( "%w: %s has no nameservers", - livedns.ErrNoAnswer, domain, + livednstest.ErrNoAnswer, domain, ) } @@ -198,7 +198,7 @@ func liveLookupNS( var out []string - livedns.Retry( + livednstest.Retry( t, "LookupNS("+domain+")", func(ctx context.Context) error { @@ -210,7 +210,7 @@ func liveLookupNS( if len(ns) == 0 { return fmt.Errorf( "%w: %s has no nameservers", - livedns.ErrNoAnswer, domain, + livednstest.ErrNoAnswer, domain, ) } @@ -240,7 +240,7 @@ func liveQueryNameserver( var out *resolver.NameserverResponse - livedns.Retry( + livednstest.Retry( t, what, func(ctx context.Context) error { @@ -255,7 +255,7 @@ func liveQueryNameserver( resp.Status == resolver.StatusError { return fmt.Errorf( "%w: %s returned %s: %s", - livedns.ErrNoAnswer, nameserver, + livednstest.ErrNoAnswer, nameserver, resp.Status, resp.Error, ) } @@ -282,7 +282,7 @@ func liveQueryAllNameservers( var out map[string]*resolver.NameserverResponse - livedns.Retry( + livednstest.Retry( t, "QueryAllNameservers("+hostname+")", func(ctx context.Context) error { @@ -294,7 +294,7 @@ func liveQueryAllNameservers( if len(results) == 0 { return fmt.Errorf( "%w: no nameservers queried for %s", - livedns.ErrNoAnswer, hostname, + livednstest.ErrNoAnswer, hostname, ) } @@ -327,7 +327,7 @@ func liveResolveIPs( var out []string - livedns.Retry( + livednstest.Retry( t, "ResolveIPAddresses("+hostname+")", func(ctx context.Context) error { @@ -339,7 +339,7 @@ func liveResolveIPs( if len(ips) == 0 { return fmt.Errorf( "%w: no addresses for %s", - livedns.ErrNoAnswer, hostname, + livednstest.ErrNoAnswer, hostname, ) } @@ -366,7 +366,7 @@ func liveResolveIPsAllowingEmpty( var out []string - livedns.Retry( + livednstest.Retry( t, "ResolveIPAddresses("+hostname+")", func(ctx context.Context) error { diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 4755245..28544f7 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -33,7 +33,7 @@ func newTestResolver(t *testing.T) *resolver.Resolver { // findOneNSForDomain picks one authoritative nameserver to aim a // 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( t *testing.T, r *resolver.Resolver, diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go index 5ad6030..8ff670e 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -10,7 +10,7 @@ import ( "time" "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/resolver" "sneak.berlin/go/dnswatcher/internal/state" @@ -195,7 +195,7 @@ func checkOnce( return fmt.Errorf( "%s: %w, or the watcher saved no fresh "+ "result for it", - name, livedns.ErrNoAnswer, + name, livednstest.ErrNoAnswer, ) } } @@ -219,7 +219,7 @@ func runChecks( 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 w, deps = newTestWatcher(t, cfg)