test: restore transport-failure coverage with loopback nameservers

The DNS-mock removal deleted TestQueryNameserverIP_Timeout and left a
comment in its place, so the resolver's StatusTimeout / StatusError
classification branch went untested. The stated obstacle was that a
query to a black-holed RFC 5737 address comes back StatusOK, because
the build environment transparently intercepts UDP/53 and answers it
locally. That is a property of that environment, not of the resolver,
and it only rules out choosing a remote address.

internal/resolver/transport_test.go binds real nameservers on
127.0.0.1 instead and aims the query at them: one silent on A queries
and answering every other type (StatusTimeout), one answering SERVFAIL
(StatusError), and one address with nothing listening, which is
refused rather than dropped and so classifies as NoData. This is not a
mock — no DNSClient is substituted. The resolver dials a real socket,
writes a real query with the real miekg/dns client, and applies its
real deadline and real classification logic to what comes back.
Substituting the client is what TESTING.md bans; choosing the server
is not, and the resolver is aimed at a caller-chosen nameserver in
production too.

queryDNS now dials a nameserver address that already carries a port as
written, defaulting to 53 only for a bare address. That is what makes
a nameserver on any other port reachable, on loopback or otherwise.

Silence on one record type rather than all eight keeps the timeout
test to two query timeouts (4s) instead of sixteen (32s), and it is
asserted: the test fails if it ever costs more than 8s.

The watcher's assertStatePopulated and TestDomainPortAndTLSChecks
asserted only that hostname, port and certificate state were
non-empty, plus non-zero checker call counts. Neither was vacuous, but
neither would have caught the watcher resolving the wrong addresses.
The port and TLS test doubles now record their arguments, and both
tests assert that the state keys and the arguments the checkers were
actually called with match the addresses live DNS returned — exactly
those, no more and no fewer. Verified by mutation: making the watcher
drop all but one resolved address fails both tests, and it passed both
of them before.

TESTING.md records why a loopback nameserver is not a mock, so the new
tests are not mistaken for a violation of the rule they respect.

TESTING.md

@@ -2,8 +2,10 @@
 
 ## DNS Resolution Tests
 
-All resolver tests **MUST** use live queries against real DNS servers.
-No mocking of the DNS client layer is permitted.
+All tests that involve DNS resolution — in every package, including
+consumers of the resolver such as the watcher — **MUST** use live
+queries against real DNS servers. No mocking, faking, or stubbing of
+DNS at any layer is permitted.
 
 ### Rationale
 
@@ -12,6 +14,8 @@ the full delegation chain. Mocked responses cannot faithfully represent
 the variety of real-world DNS behavior (truncation, referrals, glue
 records, DNSSEC, varied response times, EDNS, etc.). Testing against
 real servers ensures the resolver works correctly in production.
+Robustness comes from handling real-world DNS behavior with tolerant
+assertions and sensible timeouts, not from mocks.
 
 ### Constraints
 
@@ -24,14 +28,64 @@ real servers ensures the resolver works correctly in production.
 - Flaky failures from transient network issues are acceptable and
   should be investigated as potential resolver bugs, not papered over
   with mocks or skip flags
+- Watcher change-detection tests seed a synthetic *previous state*
+  and compare it against fresh live lookups; the DNS side is never
+  faked
+- Live query concurrency is bounded per package (`liveGate` in
+  `internal/resolver`, `liveWatcherGate` in `internal/watcher`) so
+  parallel tests do not burst at the root servers
+- Those gates are package-scoped and therefore per test binary, so
+  `script/test` also passes `-p 1`: with both live-DNS packages
+  running at once the gates sum instead of holding, and the resolver's
+  per-attempt deadlines start expiring
+
+### Transport failures: loopback nameservers, not mocks
+
+The resolver classifies a nameserver that stays silent as
+`StatusTimeout` and one that answers SERVFAIL as `StatusError`. The
+public network cannot be made to produce either on demand — a
+black-holed address is only black-holed on some networks, and build
+environments that transparently intercept UDP/53 answer it locally —
+so a test built on a chosen remote address asserts on the network it
+happens to run on rather than on the resolver.
+
+`internal/resolver/transport_test.go` binds a real nameserver on
+`127.0.0.1` instead and points the query at it.
+`nameserverAddr` dials an address that already carries a port as
+written, so no production behaviour is bypassed to arrange this.
+
+**This is permitted, and it is not a mock.** The rule above bans
+substituting `DNSClient` or any other DNS abstraction, which lets the
+code under test skip DNS and hands it a manufactured verdict. A
+loopback nameserver does the opposite: the resolver dials a real
+socket, writes a real query with the real `miekg/dns` client, and
+applies its real deadline and its real classification logic to what
+comes back. Choosing which nameserver a live query is sent to is not
+faking DNS — the resolver is aimed at a nameserver of the caller's
+choosing in production too.
+
+The distinction to hold on to: **substituting the client is banned;
+choosing the server is not.** A test that reaches for a fake
+`DNSClient` to force a classification is still forbidden, no matter
+how awkward the alternative looks.
+
+Such a test must stay cheap. The resolver asks for eight record
+types and retries each once, so a nameserver silent on every type
+costs sixteen query timeouts. `TestQueryNameserverIP_Timeout` is
+silent on `A` alone and answers the rest, which is all the resolver
+needs to classify the response and keeps the test to two.
 
 ### What NOT to do
 
