diff --git a/REPO_POLICIES.md b/REPO_POLICIES.md index bc2f161..9aba6b0 100644 --- a/REPO_POLICIES.md +++ b/REPO_POLICIES.md @@ -1,6 +1,6 @@ --- title: Repository Policies -last_modified: 2026-07-06 +last_modified: 2026-08-07 --- This document covers repository structure, tooling, and workflow standards. Code @@ -189,8 +189,13 @@ style conventions are in separate documents: module under test to verify it compiles/parses. There is no excuse for `make test` to be a no-op. -- `make test` must complete in under 20 seconds. Add a 30-second timeout in the - Makefile. +- `make test` must complete in under 60 seconds. That is the hard cap, and a + suite that exceeds it fails. Under 20 seconds is the target. A suite between + 20 and 60 seconds is still green, but the overage must be filed as an + improvement bug against that repo. Add a 90-second timeout to the test + invocation in the Makefile (`go test -timeout 90s`). The backstop deliberately + sits above the hard cap so that it catches a genuinely hung test rather than a + merely slow one. - **`make test` should use the conditional verbose rerun pattern.** Run tests without `-v` (verbose) first. If tests fail, automatically rerun with `-v` to @@ -209,9 +214,9 @@ style conventions are in separate documents: ```makefile test: - @go test -timeout 30s -race -cover ./... || \ + @go test -timeout 90s -race -cover ./... || \ { echo "--- Rerunning with -v for details ---"; \ - go test -timeout 30s -race -v ./...; exit 1; } + go test -timeout 90s -race -v ./...; exit 1; } ``` Python example: @@ -260,7 +265,10 @@ style conventions are in separate documents: - `.golangci.yml` is standardized and must _NEVER_ be modified by an agent, only manually by the user. Fetch from - `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. + `https://git.eeqj.de/sneak/prompts/raw/branch/main/.golangci.yml`. The + canonical golangci-lint version is v2.12.2 (released 2026-05-06), installed + commit-pinned via + `go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@c0d3ddc9cf3faa61a4e378e879ece580256d76e5`. - When pinning images or packages by hash, add a comment above the reference with the version and date (YYYY-MM-DD). diff --git a/TESTING.md b/TESTING.md index 3dd34c4..0acae53 100644 --- a/TESTING.md +++ b/TESTING.md @@ -17,7 +17,7 @@ real servers ensures the resolver works correctly in production. - Tests hit real DNS infrastructure and require network access - Test duration depends on network conditions; timeout tuning keeps - the suite within the 30-second target + the suite within the 60-second target - Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus processing margin - Root server fan-out is limited to reduce parallel query load diff --git a/TODO.md b/TODO.md index 8e3909b..1b4f13f 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,16 @@ confirm make check still passes. # Completed Steps +- 2026-08-10: live-DNS test flakiness addressed by robustness rather + than gating, per the owner's ruling on #93: new + `internal/resolver/livedns_test.go` adds a package-wide concurrency + gate (so parallel tests stop bursting at the first root server), + retry with exponential backoff on transport failures only, and + quorum instead of unanimity for multi-nameserver assertions. The + `make test` cap moved to the new org-wide 60s hard cap / 20s target + with a 90s `-timeout` backstop; `REPO_POLICIES.md` re-vendored + byte-identical from `sneak/prompts`. No mocks, no `-short`, no build + tags, no skips, and no change to production resolver behaviour - 2026-08-10: all linting moved into Docker: new root `Dockerfile.lint` on the digest-pinned `golangci/golangci-lint:v2.12.2` image, `script/lint` reduced to a thin wrapper that builds it with diff --git a/internal/resolver/livedns_harness_test.go b/internal/resolver/livedns_harness_test.go new file mode 100644 index 0000000..066337d --- /dev/null +++ b/internal/resolver/livedns_harness_test.go @@ -0,0 +1,162 @@ +package resolver_test + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "sneak.berlin/go/dnswatcher/internal/resolver" +) + +// 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. + +func TestLiveQuorumIsStrictMajority(t *testing.T) { + t.Parallel() + + cases := map[int]int{ + 0: 1, + 1: 1, + 2: 2, + 3: 2, + 4: 3, + 5: 3, + 13: 7, + } + + for total, want := range cases { + assert.Equal( + t, want, liveQuorum(total), + "liveQuorum(%d)", total, + ) + } +} + +func TestStatusCountingIgnoresSilentNameservers(t *testing.T) { + t.Parallel() + + results := map[string]*resolver.NameserverResponse{ + "ns1.example.": { + Nameserver: "ns1.example.", + Status: resolver.StatusOK, + }, + "ns2.example.": { + Nameserver: "ns2.example.", + Status: resolver.StatusOK, + }, + "ns3.example.": { + Nameserver: "ns3.example.", + Status: resolver.StatusTimeout, + }, + "ns4.example.": { + Nameserver: "ns4.example.", + Status: resolver.StatusError, + }, + } + + assert.Equal( + t, 2, countStatus(results, resolver.StatusOK), + ) + assert.Equal( + t, 0, countStatus(results, resolver.StatusNXDomain), + ) + + // Two of four answered, which is short of the quorum of + // three: this is the state that triggers a retry rather + // than an assertion failure. + assert.Equal(t, 2, answeredCount(results)) + assert.Less(t, answeredCount(results), liveQuorum(len(results))) + + assert.Equal( + t, + "ns1.example.=ok ns2.example.=ok "+ + "ns3.example.=timeout ns4.example.=error", + describeStatuses(results), + ) +} + +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") + assert.LessOrEqual( + t, time.Until(deadline), liveAttemptTimeout, + ) + + return nil + }) +} + +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", + ) +} diff --git a/internal/resolver/livedns_test.go b/internal/resolver/livedns_test.go new file mode 100644 index 0000000..55ea755 --- /dev/null +++ b/internal/resolver/livedns_test.go @@ -0,0 +1,437 @@ +package resolver_test + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + "testing" + "time" + + "sneak.berlin/go/dnswatcher/internal/resolver" +) + +// ---------------------------------------------------------------- +// Live DNS test support +// ---------------------------------------------------------------- +// +// 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. +// +// Three mechanisms, all test-side: +// +// 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. + +const ( + // liveAttempts is how many times a live DNS operation is + // attempted before the test fails. + liveAttempts = 3 + + // 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, + ) +} + +// liveQuorum is how many of total nameservers must agree for a +// multi-nameserver assertion to hold: a strict majority. +func liveQuorum(total int) int { + if total < 1 { + return 1 + } + + return total/2 + 1 +} + +// countStatus counts the responses carrying the given status. +func countStatus( + results map[string]*resolver.NameserverResponse, + status string, +) int { + n := 0 + + for _, resp := range results { + if resp.Status == status { + n++ + } + } + + return n +} + +// answeredCount counts the nameservers that produced an answer of +// any kind, as opposed to failing or timing out. +func answeredCount( + results map[string]*resolver.NameserverResponse, +) int { + return len(results) - + countStatus(results, resolver.StatusError) - + countStatus(results, resolver.StatusTimeout) +} + +// describeStatuses renders per-nameserver statuses for use in +// assertion failure messages. +func describeStatuses( + results map[string]*resolver.NameserverResponse, +) string { + parts := make([]string, 0, len(results)) + for ns, resp := range results { + parts = append( + parts, fmt.Sprintf("%s=%s", ns, resp.Status), + ) + } + + sort.Strings(parts) + + return strings.Join(parts, " ") +} + +// ---------------------------------------------------------------- +// Live operation wrappers +// ---------------------------------------------------------------- + +// liveFindAuthoritative resolves a domain's authoritative +// nameservers, retrying until the delegation chain can be walked. +func liveFindAuthoritative( + t *testing.T, + r *resolver.Resolver, + domain string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "FindAuthoritativeNameservers("+domain+")", + func(ctx context.Context) error { + ns, err := r.FindAuthoritativeNameservers(ctx, domain) + if err != nil { + return err + } + + if len(ns) == 0 { + return fmt.Errorf( + "%w: %s has no nameservers", + errLiveNoAnswer, domain, + ) + } + + out = ns + + return nil + }, + ) + + return out +} + +// liveLookupNS is liveFindAuthoritative through the LookupNS entry +// point, so that both entry points stay independently exercised. +func liveLookupNS( + t *testing.T, + r *resolver.Resolver, + domain string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "LookupNS("+domain+")", + func(ctx context.Context) error { + ns, err := r.LookupNS(ctx, domain) + if err != nil { + return err + } + + if len(ns) == 0 { + return fmt.Errorf( + "%w: %s has no nameservers", + errLiveNoAnswer, domain, + ) + } + + out = ns + + return nil + }, + ) + + return out +} + +// liveQueryNameserver queries one nameserver, retrying while that +// nameserver fails to answer. NXDOMAIN and NODATA are answers and +// are returned to the caller to assert on. +func liveQueryNameserver( + t *testing.T, + r *resolver.Resolver, + nameserver string, + hostname string, +) *resolver.NameserverResponse { + t.Helper() + + what := fmt.Sprintf( + "QueryNameserver(%s, %s)", nameserver, hostname, + ) + + var out *resolver.NameserverResponse + + retryLive( + t, + what, + func(ctx context.Context) error { + resp, err := r.QueryNameserver( + ctx, nameserver, hostname, + ) + if err != nil { + return err + } + + if resp.Status == resolver.StatusTimeout || + resp.Status == resolver.StatusError { + return fmt.Errorf( + "%w: %s returned %s: %s", + errLiveNoAnswer, nameserver, + resp.Status, resp.Error, + ) + } + + out = resp + + return nil + }, + ) + + return out +} + +// liveQueryAllNameservers queries every authoritative nameserver +// for a hostname, retrying until a quorum of them has answered. +// Individual nameservers that stay silent are left in the result +// for the caller to account for. +func liveQueryAllNameservers( + t *testing.T, + r *resolver.Resolver, + hostname string, +) map[string]*resolver.NameserverResponse { + t.Helper() + + var out map[string]*resolver.NameserverResponse + + retryLive( + t, + "QueryAllNameservers("+hostname+")", + func(ctx context.Context) error { + results, err := r.QueryAllNameservers(ctx, hostname) + if err != nil { + return err + } + + if len(results) == 0 { + return fmt.Errorf( + "%w: no nameservers queried for %s", + errLiveNoAnswer, hostname, + ) + } + + answered := answeredCount(results) + if answered < liveQuorum(len(results)) { + return fmt.Errorf( + "%w: %d of %d answered: %s", + errLiveNoQuorum, answered, + len(results), describeStatuses(results), + ) + } + + out = results + + return nil + }, + ) + + return out +} + +// liveResolveIPs resolves a hostname that is expected to have +// addresses, retrying until at least one is returned. +func liveResolveIPs( + t *testing.T, + r *resolver.Resolver, + hostname string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "ResolveIPAddresses("+hostname+")", + func(ctx context.Context) error { + ips, err := r.ResolveIPAddresses(ctx, hostname) + if err != nil { + return err + } + + if len(ips) == 0 { + return fmt.Errorf( + "%w: no addresses for %s", + errLiveNoAnswer, hostname, + ) + } + + out = ips + + return nil + }, + ) + + return out +} + +// liveResolveIPsAllowingEmpty resolves a hostname that may legitimately +// have no addresses, so the empty result is returned rather than +// retried. Used for names that must not exist; the corresponding +// QueryAllNameservers test is what proves the nameservers actively +// said NXDOMAIN rather than merely staying silent. +func liveResolveIPsAllowingEmpty( + t *testing.T, + r *resolver.Resolver, + hostname string, +) []string { + t.Helper() + + var out []string + + retryLive( + t, + "ResolveIPAddresses("+hostname+")", + func(ctx context.Context) error { + ips, err := r.ResolveIPAddresses(ctx, hostname) + if err != nil { + return err + } + + out = ips + + return nil + }, + ) + + return out +} diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index bcebfb9..721847e 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -32,32 +32,17 @@ func newTestResolver(t *testing.T) *resolver.Resolver { return resolver.NewFromLogger(log) } -func testContext(t *testing.T) context.Context { - t.Helper() - - ctx, cancel := context.WithTimeout( - context.Background(), 60*time.Second, - ) - t.Cleanup(cancel) - - return ctx -} - +// findOneNSForDomain picks one authoritative nameserver to aim a +// test at. Live-DNS retry, concurrency and quorum handling live in +// livedns_test.go. func findOneNSForDomain( t *testing.T, r *resolver.Resolver, - ctx context.Context, //nolint:revive // test helper domain string, ) string { t.Helper() - nameservers, err := r.FindAuthoritativeNameservers( - ctx, domain, - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) - - return nameservers[0] + return liveFindAuthoritative(t, r, domain)[0] } // ---------------------------------------------------------------- @@ -70,13 +55,7 @@ func TestFindAuthoritativeNameservers_ValidDomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) + nameservers := liveFindAuthoritative(t, r, "google.com") hasGoogleNS := false @@ -99,13 +78,9 @@ func TestFindAuthoritativeNameservers_Subdomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + nameservers := liveFindAuthoritative(t, r, "www.google.com") - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "www.google.com", - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) + assert.NotEmpty(t, nameservers) } func TestFindAuthoritativeNameservers_ReturnsSorted( @@ -114,12 +89,7 @@ func TestFindAuthoritativeNameservers_ReturnsSorted( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) + nameservers := liveFindAuthoritative(t, r, "google.com") assert.True( t, @@ -134,17 +104,8 @@ func TestFindAuthoritativeNameservers_Deterministic( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - first, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - - second, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) + first := liveFindAuthoritative(t, r, "google.com") + second := liveFindAuthoritative(t, r, "google.com") assert.Equal(t, first, second) } @@ -155,17 +116,8 @@ func TestFindAuthoritativeNameservers_TrailingDot( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ns1, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - - ns2, err := r.FindAuthoritativeNameservers( - ctx, "google.com.", - ) - require.NoError(t, err) + ns1 := liveFindAuthoritative(t, r, "google.com") + ns2 := liveFindAuthoritative(t, r, "google.com.") assert.Equal(t, ns1, ns2) } @@ -176,13 +128,7 @@ func TestFindAuthoritativeNameservers_CloudflareDomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.FindAuthoritativeNameservers( - ctx, "cloudflare.com", - ) - require.NoError(t, err) - require.NotEmpty(t, nameservers) + nameservers := liveFindAuthoritative(t, r, "cloudflare.com") for _, ns := range nameservers { assert.True(t, strings.HasSuffix(ns, "."), @@ -199,13 +145,9 @@ func TestQueryNameserver_BasicA(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "www.google.com") - resp, err := r.QueryNameserver( - ctx, ns, "www.google.com", - ) - require.NoError(t, err) require.NotNil(t, resp) assert.Equal(t, resolver.StatusOK, resp.Status) @@ -222,13 +164,8 @@ func TestQueryNameserver_AAAA(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "cloudflare.com") - - resp, err := r.QueryNameserver( - ctx, ns, "cloudflare.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "cloudflare.com") + resp := liveQueryNameserver(t, r, ns, "cloudflare.com") aaaaRecords := resp.Records["AAAA"] require.NotEmpty(t, aaaaRecords, @@ -247,13 +184,8 @@ func TestQueryNameserver_MX(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "google.com") mxRecords := resp.Records["MX"] require.NotEmpty(t, mxRecords, @@ -265,13 +197,8 @@ func TestQueryNameserver_TXT(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "google.com") txtRecords := resp.Records["TXT"] require.NotEmpty(t, txtRecords, @@ -297,14 +224,10 @@ func TestQueryNameserver_NXDomain(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, - "this-surely-does-not-exist-xyz.google.com", + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver( + t, r, ns, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) assert.Equal(t, resolver.StatusNXDomain, resp.Status) } @@ -313,13 +236,8 @@ func TestQueryNameserver_RecordsSorted(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver(t, r, ns, "google.com") for recordType, values := range resp.Records { assert.True( @@ -336,13 +254,8 @@ func TestQueryNameserver_ResponseIncludesNameserver( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "cloudflare.com") - - resp, err := r.QueryNameserver( - ctx, ns, "cloudflare.com", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "cloudflare.com") + resp := liveQueryNameserver(t, r, ns, "cloudflare.com") assert.Equal(t, ns, resp.Nameserver) } @@ -353,14 +266,10 @@ func TestQueryNameserver_EmptyRecordsOnNXDomain( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp, err := r.QueryNameserver( - ctx, ns, - "this-surely-does-not-exist-xyz.google.com", + ns := findOneNSForDomain(t, r, "google.com") + resp := liveQueryNameserver( + t, r, ns, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) totalRecords := 0 for _, values := range resp.Records { @@ -374,18 +283,9 @@ func TestQueryNameserver_TrailingDotHandling(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - ns := findOneNSForDomain(t, r, ctx, "google.com") - - resp1, err := r.QueryNameserver( - ctx, ns, "google.com", - ) - require.NoError(t, err) - - resp2, err := r.QueryNameserver( - ctx, ns, "google.com.", - ) - require.NoError(t, err) + ns := findOneNSForDomain(t, r, "google.com") + resp1 := liveQueryNameserver(t, r, ns, "google.com") + resp2 := liveQueryNameserver(t, r, ns, "google.com.") assert.Equal(t, resp1.Status, resp2.Status) } @@ -398,15 +298,9 @@ func TestQueryAllNameservers_ReturnsAllNS(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + results := liveQueryAllNameservers(t, r, "google.com") - results, err := r.QueryAllNameservers( - ctx, "google.com", - ) - require.NoError(t, err) - require.NotEmpty(t, results) - - assert.GreaterOrEqual(t, len(results), 2) + assert.GreaterOrEqual(t, len(results), minNameservers) for ns, resp := range results { assert.Equal(t, ns, resp.Nameserver) @@ -417,19 +311,27 @@ func TestQueryAllNameservers_AllReturnOK(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + results := liveQueryAllNameservers(t, r, "google.com") - results, err := r.QueryAllNameservers( - ctx, "google.com", + // A quorum, not unanimity: one authoritative server being + // slow or rate-limiting us is a property of the live + // internet, not a resolver defect. + assert.GreaterOrEqual( + t, + countStatus(results, resolver.StatusOK), + liveQuorum(len(results)), + "a quorum of nameservers should answer OK: %s", + describeStatuses(results), ) - require.NoError(t, err) - for ns, resp := range results { - assert.Equal( - t, resolver.StatusOK, resp.Status, - "NS %s should return OK", ns, - ) - } + // Any nameserver claiming google.com does not exist is a + // real failure and is never tolerated. + assert.Zero( + t, + countStatus(results, resolver.StatusNXDomain), + "no nameserver should report NXDOMAIN: %s", + describeStatuses(results), + ) } func TestQueryAllNameservers_NXDomainFromAllNS( @@ -438,20 +340,26 @@ func TestQueryAllNameservers_NXDomainFromAllNS( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - results, err := r.QueryAllNameservers( - ctx, - "this-surely-does-not-exist-xyz.google.com", + results := liveQueryAllNameservers( + t, r, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) - for ns, resp := range results { - assert.Equal( - t, resolver.StatusNXDomain, resp.Status, - "NS %s should return nxdomain", ns, - ) - } + assert.GreaterOrEqual( + t, + countStatus(results, resolver.StatusNXDomain), + liveQuorum(len(results)), + "a quorum of nameservers should report NXDOMAIN: %s", + describeStatuses(results), + ) + + // Silence is tolerated; a positive answer for a name that + // does not exist is not. + assert.Zero( + t, + countStatus(results, resolver.StatusOK), + "no nameserver should answer OK: %s", + describeStatuses(results), + ) } // ---------------------------------------------------------------- @@ -462,11 +370,7 @@ func TestLookupNS_ValidDomain(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.LookupNS(ctx, "google.com") - require.NoError(t, err) - require.NotEmpty(t, nameservers) + nameservers := liveLookupNS(t, r, "google.com") for _, ns := range nameservers { assert.True(t, strings.HasSuffix(ns, "."), @@ -479,10 +383,7 @@ func TestLookupNS_Sorted(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - nameservers, err := r.LookupNS(ctx, "google.com") - require.NoError(t, err) + nameservers := liveLookupNS(t, r, "google.com") assert.True(t, sort.StringsAreSorted(nameservers)) } @@ -491,15 +392,8 @@ func TestLookupNS_MatchesFindAuthoritative(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - fromLookup, err := r.LookupNS(ctx, "google.com") - require.NoError(t, err) - - fromFind, err := r.FindAuthoritativeNameservers( - ctx, "google.com", - ) - require.NoError(t, err) + fromLookup := liveLookupNS(t, r, "google.com") + fromFind := liveFindAuthoritative(t, r, "google.com") assert.Equal(t, fromFind, fromLookup) } @@ -512,11 +406,7 @@ func TestResolveIPAddresses_ReturnsIPs(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses(ctx, "google.com") - require.NoError(t, err) - require.NotEmpty(t, ips) + ips := liveResolveIPs(t, r, "google.com") for _, ip := range ips { parsed := net.ParseIP(ip) @@ -530,10 +420,7 @@ func TestResolveIPAddresses_Deduplicated(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses(ctx, "google.com") - require.NoError(t, err) + ips := liveResolveIPs(t, r, "google.com") seen := make(map[string]bool) @@ -547,10 +434,7 @@ func TestResolveIPAddresses_Sorted(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses(ctx, "google.com") - require.NoError(t, err) + ips := liveResolveIPs(t, r, "google.com") assert.True(t, sort.StringsAreSorted(ips)) } @@ -561,13 +445,10 @@ func TestResolveIPAddresses_NXDomainReturnsEmpty( t.Parallel() r := newTestResolver(t) - ctx := testContext(t) - - ips, err := r.ResolveIPAddresses( - ctx, - "this-surely-does-not-exist-xyz.google.com", + ips := liveResolveIPsAllowingEmpty( + t, r, "this-surely-does-not-exist-xyz.google.com", ) - require.NoError(t, err) + assert.Empty(t, ips) } @@ -575,11 +456,9 @@ func TestResolveIPAddresses_CloudflareDomain(t *testing.T) { t.Parallel() r := newTestResolver(t) - ctx := testContext(t) + ips := liveResolveIPs(t, r, "cloudflare.com") - ips, err := r.ResolveIPAddresses(ctx, "cloudflare.com") - require.NoError(t, err) - require.NotEmpty(t, ips) + assert.NotEmpty(t, ips) } // ---------------------------------------------------------------- diff --git a/script/test b/script/test index 50b1730..568e3f5 100755 --- a/script/test +++ b/script/test @@ -6,7 +6,7 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" main() { cd "$ROOT" - go test -v -race -timeout 30s -cover ./... + go test -v -race -timeout 90s -cover ./... } main "$@"