tests: remove the DNS stand-ins from the watcher and resolver tests (closes #159)

The watcher tests used a stand-in resolver and the resolver timeout test
a stand-in DNS client, against the rule that DNS is never mocked. The
watcher tests now run the real resolver against live DNS. A change is
tested by saving values live DNS never returns (names under .invalid,
192.0.2.1) in the state a check starts from, or by marking a real
nameserver failed. The timeout test queries 192.0.2.1, where nothing
answers. The live-DNS retry and concurrency limit moved from the
resolver tests to internal/livedns, so both packages share them.
NewFromLoggerWithClient had no other use and is gone. TESTING.md and
the DNSClient comment now state the README's rule.

Model: opus-5-5

internal/resolver/dns_client.go

@@ -7,8 +7,8 @@ import (
 	"github.com/miekg/dns"
 )
 
-// DNSClient abstracts DNS wire-protocol exchanges so the resolver
-// can be tested without hitting real nameservers.
+// DNSClient sends one DNS message to a nameserver and returns the
+// reply. The resolver holds one for UDP and one for TCP.
 type DNSClient interface {
 	ExchangeContext(
 		ctx context.Context,

internal/resolver/livedns_harness_test.go

@@ -1,10 +1,7 @@
 package resolver_test
 
 import (
-	"context"
-	"sync"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 
@@ -12,9 +9,8 @@ import (
 )
 
 // Tests for the live-DNS harness in livedns_test.go itself. These
-// exercise pure logic and the retry/concurrency plumbing; they
-// perform no DNS resolution of any kind, so they neither mock DNS
-// nor depend on it.
+// exercise pure logic; they perform no DNS resolution of any kind, so
+// they neither mock DNS nor depend on it.
 
 // Names for the synthetic status maps below. Nothing is ever queried
 // at them: they are map keys handed to the package's pure counting
@@ -90,47 +86,6 @@ func TestStatusCountingIgnoresSilentNameservers(t *testing.T) {
 	)
 }
 
-func TestRetryLiveRecoversFromTransientFailure(t *testing.T) {
-	t.Parallel()
-
-	const wantAttempts = 2
-
-	attempts := 0
-
-	retryLive(t, "transient", func(_ context.Context) error {
-		attempts++
-
-		if attempts < wantAttempts {
-			return errLiveNoAnswer
-		}
-
-		return nil
-	})
-
-	assert.Equal(t, wantAttempts, attempts)
-}
-
-func TestRetryLiveGivesEachAttemptADeadline(t *testing.T) {
-	t.Parallel()
-
-	retryLive(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, liveAttemptTimeout)
-
-		// 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, liveAttemptTimeout/2)
-
-		return nil
-	})
-}
-
 // TestUnsanctionedStatusesRejectsWrongAnswers is the regression test
 // for the defect this allowlist exists to prevent: a minority of
 // nameservers answering WRONGLY while quorum keeps the suite green.
@@ -235,50 +190,3 @@ func TestUnsanctionedStatusesToleratesSilenceOnly(t *testing.T) {
 		unsanctionedStatuses(results, allowed...),
 	)
 }
-
-func TestRunLiveBoundsConcurrency(t *testing.T) {
-	t.Parallel()
-
-	const workers = 24
-
-	var (
-		mu       sync.Mutex
-		wg       sync.WaitGroup
-		inFlight int
-		maxSeen  int
-	)
-
-	wg.Add(workers)
-
-	for range workers {
-		go func() {
-			defer wg.Done()
-
-			_ = runLive(func(_ context.Context) error {
-				mu.Lock()
-				inFlight++
-
-				if inFlight > maxSeen {
-					maxSeen = inFlight
-				}
-				mu.Unlock()
-
-				time.Sleep(time.Millisecond)
-
-				mu.Lock()
-				inFlight--
-				mu.Unlock()
-
-				return nil
-			})
-		}()
-	}
-
-	wg.Wait()
-
-	assert.Positive(t, maxSeen)
-	assert.LessOrEqual(
-		t, maxSeen, liveConcurrency,
-		"live queries must stay under the package-wide gate",
-	)
-}