-- **Do not mock `DNSClient`** for resolver tests (the mock constructor
-  exists for unit-testing other packages that consume the resolver)
+- **Do not mock `DNSClient`**, the watcher's `DNSResolver` interface,
+  or any other DNS abstraction — in any package, for any reason
 - **Do not add `-short` flags** to skip slow tests
 - **Do not increase `-timeout`** to hide hanging queries
 - **Do not remove `-count=1` from `script/test`** — Go's test cache
   replays a previous run's output without querying anything, so a
   cached pass is not evidence that live resolution works
+- **Do not remove `-p 1` from `script/test`** — running the live-DNS
+  packages in parallel oversubscribes live DNS past what their gates
+  bound, which surfaces as unrelated resolver tests failing on
+  expired deadlines
 - **Do not modify linter configuration** to suppress findings

TODO.md

@@ -14,7 +14,10 @@ pre-1.0. No git tags. Core resolver work in flight on feature/resolver
 (dirty: internal/resolver/resolver_test.go). Local checkout has diverged
 from origin: origin/main is 8 commits ahead (watcher orchestrator,
 unified TARGETS) and origin/feature/resolver already contains the full
-iterative resolver implementation with hermetic mocked tests.
+iterative resolver implementation. DNS mocking is banned in this repo
+(see `TESTING.md`): all tests use live DNS only. The hermetic mocked
+tests previously noted on `feature/resolver` are gone from its current
+tip, which carries a live-DNS suite against `*.dns.sneak.cloud`.
 
 # Next Step
 
@@ -23,6 +26,22 @@ Rationale, Design, TODO, License, Author) if any are still missing.
 
 # Completed Steps
 
+- 2026-09-03: restored the transport-failure classification coverage
+  the DNS-mock removal had dropped, and tightened two over-tolerant
+  watcher assertions. `internal/resolver/transport_test.go` covers
+  `StatusTimeout`, `StatusError` and the connection-refused path by
+  binding real nameservers on loopback rather than by mocking
+  `DNSClient` or by aiming a query at a remote address and hoping the
+  network black-holes it; `queryDNS` now honours a port already
+  present in a nameserver address, which is what lets a query be
+  aimed at one. The watcher's `assertStatePopulated` and
+  `TestDomainPortAndTLSChecks` now assert that port and certificate
+  state, and the arguments the port and TLS checkers were called
+  with, match the addresses live DNS returned — previously they
+  asserted only that those sets were non-empty, which would not have
+  caught resolving the wrong addresses. `TESTING.md` records why a
+  loopback nameserver is not a mock.
+
 - 2026-08-10: comment-only corrections to `script/bootstrap`,
   `script/cibuild`, and `Dockerfile.lint`. The `goimports` pin in
   `script/bootstrap` was justified by a claim that `script/fmt-check`
@@ -92,6 +111,10 @@ Rationale, Design, TODO, License, Author) if any are still missing.
   `make check` into `script/lint`. `golangci-lint config verify` is
   deliberately omitted: it fetches its schema over an unpinned live
   HTTPS call
+- 2026-08-07: DNS mocking removed from the entire test suite; watcher
+  tests now drive the real iterative resolver against live DNS and
+  `TESTING.md` bans DNS mocks in every package (`remove-dns-mocking`
+  branch)
 - 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
   in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
   org-standard v2-schema config used across the org's repos
@@ -103,7 +126,8 @@ Rationale, Design, TODO, License, Author) if any are still missing.
 - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
   Makefile shims, README Entrypoints section
 - 2026-02-20: iterative DNS resolver implemented; tests made hermetic
