Compare commits

..
1 Commits
Author SHA1 Message Date
clawbot bdd4686200 tests: remove the DNS stand-ins from the watcher and resolver tests (closes #159)
check / check (push) Successful in 1m11s
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.
Watcher tests that look something up in DNS now run the real resolver
against live servers, each attempt on a new watcher. A DNS 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 now
states the README's rule.

Model: opus-5-5
2026-09-29 04:59:14 +00:00
5 changed files with 94 additions and 102 deletions
+10 -13
View File
@@ -10,11 +10,7 @@
# Status # Status
pre-1.0. No git tags. Core resolver work in flight on feature/resolver pre-1.0. No git tags.
(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.
# Next Step # Next Step
@@ -23,13 +19,13 @@ Rationale, Design, TODO, License, Author) if any are still missing.
# Completed Steps # Completed Steps
- 2026-09-29: nothing stands in for DNS any more. The watcher tests use the - 2026-09-29: nothing stands in for DNS any more. Watcher tests that look
real resolver against live DNS and test changes by preparing the saved something up in DNS use the real resolver against live DNS servers and test
state a check starts from; the resolver timeout test queries an address record and nameserver changes by preparing the saved state a check starts
that never answers, and `NewFromLoggerWithClient`, used only by its from; the resolver timeout test queries an address that never answers, and
stand-in client, is gone. The live-DNS retry and concurrency limit moved `NewFromLoggerWithClient`, used only by its stand-in client, is gone. The
to `internal/livedns`, which both test packages use. `TESTING.md` states live-DNS retry and concurrency limit moved to `internal/livedns`, which both
the README's rule (closes #159). 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
@@ -268,4 +264,5 @@ Infrastructure notes (from untracked TODO.md):
- Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de - Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de
remote intentionally; do not "fix" it remote intentionally; do not "fix" it
- Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix - Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix
- Tests use live DNS and never mock it (README, "No DNS mocking. Ever.") - DNS is never mocked; tests that look something up in DNS query live DNS
servers (README, "No DNS mocking. Ever.")
+2 -2
View File
@@ -1,5 +1,5 @@
// Package livedns runs the live DNS operations of tests. Every test in // Package livedns runs the live DNS operations of tests. Tests that
// this project that needs DNS resolves against the real, live DNS — // 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
// queries are issued, so that a single dropped UDP packet or one slow // queries are issued, so that a single dropped UDP packet or one slow
+6 -5
View File
@@ -17,11 +17,12 @@ import (
// Live DNS test support // Live DNS test support
// ---------------------------------------------------------------- // ----------------------------------------------------------------
// //
// Every test in this package resolves against the real, live DNS — // Tests that look something up in DNS query live DNS servers, never a
// see TESTING.md. Each live operation below goes through // stand-in; logic that works on record data may be tested on that
// livedns.Retry, which bounds how many resolutions are in flight at // data with no lookup (see TESTING.md). Each live operation below goes
// once and retries an operation that got no answer (see package // through livedns.Retry, which bounds how many resolutions are in
// livedns). // flight at once and retries an operation that got no answer (see
// package livedns).
// //
// 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
+2 -2
View File
@@ -32,8 +32,8 @@ 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. Live-DNS retry, concurrency and quorum handling live in // test at. Quorum handling lives in livedns_test.go, and the live-DNS
// livedns_test.go. // retry and concurrency limit in package livedns.
func findOneNSForDomain( func findOneNSForDomain(
t *testing.T, t *testing.T,
r *resolver.Resolver, r *resolver.Resolver,
+74 -80
View File
@@ -19,9 +19,10 @@ import (
) )
// The watcher looks these names up in live DNS with the real resolver, // The watcher looks these names up in live DNS with the real resolver,
// so tests assert on notifications and saved state, never on the // so tests assert on what the watcher does with the answers, never on
// records these zones publish. testHost's addresses stay the same from // the records these zones publish. testHost's nameservers and addresses
// one check to the next, which the tests that check it twice rely on. // stay the same from one check to the next, which the tests that check
// it twice rely on.
const ( const (
testDomain = "google.com" testDomain = "google.com"
testHost = "cloudflare.com" testHost = "cloudflare.com"
@@ -38,7 +39,8 @@ const (
// --- Stand-ins for the port checker, TLS checker and notifier --- // --- Stand-ins for the port checker, TLS checker and notifier ---
// //
// DNS has none: the watcher uses the real resolver (see TESTING.md). // DNS has none: the watchers built here use the real resolver (see
// TESTING.md).
// mockPortChecker reports every port open until closed is set. // mockPortChecker reports every port open until closed is set.
type mockPortChecker struct { type mockPortChecker struct {
@@ -173,9 +175,8 @@ func defaultTestConfig(t *testing.T) *config.Config {
// checkOnce runs the watcher's checks once and returns an error when a // checkOnce runs the watcher's checks once and returns an error when a
// configured name has no hostname state saved by this check, or that // configured name has no hostname state saved by this check, or that
// state holds no address. The watcher saves a name's hostname state // state holds no address. Either live DNS gave no answer for the name,
// only when all of the name's lookups succeed, so this means either // or the watcher saved no fresh result for it.
// live DNS did not answer or the watcher did not save what it got.
func checkOnce( func checkOnce(
ctx context.Context, ctx context.Context,
w *watcher.Watcher, w *watcher.Watcher,
@@ -202,46 +203,42 @@ func checkOnce(
return nil return nil
} }
// runFirstCheck builds a watcher, lets prepare set up the saved state // runChecks builds a watcher, lets prepare set up the saved state and
// and stand-ins it starts from, and runs its checks once against live // stand-ins it starts from, and runs its checks once against live DNS.
// DNS. When live DNS does not answer, the watcher is thrown away and // If change is not nil, change then alters the saved state or stand-ins
// built again, so a failed attempt leaves nothing behind in the state // and the checks run a second time. When either check finds no fresh
// or the notifications. // address for a name (see checkOnce), the watcher is thrown away and
func runFirstCheck( // 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, t *testing.T,
cfg *config.Config, cfg *config.Config,
prepare func(deps *testDeps), prepare, change func(deps *testDeps),
) (*watcher.Watcher, *testDeps) { ) *testDeps {
t.Helper() t.Helper()
var ( var deps *testDeps
w *watcher.Watcher
deps *testDeps livedns.Retry(t, "watcher checks", func(ctx context.Context) error {
) var w *watcher.Watcher
livedns.Retry(t, "first check", func(ctx context.Context) error {
w, deps = newTestWatcher(t, cfg) w, deps = newTestWatcher(t, cfg)
if prepare != nil { if prepare != nil {
prepare(deps) prepare(deps)
} }
err := checkOnce(ctx, w, deps)
if err != nil || change == nil {
return err
}
change(deps)
return checkOnce(ctx, w, deps) return checkOnce(ctx, w, deps)
}) })
return w, deps return deps
}
// runCheck runs the watcher's checks once more against live DNS,
// repeating them while live DNS does not answer. A failed lookup keeps
// the name's saved records, so a repeat compares against the same
// saved state.
func runCheck(t *testing.T, w *watcher.Watcher, deps *testDeps) {
t.Helper()
livedns.Retry(t, "check", func(ctx context.Context) error {
return checkOnce(ctx, w, deps)
})
} }
// addresses returns the A and AAAA values saved for a hostname. // addresses returns the A and AAAA values saved for a hostname.
@@ -299,7 +296,7 @@ func TestFirstRunBaseline(t *testing.T) {
cfg.Domains = []string{testDomain} cfg.Domains = []string{testDomain}
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
_, deps := runFirstCheck(t, cfg, nil) deps := runChecks(t, cfg, nil, nil)
assertNoNotifications(t, deps) assertNoNotifications(t, deps)
assertStatePopulated(t, deps) assertStatePopulated(t, deps)
@@ -351,7 +348,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Domains = []string{testDomain} cfg.Domains = []string{testDomain}
_, deps := runFirstCheck(t, cfg, nil) deps := runChecks(t, cfg, nil, nil)
snap := deps.state.GetSnapshot() snap := deps.state.GetSnapshot()
@@ -391,11 +388,11 @@ func TestNSChangeDetection(t *testing.T) {
cfg.Domains = []string{testDomain} cfg.Domains = []string{testDomain}
// The saved state lists nameservers that live DNS does not. // The saved state lists nameservers that live DNS does not.
_, deps := runFirstCheck(t, cfg, func(deps *testDeps) { deps := runChecks(t, cfg, func(deps *testDeps) {
deps.state.SetDomainState(testDomain, &state.DomainState{ deps.state.SetDomainState(testDomain, &state.DomainState{
Nameservers: []string{oldNS1, oldNS2}, Nameservers: []string{oldNS1, oldNS2},
}) })
}) }, nil)
assertNotified(t, deps, "NS Change: "+testDomain, "warning") assertNotified(t, deps, "NS Change: "+testDomain, "warning")
@@ -411,17 +408,16 @@ func TestRecordChangeDetection(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
w, deps := runFirstCheck(t, cfg, nil) // 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}}
}
// Save, for every nameserver, an address live DNS never returns. deps.state.SetHostnameState(testHost, hs)
hs, _ := deps.state.GetHostnameState(testHost) })
for _, nsState := range hs.RecordsByNameserver {
nsState.Records = map[string][]string{"A": {oldIP}}
}
deps.state.SetHostnameState(testHost, hs)
runCheck(t, w, deps)
assertNotified(t, deps, "Record Change: "+testHost, "warning") assertNotified(t, deps, "Record Change: "+testHost, "warning")
} }
@@ -432,13 +428,12 @@ func TestPortStateChange(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
w, deps := runFirstCheck(t, cfg, nil) // Between the checks, every port closes.
deps := runChecks(t, cfg, nil, func(deps *testDeps) {
deps.portChecker.mu.Lock() deps.portChecker.mu.Lock()
deps.portChecker.closed = true deps.portChecker.closed = true
deps.portChecker.mu.Unlock() deps.portChecker.mu.Unlock()
})
runCheck(t, w, deps)
hs, _ := deps.state.GetHostnameState(testHost) hs, _ := deps.state.GetHostnameState(testHost)
assertNotified( assertNotified(
@@ -458,7 +453,7 @@ func TestTLSExpiryWarning(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
_, deps := runFirstCheck(t, cfg, expiresInThreeDays) deps := runChecks(t, cfg, expiresInThreeDays, nil)
assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning") assertNotified(t, deps, "TLS Expiry Warning: "+testHost, "warning")
} }
@@ -470,18 +465,19 @@ func TestTLSExpiryWarningDedup(t *testing.T) {
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
cfg.TLSInterval = 24 * time.Hour cfg.TLSInterval = 24 * time.Hour
w, deps := runFirstCheck(t, cfg, expiresInThreeDays)
title := "TLS Expiry Warning: " + testHost title := "TLS Expiry Warning: " + testHost
warnings := countNotifications(deps, title)
if warnings == 0 {
t.Fatal("expected expiry warnings from the first check")
}
// The second check comes within the TLS interval of the first, // The second check comes within the TLS interval of the first,
// so it must not warn again. // so it must not warn again.
runCheck(t, w, deps) var warnings int
deps := runChecks(t, cfg, expiresInThreeDays, func(deps *testDeps) {
warnings = countNotifications(deps, title)
})
if warnings == 0 {
t.Fatal("expected expiry warnings from the first check")
}
got := countNotifications(deps, title) got := countNotifications(deps, title)
if got != warnings { if got != warnings {
@@ -529,7 +525,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
// The saved state says the last check found testHost at oldIP. // The saved state says the last check found testHost at oldIP.
_, deps := runFirstCheck(t, cfg, func(deps *testDeps) { deps := runChecks(t, cfg, func(deps *testDeps) {
deps.state.SetHostnameState(testHost, &state.HostnameState{ deps.state.SetHostnameState(testHost, &state.HostnameState{
RecordsByNameserver: map[string]*state.NameserverRecordState{ RecordsByNameserver: map[string]*state.NameserverRecordState{
oldNS1: { oldNS1: {
@@ -538,7 +534,7 @@ func TestDNSRunsBeforePortAndTLSChecks(t *testing.T) {
}, },
}, },
}) })
}) }, nil)
snap := deps.state.GetSnapshot() snap := deps.state.GetSnapshot()
@@ -669,23 +665,21 @@ func TestNSFailureAndRecovery(t *testing.T) {
cfg := defaultTestConfig(t) cfg := defaultTestConfig(t)
cfg.Hostnames = []string{testHost} cfg.Hostnames = []string{testHost}
w, deps := runFirstCheck(t, cfg, nil) // Between the checks, save every nameserver the first check found
// as failed, and add, as answering, one that live DNS does not list.
deps := runChecks(t, cfg, nil, func(deps *testDeps) {
hs, _ := deps.state.GetHostnameState(testHost)
for _, nsState := range hs.RecordsByNameserver {
nsState.Status = "error"
}
// Save every nameserver the first check found as failed, and add hs.RecordsByNameserver[oldNS1] = &state.NameserverRecordState{
// one that live DNS does not list as having answered. Records: map[string][]string{"A": {oldIP}},
hs, _ := deps.state.GetHostnameState(testHost) Status: "ok",
for _, nsState := range hs.RecordsByNameserver { }
nsState.Status = "error"
}
hs.RecordsByNameserver[oldNS1] = &state.NameserverRecordState{ deps.state.SetHostnameState(testHost, hs)
Records: map[string][]string{"A": {oldIP}}, })
Status: "ok",
}
deps.state.SetHostnameState(testHost, hs)
runCheck(t, w, deps)
assertNotified(t, deps, "NS Failure: "+testHost, "error") assertNotified(t, deps, "NS Failure: "+testHost, "error")
assertNotified(t, deps, "NS Recovery: "+testHost, "success") assertNotified(t, deps, "NS Recovery: "+testHost, "success")