tests: rename internal/livedns to livednstest and deny it outside tests (closes #164)
check / check (push) Successful in 57s
check / check (push) Successful in 57s
The live-DNS retry and concurrency limit is only for tests, but nothing stopped program code from importing it and compiling it into the binary. Its directory name now ends in test, and its import path is on the test-support deny list in .golangci.yml, so make lint fails when a file that is not a test imports it. Every import and mention is updated to the new name. Model: opus-5-5
This commit is contained in:
@@ -60,6 +60,8 @@ 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 only.
|
||||||
# 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:
|
||||||
|
|||||||
@@ -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
@@ -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
|
||||||
|
|||||||
@@ -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",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -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 {
|
||||||
|
|||||||
@@ -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,
|
||||||
|
|||||||
@@ -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)
|
||||||
|
|||||||
Reference in New Issue
Block a user