-  with mocked DNS (origin/feature/resolver, unmerged)
+  with mocked DNS (origin/feature/resolver, unmerged; superseded — DNS
+  mocking is banned, see `TESTING.md`)
 - 2026-02-20: CI actions and go install refs pinned to commit SHAs;
   Gitea Actions workflow for make check (origin/ci/make-check, unmerged)
 - 2026-02-20: watcher monitoring orchestrator merged to main (#8)
@@ -128,8 +152,9 @@ Branch reconciliation:
 - Sync local checkout with origin: local main is 8 commits behind
   origin/main; local feature/resolver has diverged from
   origin/feature/resolver, which already implements the resolver
-- Merge in-flight branches to main once green: feature/resolver,
-  ci/make-check, feature/portcheck-implementation,
+- Merge in-flight branches to main once green: feature/resolver
+  (confirm its tests remain live-DNS — DNS mocking is banned, see
+  `TESTING.md`), ci/make-check, feature/portcheck-implementation,
   feature/tlscheck-implementation
 
 Resolver (plan from untracked TODO.md; largely implemented on
@@ -213,6 +238,7 @@ Infrastructure notes (from untracked TODO.md):
 - Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de
   remote intentionally; do not "fix" it
 - Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix
-- Resolver tests originally used live DNS against *.dns.sneak.cloud
-  (required records documented in the test file header); origin now has
-  mocked hermetic tests, keep them hermetic
+- Resolver tests originally used live DNS against `*.dns.sneak.cloud`
+  (required records documented in the test file header); `main` now
+  tests against live public DNS. DNS mocking is banned (see
+  `TESTING.md`); never reintroduce hermetic mocked DNS tests

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 abstracts DNS wire-protocol exchanges over a single
+// transport, letting the resolver switch between UDP and TCP.
 type DNSClient interface {
 	ExchangeContext(
 		ctx context.Context,

internal/resolver/iterative.go

@@ -20,6 +20,10 @@ const (
 	minDomainLabels      = 2
 )
 
+// defaultDNSPort is the port a nameserver is assumed to listen on
+// when its address does not carry one.
+const defaultDNSPort = "53"
+
 // ErrRefused is returned when a DNS server refuses a query.
 var ErrRefused = errors.New("dns query refused")
 
@@ -106,6 +110,19 @@ func (r *Resolver) retryTCP(
 	return resp
 }
 
+// nameserverAddr renders a nameserver address for dialling. A bare
+// address — the normal case, and what a delegation's glue records
+// carry — is given the default DNS port. An address that already
+// specifies a port is dialled as written, which is what makes a
+// nameserver listening somewhere other than 53 reachable.
+func nameserverAddr(nsIP string) string {
+	if _, _, err := net.SplitHostPort(nsIP); err == nil {
+		return nsIP
+	}
+
+	return net.JoinHostPort(nsIP, defaultDNSPort)
+}
+
 // queryDNS sends a DNS query to a specific server IP.
 // Tries non-recursive first, falls back to recursive on
 // REFUSED (handles DNS interception environments).
@@ -120,7 +137,7 @@ func (r *Resolver) queryDNS(
 	}
 
 	name = dns.Fqdn(name)
-	addr := net.JoinHostPort(serverIP, "53")
+	addr := nameserverAddr(serverIP)
 
 	msg := new(dns.Msg)
 	msg.SetQuestion(name, qtype)

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

@@ -8,9 +8,7 @@ import (
 	"sort"
 	"strings"
 	"testing"
-	"time"
 
-	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -519,58 +517,9 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) {
 	assert.Error(t, err)
 }
 
-// ----------------------------------------------------------------
-// Timeout tests
-// ----------------------------------------------------------------
-
-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{},
-	)
-
-	ctx, cancel := context.WithTimeout(
-		context.Background(), 10*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",
-	)
-	require.NoError(t, err)
-
-	assert.Equal(t, resolver.StatusTimeout, resp.Status)
-	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 }
+// Transport-failure classification (StatusTimeout / StatusError)
+// is covered in transport_test.go, against real nameservers bound on
+// loopback.
 
 func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
 	t.Parallel()

internal/resolver/transport_test.go

@@ -0,0 +1,253 @@
+package resolver_test
+
+// Transport-failure classification tests.
+//
+// These are live tests, not mocks. Nothing here substitutes the
+// resolver's DNSClient: the resolver dials a real UDP socket, writes
+// a real DNS query with the real miekg/dns client, and applies its
+// real deadline and its real classification logic to what comes
+// back. The only thing under test control is which address the query
+// is sent to, and what — if anything — is listening there.
+//
+// That distinction is what the no-DNS-mocks rule in TESTING.md is
+// about. A fake DNSClient lets the code under test skip DNS entirely
+// and hands it a manufactured verdict; a nameserver bound on
+// loopback makes it speak DNS for real and earn one. Pointing a live
+// query at a nameserver of the test's choosing is no more a mock
+// than pointing it at a.root-servers.net.
+//
+// The public network cannot produce these outcomes on demand. A
+// black-holed address is not black-holed everywhere — build
+// environments that intercept UDP/53 answer it locally — so a test
+// built on one asserts on the network it happens to run on rather
+// than on the resolver. A loopback nameserver is deterministic
+// everywhere, and it is fast, because the test picks the deadline.
+
+import (
+	"context"
+	"net"
+	"testing"
+	"time"
+
+	"github.com/miekg/dns"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"sneak.berlin/go/dnswatcher/internal/resolver"
+)
+
+const (
+	// transportBudget is the wall time the timeout test must stay
+	// under. The resolver asks a nameserver for eight record types
+	// in turn and retries each one once, so a nameserver silent on
+	// every type would cost sixteen query timeouts. The test's
+	// nameserver is silent on exactly one type, which costs two,
+	// and this budget fails loudly if that ever stops being true.
+	transportBudget = 8 * time.Second
+
+	// transportDeadline is the caller deadline the tests run
+	// under. It is generous on purpose: these tests are about the
+	// resolver classifying a nameserver's behaviour, so the
+	// caller's deadline must never be the thing that expires.
+	transportDeadline = 30 * time.Second
+
+	// silentNS and failingNS are the nameserver names reported
+	// back in NameserverResponse.Nameserver. They are .test names
+	// (RFC 6761) and are never resolved: the tests address the
+	// nameserver by its socket address.
+	silentNS  = "silent.ns.test."
+	failingNS = "servfail.ns.test."
+
+	// transportHostname is the name queried. Nothing resolves it;
+	// the point is entirely how the nameserver behaves.
+	transportHostname = "example.com"
+)
+
+// startNameserver binds a real UDP nameserver on loopback and serves
+// every datagram it receives with handle, which returns the reply to
+// send or nil to stay silent. It returns the "host:port" address to
+// aim a query at, and stops the server when the test ends.
+func startNameserver(
+	t *testing.T,
+	handle func(query *dns.Msg) *dns.Msg,
+) string {
+	t.Helper()
+
+	conn, err := net.ListenPacket("udp", "127.0.0.1:0")
+	require.NoError(t, err, "binding loopback nameserver")
+
+	stopped := make(chan struct{})
+
+	t.Cleanup(func() {
+		_ = conn.Close()
+		<-stopped
+	})
+
+	go serveNameserver(conn, handle, stopped)
+
+	return conn.LocalAddr().String()
+}
+
+// serveNameserver reads queries until conn is closed, replying with
+// whatever handle produces.
+func serveNameserver(
+	conn net.PacketConn,
+	handle func(query *dns.Msg) *dns.Msg,
+	stopped chan<- struct{},
+) {
+	defer close(stopped)
+
+	buf := make([]byte, dns.MaxMsgSize)
+
+	for {
+		n, from, err := conn.ReadFrom(buf)
+		if err != nil {
+			return
+		}
+
+		query := new(dns.Msg)
+		if query.Unpack(buf[:n]) != nil {
+			continue
+		}
+
+		reply := handle(query)
+		if reply == nil {
+			continue
+		}
+
+		wire, err := reply.Pack()
+		if err != nil {
+			continue
+		}
+
+		if _, err := conn.WriteTo(wire, from); err != nil {
+			return
+		}
+	}
+}
+
+// unservedAddr returns a loopback address with nothing listening on
+// it, by binding a port and releasing it again.
+func unservedAddr(t *testing.T) string {
+	t.Helper()
+
+	conn, err := net.ListenPacket("udp", "127.0.0.1:0")
+	require.NoError(t, err, "binding loopback port")
+
+	addr := conn.LocalAddr().String()
+	require.NoError(t, conn.Close(), "releasing loopback port")
+
+	return addr
+}
+
+// TestQueryNameserverIP_Timeout covers the StatusTimeout branch: a
+// nameserver that takes the query and never answers.
+func TestQueryNameserverIP_Timeout(t *testing.T) {
+	t.Parallel()
+
+	// A real nameserver that drops A queries and answers every
+	// other type. Silence on one type is all the resolver needs to
+	// classify the response as a timeout, and it keeps the test
+	// two query timeouts long instead of sixteen.
+	addr := startNameserver(t, func(query *dns.Msg) *dns.Msg {
+		if len(query.Question) > 0 &&
+			query.Question[0].Qtype == dns.TypeA {
+			return nil
+		}
+
+		reply := new(dns.Msg)
+		reply.SetReply(query)
+
+		return reply
+	})
+
+	r := newTestResolver(t)
+
+	ctx, cancel := context.WithTimeout(
+		context.Background(), transportDeadline,
+	)
+	defer cancel()
+
+	start := time.Now()
+
+	resp, err := r.QueryNameserverIP(
+		ctx, silentNS, addr, transportHostname,
+	)
+
+	elapsed := time.Since(start)
+
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+
+	assert.Equal(t, resolver.StatusTimeout, resp.Status)
+	assert.Equal(t, "all queries timed out", resp.Error)
+	assert.Empty(t, resp.Records)
+	assert.Equal(t, silentNS, resp.Nameserver)
+
+	assert.Less(
+		t, elapsed, transportBudget,
+		"one silent record type must cost one query's retries, "+
+			"not every record type's",
+	)
+}
+
+// TestQueryNameserverIP_ServFail covers the StatusError branch: a
+// nameserver that answers, and answers SERVFAIL.
+func TestQueryNameserverIP_ServFail(t *testing.T) {
+	t.Parallel()
+
+	addr := startNameserver(t, func(query *dns.Msg) *dns.Msg {
+		reply := new(dns.Msg)
+		reply.SetRcode(query, dns.RcodeServerFailure)
+
+		return reply
+	})
+
+	r := newTestResolver(t)
+
+	ctx, cancel := context.WithTimeout(
+		context.Background(), transportDeadline,
+	)
+	defer cancel()
+
+	resp, err := r.QueryNameserverIP(
+		ctx, failingNS, addr, transportHostname,
+	)
+
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+
+	assert.Equal(t, resolver.StatusError, resp.Status)
+	assert.Equal(t, "server returned SERVFAIL", resp.Error)
+	assert.Empty(t, resp.Records)
+	assert.Equal(t, failingNS, resp.Nameserver)
+}
+
+// TestQueryNameserverIP_NoListener pins the third transport outcome:
+// a refused datagram is not a timeout. The socket fails immediately
+// with ECONNREFUSED rather than going quiet, so isTimeout is false,
+// no failure flag is set, and the response classifies as NoData.
+// Asserting it here is what stops that path being mistaken for the
+// timeout path, in either direction.
+func TestQueryNameserverIP_NoListener(t *testing.T) {
+	t.Parallel()
+
+	addr := unservedAddr(t)
+
+	r := newTestResolver(t)
+
+	ctx, cancel := context.WithTimeout(
+		context.Background(), transportDeadline,
+	)
+	defer cancel()
+
+	resp, err := r.QueryNameserverIP(
+		ctx, silentNS, addr, transportHostname,
+	)
+
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+
+	assert.Equal(t, resolver.StatusNoData, resp.Status)
+	assert.Empty(t, resp.Records)
+}

script/test

@@ -19,15 +19,26 @@
 #
 # -timeout 90s is a deliberate backstop above the 60s hard cap on
 # suite duration. Do not lower it.
+#
+# -p 1 runs one test package at a time, and is load-bearing. Live DNS
+# is a resource outside the process: the concurrency gates that keep
+# this suite from bursting at the root servers
+# (internal/resolver/livedns_test.go, internal/watcher/watcher_test.go)
+# are package-scoped, so each one only bounds its own test binary. Go
+# runs package binaries in parallel by default, so with both live-DNS
+# packages in flight at once their gates sum instead of holding, the
+# root and TLD servers rate-limit the excess, and the resolver
+# package's per-attempt deadlines expire. Serialising packages is what
+# makes each gate authoritative while its package runs.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
 
 main() {
     cd "$ROOT"
-    go test -count=1 -race -timeout 90s -cover ./... || {
+    go test -count=1 -p 1 -race -timeout 90s -cover ./... || {
         echo "--- Rerunning with -v for details ---" >&2
-        go test -count=1 -race -timeout 90s -v ./... || true
+        go test -count=1 -p 1 -race -timeout 90s -v ./... || true
         exit 1
     }
 }