internal/resolver/livedns_test.go

@@ -8,8 +8,8 @@ import (
 	"sort"
 	"strings"
 	"testing"
-	"time"
 
+	"sneak.berlin/go/dnswatcher/internal/livedns"
 	"sneak.berlin/go/dnswatcher/internal/resolver"
 )
 
@@ -18,143 +18,32 @@ import (
 // ----------------------------------------------------------------
 //
 // Every test in this package resolves against the real, live DNS —
-// see TESTING.md. Nothing here mocks, fakes, stubs, records or
-// replays DNS, and nothing here skips or gates a test: the helpers
-// below only change *how* the live queries are issued, so that a
-// single dropped UDP packet or one slow authoritative server does
-// not turn a correct resolver into a red build.
+// see TESTING.md. Each live operation below goes through
+// livedns.Retry, which bounds how many resolutions are in flight at
+// once and retries an operation that got no answer (see package
+// livedns).
 //
-// Three mechanisms, all test-side:
+// Where an assertion spans several independent nameservers, a quorum
+// is enough: a strict majority answering as expected. A server that
+// fails to answer is tolerated, while a server that answers *wrongly*
+// still fails the test.
 //
-//  1. Bounded concurrency. The package's tests are parallel and the
-//     build hosts have many cores, so without a limit every test
-//     starts its own iterative resolution at the same instant and
-//     they all hit the first root server in rootServerList() within
-//     a few milliseconds of each other. Root servers rate-limit
-//     that, which shows up as a different arbitrary subset of tests
-//     failing on each run. liveGate caps how many resolutions are
-//     in flight at once.
-//
-//  2. Retry with exponential backoff. Each live operation gets
-//     several attempts with its own timeout. The retry predicate is
-//     strictly transport-level — "did a nameserver answer at all" —
-//     never the assertion the test is making. A resolver that
-//     answers incorrectly still fails on the first attempt.
-//
-//  3. Quorum. Where an assertion spans several independent
-//     nameservers, a strict majority answering as expected is
-//     enough; a server that fails to answer is tolerated, while a
-//     server that answers *wrongly* still fails the test.
-//
-// The tolerance in (3) is expressed as an ALLOWLIST of sanctioned
-// statuses, never as a blocklist of known-bad ones. A blocklist bans
-// the one wrong answer its author thought of and silently admits
-// every other status, including any added to the resolver later; an
-// allowlist fails on anything nobody explicitly sanctioned. Silence
-// (timeout, error) is the only thing quorum exists to tolerate. A
-// *wrong answer* — nxdomain for a name that exists, ok for one that
-// does not, nodata for either — is never tolerated at any count.
+// That tolerance is expressed as an ALLOWLIST of sanctioned statuses,
+// never as a blocklist of known-bad ones. A blocklist bans the one
+// wrong answer its author thought of and silently admits every other
+// status, including any added to the resolver later; an allowlist
+// fails on anything nobody explicitly sanctioned. Silence (timeout,
+// error) is the only thing quorum exists to tolerate. A *wrong
+// answer* — nxdomain for a name that exists, ok for one that does
+// not, nodata for either — is never tolerated at any count.
 
-const (
-	// liveAttempts is how many times a live DNS operation is
-	// attempted before the test fails.
-	liveAttempts = 3
+// minNameservers is the smallest nameserver count a well-run zone is
+// expected to publish.
+const minNameservers = 2
 
-	// liveAttemptTimeout bounds one attempt. Worst case for an
-	// operation is liveAttempts * liveAttemptTimeout plus the
-	// backoff — about 26 seconds, well inside the 90-second
-	// `go test -timeout` backstop even when several operations
-	// exhaust their attempts.
-	liveAttemptTimeout = 8 * time.Second
-
-	// liveBackoffBase is the delay after the first failed
-	// attempt; it is multiplied by liveBackoffFactor each time.
-	liveBackoffBase = 500 * time.Millisecond
-
-	// liveBackoffFactor is the exponential backoff multiplier.
-	liveBackoffFactor = 2
-
-	// liveConcurrency caps how many live resolutions may be in
-	// flight across the whole package at once.
-	liveConcurrency = 6
-
-	// minNameservers is the smallest nameserver count a
-	// well-run zone is expected to publish.
-	minNameservers = 2
-)
-
-// liveGate bounds concurrent live resolutions package-wide. It has
-// to be package scoped: the whole point is that it is shared by
-// every parallel test in the package.
-//
-//nolint:gochecknoglobals // package-wide live query rate limit
-var liveGate = make(chan struct{}, liveConcurrency)
-
-var (
-	// errLiveNoAnswer reports that a live operation produced no
-	// usable answer, which is retried rather than asserted on.
-	errLiveNoAnswer = errors.New("no answer from live DNS")
-
-	// errLiveNoQuorum reports that too few of a domain's
-	// nameservers answered for a quorum assertion to be made.
-	errLiveNoQuorum = errors.New("no nameserver quorum")
-)
-
-// runLive executes one attempt of a live operation, holding a slot
-// in liveGate for its duration and bounding it with its own
-// timeout.
-func runLive(op func(ctx context.Context) error) error {
-	liveGate <- struct{}{}
-	defer func() { <-liveGate }()
-
-	ctx, cancel := context.WithTimeout(
-		context.Background(), liveAttemptTimeout,
-	)
-	defer cancel()
-
-	return op(ctx)
-}
-
-// retryLive runs op until it reports success, retrying transport
-// failures with exponential backoff, and fails the test if every
-// attempt fails. op returns an error only for a failure to obtain
-// an answer — never for an answer the test disagrees with, which
-// belongs in an assertion so that it fails immediately. op stores
-// whatever it obtained where its caller can find it.
-func retryLive(
-	t *testing.T,
-	what string,
-	op func(ctx context.Context) error,
-) {
-	t.Helper()
-
-	var last error
-
-	backoff := liveBackoffBase
-
-	for attempt := range liveAttempts {
-		if attempt > 0 {
-			t.Logf(
-				"%s: attempt %d of %d failed (%v), "+
-					"retrying in %s",
-				what, attempt, liveAttempts, last, backoff,
-			)
-			time.Sleep(backoff)
-
-			backoff *= liveBackoffFactor
-		}
-
-		last = runLive(op)
-		if last == nil {
-			return
-		}
-	}
-
-	t.Fatalf(
-		"%s: no answer after %d live attempts: %v",
-		what, liveAttempts, last,
-	)
-}
+// errLiveNoQuorum reports that too few of a domain's nameservers
+// answered for a quorum assertion to be made.
+var errLiveNoQuorum = errors.New("no nameserver quorum")
 
 // liveQuorum is how many of total nameservers must agree for a
 // multi-nameserver assertion to hold: a strict majority.
@@ -272,7 +161,7 @@ func liveFindAuthoritative(
 
 	var out []string
 
-	retryLive(
+	livedns.Retry(
 		t,
 		"FindAuthoritativeNameservers("+domain+")",
 		func(ctx context.Context) error {
@@ -284,7 +173,7 @@ func liveFindAuthoritative(
 			if len(ns) == 0 {
 				return fmt.Errorf(
 					"%w: %s has no nameservers",
-					errLiveNoAnswer, domain,
+					livedns.ErrNoAnswer, domain,
 				)
 			}
 
@@ -308,7 +197,7 @@ func liveLookupNS(
 
 	var out []string
 
-	retryLive(
+	livedns.Retry(
 		t,
 		"LookupNS("+domain+")",
 		func(ctx context.Context) error {
@@ -320,7 +209,7 @@ func liveLookupNS(
 			if len(ns) == 0 {
 				return fmt.Errorf(
 					"%w: %s has no nameservers",
-					errLiveNoAnswer, domain,
+					livedns.ErrNoAnswer, domain,
 				)
 			}
 
@@ -350,7 +239,7 @@ func liveQueryNameserver(
 
 	var out *resolver.NameserverResponse
 
-	retryLive(
+	livedns.Retry(
 		t,
 		what,
 		func(ctx context.Context) error {
@@ -365,7 +254,7 @@ func liveQueryNameserver(
 				resp.Status == resolver.StatusError {
 				return fmt.Errorf(
 					"%w: %s returned %s: %s",
-					errLiveNoAnswer, nameserver,
+					livedns.ErrNoAnswer, nameserver,
 					resp.Status, resp.Error,
 				)
 			}
@@ -392,7 +281,7 @@ func liveQueryAllNameservers(
 
 	var out map[string]*resolver.NameserverResponse
 
-	retryLive(
+	livedns.Retry(
 		t,
 		"QueryAllNameservers("+hostname+")",
 		func(ctx context.Context) error {
@@ -404,7 +293,7 @@ func liveQueryAllNameservers(
 			if len(results) == 0 {
 				return fmt.Errorf(
 					"%w: no nameservers queried for %s",
-					errLiveNoAnswer, hostname,
+					livedns.ErrNoAnswer, hostname,
 				)
 			}
 
@@ -437,7 +326,7 @@ func liveResolveIPs(
 
 	var out []string
 
-	retryLive(
+	livedns.Retry(
 		t,
 		"ResolveIPAddresses("+hostname+")",
 		func(ctx context.Context) error {
@@ -449,7 +338,7 @@ func liveResolveIPs(
 			if len(ips) == 0 {
 				return fmt.Errorf(
 					"%w: no addresses for %s",
-					errLiveNoAnswer, hostname,
+					livedns.ErrNoAnswer, hostname,
 				)
 			}
 
@@ -476,7 +365,7 @@ func liveResolveIPsAllowingEmpty(
 
 	var out []string
 
-	retryLive(
+	livedns.Retry(
 		t,
 		"ResolveIPAddresses("+hostname+")",
 		func(ctx context.Context) error {

internal/resolver/resolver.go

@@ -67,17 +67,4 @@ func NewFromLogger(log *slog.Logger) *Resolver {
 	}
 }
 
-// NewFromLoggerWithClient creates a Resolver with a custom DNS
-// client, useful for testing with mock DNS responses.
-func NewFromLoggerWithClient(
-	log *slog.Logger,
-	client DNSClient,
-) *Resolver {
-	return &Resolver{
-		log:    log,
-		client: client,
-		tcp:    client,
-	}
-}
-
 // Method implementations are in iterative.go.

internal/resolver/resolver_test.go

@@ -10,7 +10,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -526,21 +525,16 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) {
 func TestQueryNameserverIP_Timeout(t *testing.T) {
 	t.Parallel()
 
-	log := slog.New(slog.NewTextHandler(
-		os.Stderr,
-		&slog.HandlerOptions{Level: slog.LevelDebug},
-	))
-
-	r := resolver.NewFromLoggerWithClient(
-		log, &timeoutClient{},
-	)
+	r := newTestResolver(t)
 
+	// Nothing answers at 192.0.2.1, a documentation address. The
+	// deadline must outlast one query timeout (two seconds): a query
+	// cut short by the deadline itself is not reported as a timeout.
 	ctx, cancel := context.WithTimeout(
-		context.Background(), 10*time.Second,
+		context.Background(), 3*time.Second,
 	)
 	t.Cleanup(cancel)
 
-	// Query any IP — the client always returns a timeout error.
 	resp, err := r.QueryNameserverIP(
 		ctx, "unreachable.test.", "192.0.2.1",
 		"example.com",
@@ -551,27 +545,6 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
 	assert.NotEmpty(t, resp.Error)
 }
 
-// timeoutClient simulates DNS timeout errors for testing.
-type timeoutClient struct{}
-
-func (c *timeoutClient) ExchangeContext(
-	_ context.Context,
-	_ *dns.Msg,
-	_ string,
-) (*dns.Msg, time.Duration, error) {
-	return nil, 0, &net.OpError{
-		Op:  "read",
-		Net: "udp",
-		Err: &timeoutError{},
-	}
-}
-
-type timeoutError struct{}
-
-func (e *timeoutError) Error() string   { return "i/o timeout" }
-func (e *timeoutError) Timeout() bool   { return true }
-func (e *timeoutError) Temporary() bool { return true }
-
 func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
 	t.Parallel()