watcher tests: fewer queries per domain check, longer live attempts (closes #214)

ResolveIPAddresses, which a domain check runs for each nameserver, asked
every nameserver of the name's zone for all eight record types and read
only A, AAAA and CNAME. It now asks for those three, so a domain check
of example.com sends about a third fewer queries.

The per-attempt deadline in livednstest goes from 8 to 18 seconds. It
was sized for one lookup, while a watcher check sends over a hundred
queries in a row and on the CI runner ran past 8 seconds. Three attempts
still end under the 60-second cap.

A nameserver that answers none of the three queries now counts as not
answering even if it would have answered another type; the watcher keeps
the previous addresses either way.

Model: opus-5-5

TODO.md

@@ -19,8 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
 
 # Completed Steps
 
-- 2026-10-02: watcher tests send far fewer queries and a live attempt may take
-  18s; nameserver addresses are asked only for A, AAAA, CNAME (closes #214).
+- 2026-10-02: a nameserver's addresses are looked up asking only for A, AAAA and
+  CNAME records, and a live test attempt may take 18s, not 8s (closes #214).
 - 2026-10-02: the resolver tries root servers, and every other server list it
   walks, in a random order each time, not always from the top (closes #138).
 - 2026-10-02: a name listed more than once in `DNSWATCHER_TARGETS`, in any

internal/livednstest/livednstest.go

@@ -39,14 +39,10 @@ const (
 	// AttemptTimeout bounds one attempt. It must fit the longest
 	// operation, a watcher check, which sends over a hundred queries one
 	// after another and on a slow build host takes several times as long
-	// as the few seconds it takes on a fast one. An operation whose
-	// every attempt fails takes attempts * AttemptTimeout plus the
-	// backoff, about 56 seconds, after it waits for one of the
-	// Concurrency slots that every live operation in the test binary
-	// shares. So when live DNS does not answer at all, a test binary
-	// with more live operations than slots runs into the 90-second
-	// `go test -timeout` backstop instead of each test failing on its
-	// own.
+	// as the few seconds it takes on a fast one. Worst case for an
+	// operation is attempts * AttemptTimeout plus the backoff — about 56
+	// seconds, under the suite's 60-second cap and inside the 90-second
+	// `go test -timeout` backstop.
 	AttemptTimeout = 18 * time.Second
 
 	// backoffBase is the delay after the first failed attempt; it is

internal/resolver/resolver_test.go

@@ -562,34 +562,6 @@ func TestResolveIPAddresses_CloudflareDomain(t *testing.T) {
 	assert.NotEmpty(t, ips)
 }
 
-// TestResolveIPAddresses_NameserverIPv4AndIPv6 looks up the addresses of
-// one of cloudflare.com's nameservers, as a domain check does for each
-// nameserver. That name has A and AAAA records, so both kinds of address
-// come back.
-func TestResolveIPAddresses_NameserverIPv4AndIPv6(t *testing.T) {
-	t.Parallel()
-
-	r := newTestResolver(t)
-	ns := findOneNSForDomain(t, r, "cloudflare.com")
-	ips := liveResolveIPs(t, r, ns)
-
-	var ipv4, ipv6 int
-
-	for _, ip := range ips {
-		parsed := net.ParseIP(ip)
-		require.NotNil(t, parsed, "should be valid IP: %s", ip)
-
-		if parsed.To4() != nil {
-			ipv4++
-		} else {
-			ipv6++
-		}
-	}
-
-	assert.Positive(t, ipv4, "no IPv4 address for %s: %v", ns, ips)
-	assert.Positive(t, ipv6, "no IPv6 address for %s: %v", ns, ips)
-}
-
 // ----------------------------------------------------------------
 // Context cancellation tests
 // ----------------------------------------------------------------

internal/watcher/watcher_test.go

@@ -26,22 +26,18 @@ import (
 
 // The watcher looks these names up in live DNS with the real resolver,
 // so tests assert on what the watcher does with the answers, never on
-// the records these zones publish. The nameservers of testHost and
-// testSmallDomain stay the same between a test looking them up and its
-// check. Every query a check sends is one more that can be lost, so the
-// tests keep them few. A check asks each of a name's nameservers about
-// every record type, and both names have two. A domain check also looks
-// up each nameserver's addresses at every nameserver of the zone that
-// nameserver is in: testSmallDomain's nameservers are in zones with two
-// nameservers, while a domain whose nameservers are in, say,
-// cloudflare.com, which has five, makes each domain check much longer.
-// A test checks a domain only when it is about domains, and checks once,
-// from saved state it builds, rather than twice. The tests that query
-// testDomain's nameservers directly do no domain check.
+// the records these zones publish. testHost's nameservers and addresses
+// stay the same from one check to the next, which the tests that check
+// it twice rely on, and testSmallDomain's nameservers stay the same
+// between a test looking them up and its check. A domain check looks up
+// each nameserver's addresses, about a second per nameserver, so the
+// tests that check a domain use testSmallDomain, which has two
+// nameservers, and check it once. The tests that query testDomain's
+// nameservers directly do no domain check.
 const (
 	testDomain      = "google.com"
-	testSmallDomain = "desec.io"
-	testHost        = "example.org"
+	testSmallDomain = "example.com"
+	testHost        = "cloudflare.com"
 	testIssuer      = "DigiCert"
 )
 
@@ -263,48 +259,55 @@ func checkOnce(
 
 // runChecks builds a watcher, lets prepare set up the saved state and
 // stand-ins it starts from, and runs its checks once against live DNS.
-// When the check finds no fresh address for a name (see checkOnce), the
-// watcher is thrown away and all of this runs again on a new one, so a
-// failed attempt leaves nothing behind in the saved state, the
-// stand-ins or the notifications.
+// If change is not nil, change then alters the saved state or stand-ins
+// and the checks run a second time. When either check finds no fresh
+// address for a name (see checkOnce), the watcher is thrown away and
+// all of this runs again on a new one, so a failed attempt leaves
+// nothing behind in the saved state, the stand-ins or the notifications.
 func runChecks(
 	t *testing.T,
 	cfg *config.Config,
-	prepare func(deps *testDeps),
-) (*watcher.Watcher, *testDeps) {
+	prepare, change func(deps *testDeps),
+) *testDeps {
 	t.Helper()
 
-	var (
-		w    *watcher.Watcher
-		deps *testDeps
-	)
+	var deps *testDeps
 
 	livednstest.Retry(t, "watcher checks", func(ctx context.Context) error {
+		var w *watcher.Watcher
+
 		w, deps = newTestWatcher(t, cfg)
 
 		if prepare != nil {
 			prepare(deps)
 		}
 
+		err := checkOnce(ctx, w, deps)
+		if err != nil || change == nil {
+			return err
+		}
+
+		change(deps)
+
 		return checkOnce(ctx, w, deps)
 	})
 
-	return w, deps
+	return deps
 }
 
-// lookupNameservers returns the nameservers live DNS lists for name,
+// lookupNameservers returns the nameservers live DNS lists for domain,
 // for a test to save in the state its check starts from.
-func lookupNameservers(t *testing.T, name string) []string {
+func lookupNameservers(t *testing.T, domain string) []string {
 	t.Helper()
 
 	res := resolver.NewFromLogger(slog.Default())
 
 	var nameservers []string
 
-	livednstest.Retry(t, "LookupNS("+name+")", func(ctx context.Context) error {
+	livednstest.Retry(t, "LookupNS("+domain+")", func(ctx context.Context) error {
 		var err error
 
-		nameservers, err = res.LookupNS(ctx, name)
+		nameservers, err = res.LookupNS(ctx, domain)
 
 		return err
 	})
@@ -367,7 +370,7 @@ func TestFirstRunBaseline(t *testing.T) {
 	cfg.Domains = []string{testSmallDomain}
 	cfg.Hostnames = []string{testHost}
 
-	_, deps := runChecks(t, cfg, nil)
+	deps := runChecks(t, cfg, nil, nil)
 
 	assertNoNotifications(t, deps)
 	assertStatePopulated(t, deps)
@@ -419,7 +422,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Domains = []string{testSmallDomain}
 
-	_, deps := runChecks(t, cfg, nil)
+	deps := runChecks(t, cfg, nil, nil)
 
 	snap := deps.state.GetSnapshot()
 
@@ -459,11 +462,11 @@ func TestNSChangeDetection(t *testing.T) {
 	cfg.Domains = []string{testSmallDomain}
 
 	// The saved state lists nameservers that live DNS does not.
-	_, deps := runChecks(t, cfg, func(deps *testDeps) {
+	deps := runChecks(t, cfg, func(deps *testDeps) {
 		deps.state.SetDomainState(testSmallDomain, &state.DomainState{
 			Nameservers: []string{oldNS1, oldNS2},
 		})
-	})
+	}, nil)
 
 	assertNotified(t, deps, "NS Change: "+testSmallDomain, "warning")
 
@@ -483,7 +486,7 @@ func TestNSAddressChangeDetection(t *testing.T) {
 
 	// The saved state lists the nameservers live DNS lists, each at an
 	// address live DNS never returns.
-	_, deps := runChecks(t, cfg, func(deps *testDeps) {
+	deps := runChecks(t, cfg, func(deps *testDeps) {
 		nsAddresses := make(map[string][]string, len(nameservers))
 		for _, ns := range nameservers {
 			nsAddresses[ns] = []string{oldIP}
@@ -493,7 +496,7 @@ func TestNSAddressChangeDetection(t *testing.T) {
 			Nameservers:         nameservers,
 			NameserverAddresses: nsAddresses,
 		})
-	})
+	}, nil)
 
 	title := "NS Address Change: " + testSmallDomain
 	ds, _ := deps.state.GetDomainState(testSmallDomain)
@@ -539,12 +542,12 @@ func TestNSAddedAndRemovedIsNoAddressChange(t *testing.T) {
 	// The saved state lists oldNS1, which live DNS does not, in place of
 	// the first nameserver live DNS lists, so that the check finds that
 	// one added and oldNS1 removed. Only oldNS1 has addresses saved.
-	_, deps := runChecks(t, cfg, func(deps *testDeps) {
+	deps := runChecks(t, cfg, func(deps *testDeps) {
 		deps.state.SetDomainState(testSmallDomain, &state.DomainState{
 			Nameservers:         append([]string{oldNS1}, nameservers[1:]...),
 			NameserverAddresses: map[string][]string{oldNS1: {oldIP}},
 		})
-	})
+	}, nil)
 
 	if n := countNotifications(deps, "NS Change: "+testSmallDomain); n != 1 {
 		t.Errorf("sent %d NS changes, want 1", n)
@@ -562,17 +565,15 @@ func TestRecordChangeDetection(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	nameservers := lookupNameservers(t, testHost)
-
-	// The saved state has every nameserver live DNS lists answering
-	// with an address live DNS never returns.
-	_, deps := runChecks(t, cfg, func(deps *testDeps) {
-		byNameserver := make(map[string]*state.NameserverRecordState)
-		for _, ns := range nameservers {
-			byNameserver[ns] = answered(map[string][]string{"A": {oldIP}})
+	// Between the checks, save for every nameserver an address live DNS
+	// never returns.
+	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
+		hs, _ := deps.state.GetHostnameState(testHost)
+		for _, nsState := range hs.RecordsByNameserver {
+			nsState.Records = map[string][]string{"A": {oldIP}}
 		}
 
-		deps.state.SetHostnameState(testHost, saved(byNameserver))
+		deps.state.SetHostnameState(testHost, hs)
 	})
 
 	assertNotified(t, deps, "Record Change: "+testHost, "warning")
@@ -584,15 +585,12 @@ func TestPortStateChange(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	w, deps := runChecks(t, cfg, nil)
-
-	// Every port closes, and the port checks run again. They look
-	// nothing up.
-	deps.portChecker.mu.Lock()
-	deps.portChecker.closed = true
-	deps.portChecker.mu.Unlock()
-
-	w.CheckAllPorts(t.Context())
+	// Between the checks, every port closes.
+	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
+		deps.portChecker.mu.Lock()
+		deps.portChecker.closed = true
+		deps.portChecker.mu.Unlock()
+	})
 
 	hs, _ := deps.state.GetHostnameState(testHost)
 	assertNotified(
@@ -612,7 +610,7 @@ func TestTLSExpiryWarning(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	_, deps := runChecks(t, cfg, expiresInThreeDays)
+	deps := runChecks(t, cfg, expiresInThreeDays, nil)
 
 	assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning")
 }
@@ -784,7 +782,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
 	cfg.Hostnames = []string{testHost}
 
 	// The saved state says the last check found testHost at oldIP.
-	_, deps := runChecks(t, cfg, func(deps *testDeps) {
+	deps := runChecks(t, cfg, func(deps *testDeps) {
 		deps.state.SetHostnameState(testHost, &state.HostnameState{
 			RecordsByNameserver: map[string]*state.NameserverRecordState{
 				oldNS1: {
@@ -793,7 +791,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
 				},
 			},
 		})
-	})
+	}, nil)
 
 	snap := deps.state.GetSnapshot()
 
@@ -924,20 +922,21 @@ func TestNSFailureAndRecovery(t *testing.T) {
 	cfg := defaultTestConfig(t)
 	cfg.Hostnames = []string{testHost}
 
-	nameservers := lookupNameservers(t, testHost)
-
-	// The saved state has every nameserver live DNS lists as one that
-	// did not answer, and, as answering, one that live DNS does not
-	// list, which then disappears.
-	_, deps := runChecks(t, cfg, func(deps *testDeps) {
-		byNameserver := map[string]*state.NameserverRecordState{
-			oldNS1: answered(map[string][]string{"A": {oldIP}}),
-		}
-		for _, ns := range nameservers {
-			byNameserver[ns] = failed()
+	// Between the checks, save every nameserver the first check found
+	// as one that did not answer, and add, as answering, one that live
+	// DNS does not list, which then disappears.
+	deps := runChecks(t, cfg, nil, func(deps *testDeps) {
+		hs, _ := deps.state.GetHostnameState(testHost)
+		for ns := range hs.RecordsByNameserver {
+			hs.RecordsByNameserver[ns] = failed()
 		}
 
-		deps.state.SetHostnameState(testHost, saved(byNameserver))
+		hs.RecordsByNameserver[oldNS1] = &state.NameserverRecordState{
+			Records: map[string][]string{"A": {oldIP}},
+			Status:  "ok",
+		}
+
+		deps.state.SetHostnameState(testHost, hs)
 	})
 
 	assertNotified(t, deps, "NS Failure: "+testHost, "error")