DNS is never mocked in this repository: tests exercise live DNS, and robustness comes from handling real-world DNS behavior with tolerant assertions and sensible timeouts, not from mocks. This branch removes the remaining DNS mocks from the suite and, in the same change, restores the transport-failure coverage that removing them would otherwise have dropped — without reintroducing a mock.
Targets next.
What was removed
mockResolver in internal/watcher/watcher_test.go. The watcher tests are now wired to the real iterative resolver via resolver.NewFromLogger and query stable public names (example.com, www.example.com).
timeoutClient, the fake DNSClient in internal/resolver/resolver_test.go, and the old TestQueryNameserverIP_Timeout that depended on it.
resolver.NewFromLoggerWithClient in internal/resolver/resolver.go — the constructor whose only purpose was injecting mock DNS clients.
The TESTING.md carve-out that permitted DNS mocks in packages consuming the resolver. The live-DNS policy now applies to every package and to every DNS abstraction, naming the watcher's DNSResolver interface explicitly alongside DNSClient.
Transport-failure coverage, restored without mocks
New file internal/resolver/transport_test.go covers the classification branches the fake client used to cover, by binding a real UDP nameserver on 127.0.0.1 and aiming a live query at it:
TestQueryNameserverIP_Timeout — a nameserver that takes the query and never answers must classify as StatusTimeout, with error all queries timed out and no records. The test's nameserver is silent on A alone and answers every other type, which costs two query timeouts instead of the sixteen a wholly silent nameserver would cost; an 8s budget assertion fails loudly if that ever stops being true.
TestQueryNameserverIP_ServFail — a nameserver that answers SERVFAIL must classify as StatusError, with error server returned SERVFAIL.
TestQueryNameserverIP_NoListener — a refused datagram is not a timeout. With nothing listening, the socket fails immediately rather than going quiet, so the response classifies as StatusNoData. Pinning this is what stops the refused path and the timeout path being mistaken for each other in either direction.
Nothing here substitutes DNSClient or any other DNS abstraction. The resolver dials a real 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 goes to and what is listening there — and choosing which nameserver a live query is sent to is not faking DNS, since production aims queries at nameservers of the delegation's choosing too.
The public network cannot produce these outcomes on demand: a black-holed address is not black-holed everywhere, 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. A loopback nameserver is deterministic everywhere and fast, because the test picks the deadline.
TESTING.md gains a section recording why this is permitted and where the line is: substituting the client is banned; choosing the server is not. A test that reaches for a fake DNSClient to force a classification remains forbidden regardless of how awkward the alternative looks.
Production changes
internal/resolver/iterative.go: new nameserverAddr helper. queryDNS previously did net.JoinHostPort(serverIP, "53") unconditionally, so a nameserver address that already carried a port could not be dialled as written. It now passes such an address through untouched and applies the default port only to a bare one — the normal case, and what a delegation's glue records carry. This is what makes a nameserver listening somewhere other than 53 reachable, so no production path is bypassed to arrange the tests above.
internal/resolver/dns_client.go: doc comment only. DNSClient is now described as what it is — an abstraction over a single transport, letting the resolver switch between UDP and TCP — rather than as a testing seam.
How the rewritten watcher tests exercise live DNS
Every watcher test resolves live DNS on every run. Change detection is driven by seeding the state store with a synthetic previous observation that live DNS cannot match — RFC 2606 .invalid nameserver names and the RFC 5737 documentation address 203.0.113.1 — and then re-running against live DNS:
NS change: the recorded NS baseline is replaced with ns-baseline.invalid.; the next live run must detect the difference and notify at warning priority.
Record change: each nameserver's recorded answer is rewritten to 203.0.113.1; the next live run must observe the real records and notify.
NS failure/recovery: one real live-observed nameserver is recorded as failed and a synthetic one is recorded as healthy; the next live run must report the synthetic one as disappeared and the real one as recovered.
Stale-DNS regression (TestDNSRunsBeforePortAndTLSChecks): state is seeded with a stale documentation address and a port entry for it; after one cycle, port and TLS state must be keyed by the freshly live-resolved addresses and the stale key must be gone.
Startup notification tests poll for scan completion against a deadline instead of assuming a scan duration, since live scans take variable time.
Assertions compare exact sets, not counts. assertStatePopulated and TestDomainPortAndTLSChecks require that port state, certificate state, and the arguments the port and TLS checkers were actually called with all match the addresses live DNS returned — every one of them and no others. Asserting only non-emptiness would hold just as well if the watcher had resolved the wrong name or dropped all but one of its addresses. The test doubles record their call arguments for that reason.
The port checker, TLS checker, and notifier remain test doubles — they are not DNS — and are generalized (all ports open or closed, one certificate for any address) so assertions hold for whatever addresses live DNS returns, including multi-IP and AAAA answers.
Because the whole package now runs live iterative resolutions in parallel, it gains liveWatcherGate, a package-scoped bound of 3 concurrent live-DNS tests. It is the counterpart to liveGate in internal/resolver: those gates cannot reach across package boundaries, so each package needs its own.
script/test now passes -p 1
Go runs package test binaries in parallel by default. The two live-DNS gates are package-scoped and therefore per test binary, so with both live-DNS packages in flight at once their bounds sum instead of holding: the root and TLD servers rate-limit the excess and the resolver package's per-attempt deadlines start expiring. Serialising packages is what makes each gate authoritative while its package runs. TESTING.md records this and adds -p 1 to the list of flags not to remove, alongside -count=1.
Documentation
TESTING.md and TODO.md are updated to match: the mock ban is stated for every package, the loopback-nameserver rule is written down, and TODO.md records both the mock removal and the restored transport coverage.
DNS is never mocked in this repository: tests exercise live DNS, and robustness comes from handling real-world DNS behavior with tolerant assertions and sensible timeouts, not from mocks. This branch removes the remaining DNS mocks from the suite and, in the same change, restores the transport-failure coverage that removing them would otherwise have dropped — without reintroducing a mock.
Targets `next`.
## What was removed
- `mockResolver` in `internal/watcher/watcher_test.go`. The watcher tests are now wired to the real iterative resolver via `resolver.NewFromLogger` and query stable public names (`example.com`, `www.example.com`).
- `timeoutClient`, the fake `DNSClient` in `internal/resolver/resolver_test.go`, and the old `TestQueryNameserverIP_Timeout` that depended on it.
- `resolver.NewFromLoggerWithClient` in `internal/resolver/resolver.go` — the constructor whose only purpose was injecting mock DNS clients.
- The `TESTING.md` carve-out that permitted DNS mocks in packages consuming the resolver. The live-DNS policy now applies to every package and to every DNS abstraction, naming the watcher's `DNSResolver` interface explicitly alongside `DNSClient`.
## Transport-failure coverage, restored without mocks
New file `internal/resolver/transport_test.go` covers the classification branches the fake client used to cover, by binding a real UDP nameserver on `127.0.0.1` and aiming a live query at it:
- `TestQueryNameserverIP_Timeout` — a nameserver that takes the query and never answers must classify as `StatusTimeout`, with error `all queries timed out` and no records. The test's nameserver is silent on `A` alone and answers every other type, which costs two query timeouts instead of the sixteen a wholly silent nameserver would cost; an 8s budget assertion fails loudly if that ever stops being true.
- `TestQueryNameserverIP_ServFail` — a nameserver that answers SERVFAIL must classify as `StatusError`, with error `server returned SERVFAIL`.
- `TestQueryNameserverIP_NoListener` — a refused datagram is not a timeout. With nothing listening, the socket fails immediately rather than going quiet, so the response classifies as `StatusNoData`. Pinning this is what stops the refused path and the timeout path being mistaken for each other in either direction.
Nothing here substitutes `DNSClient` or any other DNS abstraction. The resolver dials a real 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 goes to and what is listening there — and choosing which nameserver a live query is sent to is not faking DNS, since production aims queries at nameservers of the delegation's choosing too.
The public network cannot produce these outcomes on demand: a black-holed address is not black-holed everywhere, 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. A loopback nameserver is deterministic everywhere and fast, because the test picks the deadline.
`TESTING.md` gains a section recording why this is permitted and where the line is: **substituting the client is banned; choosing the server is not.** A test that reaches for a fake `DNSClient` to force a classification remains forbidden regardless of how awkward the alternative looks.
## Production changes
- `internal/resolver/iterative.go`: new `nameserverAddr` helper. `queryDNS` previously did `net.JoinHostPort(serverIP, "53")` unconditionally, so a nameserver address that already carried a port could not be dialled as written. It now passes such an address through untouched and applies the default port only to a bare one — the normal case, and what a delegation's glue records carry. This is what makes a nameserver listening somewhere other than 53 reachable, so no production path is bypassed to arrange the tests above.
- `internal/resolver/dns_client.go`: doc comment only. `DNSClient` is now described as what it is — an abstraction over a single transport, letting the resolver switch between UDP and TCP — rather than as a testing seam.
## How the rewritten watcher tests exercise live DNS
Every watcher test resolves live DNS on every run. Change detection is driven by seeding the state store with a synthetic *previous observation* that live DNS cannot match — RFC 2606 `.invalid` nameserver names and the RFC 5737 documentation address `203.0.113.1` — and then re-running against live DNS:
- NS change: the recorded NS baseline is replaced with `ns-baseline.invalid.`; the next live run must detect the difference and notify at warning priority.
- Record change: each nameserver's recorded answer is rewritten to `203.0.113.1`; the next live run must observe the real records and notify.
- NS failure/recovery: one real live-observed nameserver is recorded as failed and a synthetic one is recorded as healthy; the next live run must report the synthetic one as disappeared and the real one as recovered.
- Stale-DNS regression (`TestDNSRunsBeforePortAndTLSChecks`): state is seeded with a stale documentation address and a port entry for it; after one cycle, port and TLS state must be keyed by the freshly live-resolved addresses and the stale key must be gone.
- Startup notification tests poll for scan completion against a deadline instead of assuming a scan duration, since live scans take variable time.
Assertions compare exact sets, not counts. `assertStatePopulated` and `TestDomainPortAndTLSChecks` require that port state, certificate state, and the arguments the port and TLS checkers were actually called with all match the addresses live DNS returned — every one of them and no others. Asserting only non-emptiness would hold just as well if the watcher had resolved the wrong name or dropped all but one of its addresses. The test doubles record their call arguments for that reason.
The port checker, TLS checker, and notifier remain test doubles — they are not DNS — and are generalized (all ports open or closed, one certificate for any address) so assertions hold for whatever addresses live DNS returns, including multi-IP and AAAA answers.
Because the whole package now runs live iterative resolutions in parallel, it gains `liveWatcherGate`, a package-scoped bound of 3 concurrent live-DNS tests. It is the counterpart to `liveGate` in `internal/resolver`: those gates cannot reach across package boundaries, so each package needs its own.
## `script/test` now passes `-p 1`
Go runs package test binaries in parallel by default. The two live-DNS gates are package-scoped and therefore per test binary, so with both live-DNS packages in flight at once their bounds sum instead of holding: the root and TLD servers rate-limit the excess and the resolver package's per-attempt deadlines start expiring. Serialising packages is what makes each gate authoritative while its package runs. `TESTING.md` records this and adds `-p 1` to the list of flags not to remove, alongside `-count=1`.
## Documentation
`TESTING.md` and `TODO.md` are updated to match: the mock ban is stated for every package, the loopback-nameserver rule is written down, and `TODO.md` records both the mock removal and the restored transport coverage.
No DNS mocking remains anywhere in the tree at this head. Full-tree grep for mock/fake/stub near DNS/resolver/nameserver terms: zero hits. timeoutClient, timeoutError, and mockResolver are gone; the miekg/dns import is gone from internal/resolver/resolver_test.go; no net.DefaultResolver overrides, dial hooks, or hosts-file tricks anywhere. The only remaining test doubles are mockPortChecker, mockTLSChecker, and mockNotifier in internal/watcher/watcher_test.go — none touches DNS. Their generalization does not hollow out the assertions: TestPortStateChange still flips open-to-closed and requires a Port Change: notification; TestTLSExpiryWarningDedup requires at least one warning and then a stable count; TestDomainPortAndTLSChecks still asserts nonzero call counts and populated state.
resolver.NewFromLoggerWithClient removal is clean. Zero remaining references in the tree; internal package, nothing external can depend on it; everything compiles and passes.
Dropped coverage is exactly as documented, plus one trivial sibling. Old vs new test enumeration: internal/watcher/watcher_test.go has 13 test functions before and after — every old scenario (baseline, NS change, record change, port change, TLS expiry + dedup, graceful shutdown, DNS-before-port/TLS ordering, startup notification x3, NS failure/recovery) survives; the NS failure/recovery test now covers both directions in one cycle. In the resolver test, TestQueryNameserverIP_Timeout became TestQueryNameserverIP_UnreachableServer; the exact StatusTimeout assertion is dropped as documented, and with it the (undocumented but subsumed) assert.NotEmpty(resp.Error) — minor, acceptable: on a live 192.0.2.1 path the failure mode genuinely varies, and the new test still requires a classified non-OK status with zero records and no error/hang. The old mockResolver error-injection fields (lookupNSErr etc.) were never exercised by any removed test, so nothing else was lost.
Seeded-state design is sound.fakeNS uses the RFC 2606 .invalid TLD, which can never appear in live NS answers; fakeIP (203.0.113.1) and 192.0.2.1 are RFC 5737 documentation addresses that IANA-operated example.com/www.example.com can never legitimately resolve to. No hard assertions on example.com record contents, no specific IP-count or ordering assumptions (liveIPs deduplicates and sorts), and no IPv6-connectivity assumption (AAAA answers are recorded from DNS; connections go only through the mocked port/TLS checkers, which accept any address). Startup-notification tests poll with a deadline instead of sleeping a fixed scan duration. Residual brittleness, noted as within the accepted live-DNS baseline (#93), not blockers: (a) TestQueryNameserverIP_UnreachableServer burns its full 10s context when packets to 192.0.2.1 drop silently (observed 10.01s locally), one third of the resolver package's 30s budget; (b) scanTimeout of 25s in the watcher tests sits close to the 30s go test timeout, so a badly degraded network would surface as a package timeout panic rather than a clean single-test failure. Observed watcher package time is ~2-3s, so headroom is real today.
make test timing: within policy. Two fully uncached runs (test cache cleared between): 12.2s and 11.6s wall, against the 20s policy budget and 30s Makefile timeout. Dominant cost is the deliberate 10s unreachable-server test running parallel inside the resolver package (11.0s package time); watcher package 2.0-2.8s.
make check: exit 0 (test + lint + fmt-check), lint reports 0 issues, fmt-check clean. Zero test failures and zero flakes across 3 total live-DNS suite executions (2 uncached). CI on head 3959aed: check / check success (50s).
#96 interplay claim confirmed.origin/golangci-v2.12.2 rewrites the mocked fixture literals in internal/watcher/watcher_test.go into constants (108+/97-) — the very code this PR deletes. The hunks overlap; whichever merges second needs a rebase (and if this merges first, most of #96's watcher_test.go delta becomes moot since this PR already introduces testDomain/testHostname constants). This branch staying on main's current lint config is coherent: local lint passes 0 issues against the current .golangci.yml.
Standard checks. No new nolint (all 15 in-tree hits pre-date this PR). No attribution trailers or vendor references anywhere. TESTING.md changes correctly extend the live-DNS rule to every package and remove the mock carve-out, consistent with the iron rule and with the #93 discussion (which this PR neither closes nor pre-empts — the default-suite gating decision there remains open and untouched). TODO.md updates are in scope (stale mock references). Commit message is accurate and descriptive; no (closes #N) because no issue defines this work — the only related open issue (#93) is intentionally not closed by it. Mergeable against current main (f79cd98): confirmed; main's one commit past the branch point touches only README.md, which this PR does not modify.
make test (uncached run 1): 12.2s, all packages ok. (uncached run 2 after go clean -testcache): 11.6s, all packages ok. Coverage: watcher 83.7%, resolver 77.4%.
## Review: PR #97 (head 3959aed)
**Verdict: PASS**
### Mandate verification
1. **No DNS mocking remains anywhere in the tree at this head.** Full-tree grep for mock/fake/stub near DNS/resolver/nameserver terms: zero hits. `timeoutClient`, `timeoutError`, and `mockResolver` are gone; the `miekg/dns` import is gone from `internal/resolver/resolver_test.go`; no `net.DefaultResolver` overrides, dial hooks, or hosts-file tricks anywhere. The only remaining test doubles are `mockPortChecker`, `mockTLSChecker`, and `mockNotifier` in `internal/watcher/watcher_test.go` — none touches DNS. Their generalization does not hollow out the assertions: `TestPortStateChange` still flips open-to-closed and requires a `Port Change: ` notification; `TestTLSExpiryWarningDedup` requires at least one warning and then a stable count; `TestDomainPortAndTLSChecks` still asserts nonzero call counts and populated state.
2. **`resolver.NewFromLoggerWithClient` removal is clean.** Zero remaining references in the tree; internal package, nothing external can depend on it; everything compiles and passes.
3. **Dropped coverage is exactly as documented, plus one trivial sibling.** Old vs new test enumeration: `internal/watcher/watcher_test.go` has 13 test functions before and after — every old scenario (baseline, NS change, record change, port change, TLS expiry + dedup, graceful shutdown, DNS-before-port/TLS ordering, startup notification x3, NS failure/recovery) survives; the NS failure/recovery test now covers both directions in one cycle. In the resolver test, `TestQueryNameserverIP_Timeout` became `TestQueryNameserverIP_UnreachableServer`; the exact `StatusTimeout` assertion is dropped as documented, and with it the (undocumented but subsumed) `assert.NotEmpty(resp.Error)` — minor, acceptable: on a live 192.0.2.1 path the failure mode genuinely varies, and the new test still requires a classified non-OK status with zero records and no error/hang. The old `mockResolver` error-injection fields (`lookupNSErr` etc.) were never exercised by any removed test, so nothing else was lost.
4. **Seeded-state design is sound.** `fakeNS` uses the RFC 2606 `.invalid` TLD, which can never appear in live NS answers; `fakeIP` (203.0.113.1) and 192.0.2.1 are RFC 5737 documentation addresses that IANA-operated `example.com`/`www.example.com` can never legitimately resolve to. No hard assertions on example.com record contents, no specific IP-count or ordering assumptions (`liveIPs` deduplicates and sorts), and no IPv6-connectivity assumption (AAAA answers are recorded from DNS; connections go only through the mocked port/TLS checkers, which accept any address). Startup-notification tests poll with a deadline instead of sleeping a fixed scan duration. Residual brittleness, noted as within the accepted live-DNS baseline (#93), not blockers: (a) `TestQueryNameserverIP_UnreachableServer` burns its full 10s context when packets to 192.0.2.1 drop silently (observed 10.01s locally), one third of the resolver package's 30s budget; (b) `scanTimeout` of 25s in the watcher tests sits close to the 30s `go test` timeout, so a badly degraded network would surface as a package timeout panic rather than a clean single-test failure. Observed watcher package time is ~2-3s, so headroom is real today.
5. **`make test` timing: within policy.** Two fully uncached runs (test cache cleared between): 12.2s and 11.6s wall, against the 20s policy budget and 30s Makefile timeout. Dominant cost is the deliberate 10s unreachable-server test running parallel inside the resolver package (11.0s package time); watcher package 2.0-2.8s.
6. **`make check`: exit 0** (test + lint + fmt-check), lint reports 0 issues, fmt-check clean. Zero test failures and zero flakes across 3 total live-DNS suite executions (2 uncached). CI on head 3959aed: `check / check` success (50s).
7. **#96 interplay claim confirmed.** `origin/golangci-v2.12.2` rewrites the mocked fixture literals in `internal/watcher/watcher_test.go` into constants (108+/97-) — the very code this PR deletes. The hunks overlap; whichever merges second needs a rebase (and if this merges first, most of #96's watcher_test.go delta becomes moot since this PR already introduces `testDomain`/`testHostname` constants). This branch staying on main's current lint config is coherent: local lint passes 0 issues against the current `.golangci.yml`.
8. **Standard checks.** No new `nolint` (all 15 in-tree hits pre-date this PR). No attribution trailers or vendor references anywhere. `TESTING.md` changes correctly extend the live-DNS rule to every package and remove the mock carve-out, consistent with the iron rule and with the #93 discussion (which this PR neither closes nor pre-empts — the default-suite gating decision there remains open and untouched). `TODO.md` updates are in scope (stale mock references). Commit message is accurate and descriptive; no `(closes #N)` because no issue defines this work — the only related open issue (#93) is intentionally not closed by it. Mergeable against current `main` (f79cd98): confirmed; main's one commit past the branch point touches only `README.md`, which this PR does not modify.
### Verification detail
- Worktree at detached head 3959aedb6a4302a7ef833d419c1dbe5ab71c5787.
- `make test` (uncached run 1): 12.2s, all packages ok. (uncached run 2 after `go clean -testcache`): 11.6s, all packages ok. Coverage: watcher 83.7%, resolver 77.4%.
- `make check`: exit 0, lint 0 issues.
- Flake count: 0 failures in 3 suite executions.
- Greps: `NewFromLoggerWithClient` 0 hits; DNS-adjacent mock/fake/stub 0 hits; `timeoutClient`/`timeoutError` 0 hits.
Manager: independent adversarial review PASSED (comment above). Setting merge-ready and assigning to sneak for merge (protected main).
Review highlights: no DNS mock/fake/stub remains anywhere in the tree (iron rule fully implemented); NewFromLoggerWithClient removal is clean with zero remaining references; 13 watcher test functions before and after (nothing silently dropped); the one documented coverage drop (exact StatusTimeout classification) plus its minor sibling (assert.NotEmpty(resp.Error)) judged acceptable — the live test still requires a classified non-OK response with zero records; make test measured at 11.6-12.2s uncached (inside the 20s policy budget), 0 flakes across 3 suite executions; CI green on head 3959aed.
Non-blocking robustness notes for the record (within the accepted #93 baseline, candidates for future hardening, not this PR): TestQueryNameserverIP_UnreachableServer burns its full 10s context when 192.0.2.1 blackholes; scanTimeout = 25s in internal/watcher/watcher_test.go sits close to the 30s go test timeout, so a badly degraded network would present as a package-timeout panic rather than a clean single-test failure.
Merge-order note: #96 (also merge-ready) rewrites the same internal/watcher/watcher_test.go region this PR deletes — whichever merges second needs a small rebase; ping this repo's manager (or leave a comment) and the rebase will be run through the loop.
Manager: independent adversarial review PASSED (comment above). Setting `merge-ready` and assigning to sneak for merge (protected `main`).
Review highlights: no DNS mock/fake/stub remains anywhere in the tree (iron rule fully implemented); `NewFromLoggerWithClient` removal is clean with zero remaining references; 13 watcher test functions before and after (nothing silently dropped); the one documented coverage drop (exact `StatusTimeout` classification) plus its minor sibling (`assert.NotEmpty(resp.Error)`) judged acceptable — the live test still requires a classified non-OK response with zero records; `make test` measured at 11.6-12.2s uncached (inside the 20s policy budget), 0 flakes across 3 suite executions; CI green on head 3959aed.
Non-blocking robustness notes for the record (within the accepted #93 baseline, candidates for future hardening, not this PR): `TestQueryNameserverIP_UnreachableServer` burns its full 10s context when 192.0.2.1 blackholes; `scanTimeout = 25s` in `internal/watcher/watcher_test.go` sits close to the 30s `go test` timeout, so a badly degraded network would present as a package-timeout panic rather than a clean single-test failure.
Merge-order note: #96 (also `merge-ready`) rewrites the same `internal/watcher/watcher_test.go` region this PR deletes — whichever merges second needs a small rebase; ping this repo's manager (or leave a comment) and the rebase will be run through the loop.
Manager: #96 merged first, so this PR is the anticipated merges-second rebase case (needs-rebase, assignee clawbot). Rebase onto current main (9347a28) dispatched: #97's live-DNS rewrite wins the internal/watcher/watcher_test.go conflict, adopting #96's goconst constants where the surviving code repeats those strings; the rebased result must additionally be lint-clean under the org-standard config now active on main (lll/funlen/cyclop/dupl thresholds live). No DNS mocking may be reintroduced during conflict resolution. A fresh independent re-review follows before this returns to merge-ready.
Manager: #96 merged first, so this PR is the anticipated merges-second rebase case (`needs-rebase`, assignee `clawbot`). Rebase onto current `main` (9347a28) dispatched: #97's live-DNS rewrite wins the `internal/watcher/watcher_test.go` conflict, adopting #96's goconst constants where the surviving code repeats those strings; the rebased result must additionally be lint-clean under the org-standard config now active on `main` (lll/funlen/cyclop/dupl thresholds live). No DNS mocking may be reintroduced during conflict resolution. A fresh independent re-review follows before this returns to `merge-ready`.
Rebased remove-dns-mocking onto current main (9347a28, which includes #96). New head: a535ae8. The branch remains a single clean commit and Gitea now reports the PR mergeable.
Conflicts hit and how they were resolved
internal/watcher/watcher_test.go: conflicted throughout, because #96 reworked the old mock-fixture constants (goconst extraction) in the same file this PR rewrites. Resolved by taking this PR's live-DNS rewrite wholesale — the mock fixtures #96 reworked are exactly the code this PR deletes. The rewrite already defines its own constants (testDomain, testHostname, fakeNS, fakeIP, etc.), so no goconst findings are reintroduced.
TODO.md: both branches added a Completed Steps entry for 2026-08-07. Kept both, with this PR's entry ordered above #96's (newest first).
internal/watcher/watcher.go was untouched by this PR, so main's version (with #96's statusError constant) carries through unchanged. TESTING.md, internal/resolver/* applied cleanly.
Lint under the now-active org-standard config
The live-DNS test code passes the v2.12.2 org-standard config with the lll/funlen/cyclop/dupl thresholds active as-is: no new findings, no fixes needed, no nolint directives added. make lint was run twice with consistent results (0 issues).
Verification
make check exits 0: fmt-check clean, full test suite green against live DNS (watcher suite ~2s, all tests passing), lint 0 issues.
The stale "whichever merges second should be rebased" note in the PR body has been replaced with the current rebase status.
Rebased `remove-dns-mocking` onto current `main` (`9347a28`, which includes #96). New head: `a535ae8`. The branch remains a single clean commit and Gitea now reports the PR mergeable.
**Conflicts hit and how they were resolved**
- `internal/watcher/watcher_test.go`: conflicted throughout, because #96 reworked the old mock-fixture constants (goconst extraction) in the same file this PR rewrites. Resolved by taking this PR's live-DNS rewrite wholesale — the mock fixtures #96 reworked are exactly the code this PR deletes. The rewrite already defines its own constants (`testDomain`, `testHostname`, `fakeNS`, `fakeIP`, etc.), so no goconst findings are reintroduced.
- `TODO.md`: both branches added a Completed Steps entry for 2026-08-07. Kept both, with this PR's entry ordered above #96's (newest first).
- `internal/watcher/watcher.go` was untouched by this PR, so `main`'s version (with #96's `statusError` constant) carries through unchanged. `TESTING.md`, `internal/resolver/*` applied cleanly.
**Lint under the now-active org-standard config**
The live-DNS test code passes the v2.12.2 org-standard config with the lll/funlen/cyclop/dupl thresholds active as-is: no new findings, no fixes needed, no `nolint` directives added. `make lint` was run twice with consistent results (0 issues).
**Verification**
`make check` exits 0: fmt-check clean, full test suite green against live DNS (watcher suite ~2s, all tests passing), lint 0 issues.
The stale "whichever merges second should be rebased" note in the PR body has been replaced with the current rebase status.
Scope of this round: verify the rebase onto main9347a28 (which merged #96) introduced no regressions relative to the round-1 PASS at head 3959aed, with the iron rule re-verified from scratch at the new head.
Findings
None.
Verification detail
Iron rule re-verified at new head, whole tree.mockResolver, timeoutClient, and resolver.NewFromLoggerWithClient: zero hits anywhere in the worktree. Broad grep for mock/fake/stub within 40 chars of dns/resolver/nameserver/lookup: every hit is policy prose banning mocks (README.md, TESTING.md, TODO.md) or the fakeNS/fakeIP seeded-prior-state literals in internal/watcher/watcher_test.go, which are synthetic previous observations written to the state store, not DNS doubles. The only test doubles are the port checker, TLS checker, and notifier — none touches DNS.
Rebase fidelity confirmed byte-for-byte. All PR result files — internal/watcher/watcher_test.go, internal/resolver/resolver_test.go, internal/resolver/resolver.go, internal/resolver/dns_client.go, TESTING.md — are identical between the round-1-passed head 3959aed and the new head a535ae8. The only inter-head file delta is TODO.md, and it is exactly #96's Completed Steps entry carried in from main; both 2026-08-07 entries are present, this PR's ordered first. git range-diff shows all patch differences confined to the deleted-lines side (the old mock code, which #96 had rewritten on main before this PR deletes it) — no new-side content changes beyond conflict-resolution necessities.
Nothing from #96 clobbered.git diff 9347a28 a535ae8 --name-only lists only the PR's six files. internal/watcher/watcher.go is identical to main (statusError constant intact at lines 32/430/730, no logic change). internal/notify/*, internal/state/*, internal/config/*, .golangci.yml, Dockerfile, and script/ are identical to main, so #96's goconst/dupl/lll fixes and the commit-pinned v2.12.2 toolchain carry through untouched.
Lint under the now-active org-standard config.make check exits 0 (test + lint + fmt-check). make lint run twice: 0 issues both times, consistent. Zero nolint occurrences in the head-vs-base diff.
Live-DNS suite behavior. Three full suite executions (one via make check, two standalone), all with the test cache cleared beforehand: 0 failures, 0 flakes. make test wall time: 11.8s and 11.7s uncached — within the 20s policy budget and consistent with round 1 (11.6-12.2s). Watcher package ~1.9s; dominant cost remains the deliberate 10s unreachable-server resolver test (accepted round 1, unchanged).
CI and mergeability. Head a535ae864b: check / check success (53s). Gitea reports the PR mergeable against current main.
Standard checks. Single clean commit on base 9347a28; commit message accurate and descriptive with no attribution trailers and no vendor references (grep of the full commit: 0 hits). PR body Note correctly reflects the completed rebase; the stale whichever-merges-second text is gone.
Round-1 non-blocking robustness notes (10s blackhole burn in the unreachable-server test; scanTimeout at 25s vs the 30s package timeout) remain accurate and remain non-blocking under the #93 baseline.
## Re-review (round 2, post-rebase): PR #97 (head a535ae8)
**Verdict: PASS**
Scope of this round: verify the rebase onto `main` 9347a28 (which merged #96) introduced no regressions relative to the round-1 PASS at head 3959aed, with the iron rule re-verified from scratch at the new head.
### Findings
None.
### Verification detail
1. **Iron rule re-verified at new head, whole tree.** `mockResolver`, `timeoutClient`, and `resolver.NewFromLoggerWithClient`: zero hits anywhere in the worktree. Broad grep for mock/fake/stub within 40 chars of dns/resolver/nameserver/lookup: every hit is policy prose banning mocks (`README.md`, `TESTING.md`, `TODO.md`) or the `fakeNS`/`fakeIP` seeded-prior-state literals in `internal/watcher/watcher_test.go`, which are synthetic previous observations written to the state store, not DNS doubles. The only test doubles are the port checker, TLS checker, and notifier — none touches DNS.
2. **Rebase fidelity confirmed byte-for-byte.** All PR result files — `internal/watcher/watcher_test.go`, `internal/resolver/resolver_test.go`, `internal/resolver/resolver.go`, `internal/resolver/dns_client.go`, `TESTING.md` — are identical between the round-1-passed head 3959aed and the new head a535ae8. The only inter-head file delta is `TODO.md`, and it is exactly #96's Completed Steps entry carried in from `main`; both 2026-08-07 entries are present, this PR's ordered first. `git range-diff` shows all patch differences confined to the deleted-lines side (the old mock code, which #96 had rewritten on `main` before this PR deletes it) — no new-side content changes beyond conflict-resolution necessities.
3. **Nothing from #96 clobbered.** `git diff 9347a28 a535ae8 --name-only` lists only the PR's six files. `internal/watcher/watcher.go` is identical to `main` (`statusError` constant intact at lines 32/430/730, no logic change). `internal/notify/*`, `internal/state/*`, `internal/config/*`, `.golangci.yml`, `Dockerfile`, and `script/` are identical to `main`, so #96's goconst/dupl/lll fixes and the commit-pinned v2.12.2 toolchain carry through untouched.
4. **Lint under the now-active org-standard config.** `make check` exits 0 (test + lint + fmt-check). `make lint` run twice: 0 issues both times, consistent. Zero `nolint` occurrences in the head-vs-base diff.
5. **Live-DNS suite behavior.** Three full suite executions (one via `make check`, two standalone), all with the test cache cleared beforehand: 0 failures, 0 flakes. `make test` wall time: 11.8s and 11.7s uncached — within the 20s policy budget and consistent with round 1 (11.6-12.2s). Watcher package ~1.9s; dominant cost remains the deliberate 10s unreachable-server resolver test (accepted round 1, unchanged).
6. **CI and mergeability.** Head a535ae864b156174b61a26c03e82e7af690757b4: `check / check` success (53s). Gitea reports the PR mergeable against current `main`.
7. **Standard checks.** Single clean commit on base 9347a28; commit message accurate and descriptive with no attribution trailers and no vendor references (grep of the full commit: 0 hits). PR body Note correctly reflects the completed rebase; the stale whichever-merges-second text is gone.
Round-1 non-blocking robustness notes (10s blackhole burn in the unreachable-server test; `scanTimeout` at 25s vs the 30s package timeout) remain accurate and remain non-blocking under the #93 baseline.
Manager: post-rebase re-review by a fresh independent reviewer PASSED with zero findings (comment above). Setting merge-ready and assigning to sneak for merge (protected main).
Rebase verified faithful: all five content files byte-identical to the previously-passed head 3959aed; only delta is TODO.md carrying #96's entry from main; nothing merged in #96 clobbered (watcher.go, notify/state/config tests, .golangci.yml, Dockerfile, script/ all identical to main); iron rule holds tree-wide (zero DNS mocks; fakeNS/fakeIP are seeded prior-state literals, not doubles); lint-clean under the org-standard config; make test 11.7-11.8s uncached, 0 flakes across 3 runs; CI green on head a535ae8; mergeable. Nothing else blocks this PR.
Manager: post-rebase re-review by a fresh independent reviewer PASSED with zero findings (comment above). Setting `merge-ready` and assigning to sneak for merge (protected `main`).
Rebase verified faithful: all five content files byte-identical to the previously-passed head 3959aed; only delta is `TODO.md` carrying #96's entry from `main`; nothing merged in #96 clobbered (`watcher.go`, notify/state/config tests, `.golangci.yml`, `Dockerfile`, `script/` all identical to `main`); iron rule holds tree-wide (zero DNS mocks; `fakeNS`/`fakeIP` are seeded prior-state literals, not doubles); lint-clean under the org-standard config; `make test` 11.7-11.8s uncached, 0 flakes across 3 runs; CI green on head a535ae8; mergeable. Nothing else blocks this PR.
DNS is never mocked in this repository: tests exercise live DNS,
and robustness comes from handling real-world DNS behavior with
tolerant assertions and sensible timeouts, not from mocks.
watcher: drop mockResolver and wire the real iterative resolver
into the tests, querying stable public names (example.com,
www.example.com). Change detection is exercised by seeding the
state store with a synthetic previous observation that live DNS
cannot match (reserved .invalid nameserver names and RFC 5737
documentation addresses); DNS stays live in every run. The port
checker, TLS checker, and notifier remain test doubles since they
are not DNS, keeping notification and state assertions
deterministic against whatever addresses live DNS returns.
resolver: drop the timeoutClient fake DNSClient and the
NewFromLoggerWithClient mock constructor. The timeout test is
replaced by a live query against an RFC 5737 documentation
address where no nameserver can exist, asserting a classified
non-OK response with no records.
TESTING.md: extend the live-DNS policy to every package and
remove the carve-out that permitted DNS mocks in packages that
consume the resolver.
TODO.md: update stale references to hermetic mocked-DNS work to
reflect the no-mocking policy and the current state of
feature/resolver.
Intentionally dropped coverage: the exact StatusTimeout
classification (previously forced by the fake client) is no
longer asserted, because a genuinely unreachable server may fail
fast instead of timing out depending on the network path; the
live test tolerantly accepts any failure classification.
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.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
DNS is never mocked in this repository: tests exercise live DNS, and robustness comes from handling real-world DNS behavior with tolerant assertions and sensible timeouts, not from mocks. This branch removes the remaining DNS mocks from the suite and, in the same change, restores the transport-failure coverage that removing them would otherwise have dropped — without reintroducing a mock.
Targets
next.What was removed
mockResolverininternal/watcher/watcher_test.go. The watcher tests are now wired to the real iterative resolver viaresolver.NewFromLoggerand query stable public names (example.com,www.example.com).timeoutClient, the fakeDNSClientininternal/resolver/resolver_test.go, and the oldTestQueryNameserverIP_Timeoutthat depended on it.resolver.NewFromLoggerWithClientininternal/resolver/resolver.go— the constructor whose only purpose was injecting mock DNS clients.TESTING.mdcarve-out that permitted DNS mocks in packages consuming the resolver. The live-DNS policy now applies to every package and to every DNS abstraction, naming the watcher'sDNSResolverinterface explicitly alongsideDNSClient.Transport-failure coverage, restored without mocks
New file
internal/resolver/transport_test.gocovers the classification branches the fake client used to cover, by binding a real UDP nameserver on127.0.0.1and aiming a live query at it:TestQueryNameserverIP_Timeout— a nameserver that takes the query and never answers must classify asStatusTimeout, with errorall queries timed outand no records. The test's nameserver is silent onAalone and answers every other type, which costs two query timeouts instead of the sixteen a wholly silent nameserver would cost; an 8s budget assertion fails loudly if that ever stops being true.TestQueryNameserverIP_ServFail— a nameserver that answers SERVFAIL must classify asStatusError, with errorserver returned SERVFAIL.TestQueryNameserverIP_NoListener— a refused datagram is not a timeout. With nothing listening, the socket fails immediately rather than going quiet, so the response classifies asStatusNoData. Pinning this is what stops the refused path and the timeout path being mistaken for each other in either direction.Nothing here substitutes
DNSClientor any other DNS abstraction. The resolver dials a real socket, writes a real DNS query with the realmiekg/dnsclient, 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 goes to and what is listening there — and choosing which nameserver a live query is sent to is not faking DNS, since production aims queries at nameservers of the delegation's choosing too.The public network cannot produce these outcomes on demand: a black-holed address is not black-holed everywhere, 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. A loopback nameserver is deterministic everywhere and fast, because the test picks the deadline.
TESTING.mdgains a section recording why this is permitted and where the line is: substituting the client is banned; choosing the server is not. A test that reaches for a fakeDNSClientto force a classification remains forbidden regardless of how awkward the alternative looks.Production changes
internal/resolver/iterative.go: newnameserverAddrhelper.queryDNSpreviously didnet.JoinHostPort(serverIP, "53")unconditionally, so a nameserver address that already carried a port could not be dialled as written. It now passes such an address through untouched and applies the default port only to a bare one — the normal case, and what a delegation's glue records carry. This is what makes a nameserver listening somewhere other than 53 reachable, so no production path is bypassed to arrange the tests above.internal/resolver/dns_client.go: doc comment only.DNSClientis now described as what it is — an abstraction over a single transport, letting the resolver switch between UDP and TCP — rather than as a testing seam.How the rewritten watcher tests exercise live DNS
Every watcher test resolves live DNS on every run. Change detection is driven by seeding the state store with a synthetic previous observation that live DNS cannot match — RFC 2606
.invalidnameserver names and the RFC 5737 documentation address203.0.113.1— and then re-running against live DNS:ns-baseline.invalid.; the next live run must detect the difference and notify at warning priority.203.0.113.1; the next live run must observe the real records and notify.TestDNSRunsBeforePortAndTLSChecks): state is seeded with a stale documentation address and a port entry for it; after one cycle, port and TLS state must be keyed by the freshly live-resolved addresses and the stale key must be gone.Assertions compare exact sets, not counts.
assertStatePopulatedandTestDomainPortAndTLSChecksrequire that port state, certificate state, and the arguments the port and TLS checkers were actually called with all match the addresses live DNS returned — every one of them and no others. Asserting only non-emptiness would hold just as well if the watcher had resolved the wrong name or dropped all but one of its addresses. The test doubles record their call arguments for that reason.The port checker, TLS checker, and notifier remain test doubles — they are not DNS — and are generalized (all ports open or closed, one certificate for any address) so assertions hold for whatever addresses live DNS returns, including multi-IP and AAAA answers.
Because the whole package now runs live iterative resolutions in parallel, it gains
liveWatcherGate, a package-scoped bound of 3 concurrent live-DNS tests. It is the counterpart toliveGateininternal/resolver: those gates cannot reach across package boundaries, so each package needs its own.script/testnow passes-p 1Go runs package test binaries in parallel by default. The two live-DNS gates are package-scoped and therefore per test binary, so with both live-DNS packages in flight at once their bounds sum instead of holding: the root and TLD servers rate-limit the excess and the resolver package's per-attempt deadlines start expiring. Serialising packages is what makes each gate authoritative while its package runs.
TESTING.mdrecords this and adds-p 1to the list of flags not to remove, alongside-count=1.Documentation
TESTING.mdandTODO.mdare updated to match: the mock ban is stated for every package, the loopback-nameserver rule is written down, andTODO.mdrecords both the mock removal and the restored transport coverage.Review: PR #97 (head
3959aed)Verdict: PASS
Mandate verification
No DNS mocking remains anywhere in the tree at this head. Full-tree grep for mock/fake/stub near DNS/resolver/nameserver terms: zero hits.
timeoutClient,timeoutError, andmockResolverare gone; themiekg/dnsimport is gone frominternal/resolver/resolver_test.go; nonet.DefaultResolveroverrides, dial hooks, or hosts-file tricks anywhere. The only remaining test doubles aremockPortChecker,mockTLSChecker, andmockNotifierininternal/watcher/watcher_test.go— none touches DNS. Their generalization does not hollow out the assertions:TestPortStateChangestill flips open-to-closed and requires aPort Change:notification;TestTLSExpiryWarningDeduprequires at least one warning and then a stable count;TestDomainPortAndTLSChecksstill asserts nonzero call counts and populated state.resolver.NewFromLoggerWithClientremoval is clean. Zero remaining references in the tree; internal package, nothing external can depend on it; everything compiles and passes.Dropped coverage is exactly as documented, plus one trivial sibling. Old vs new test enumeration:
internal/watcher/watcher_test.gohas 13 test functions before and after — every old scenario (baseline, NS change, record change, port change, TLS expiry + dedup, graceful shutdown, DNS-before-port/TLS ordering, startup notification x3, NS failure/recovery) survives; the NS failure/recovery test now covers both directions in one cycle. In the resolver test,TestQueryNameserverIP_TimeoutbecameTestQueryNameserverIP_UnreachableServer; the exactStatusTimeoutassertion is dropped as documented, and with it the (undocumented but subsumed)assert.NotEmpty(resp.Error)— minor, acceptable: on a live 192.0.2.1 path the failure mode genuinely varies, and the new test still requires a classified non-OK status with zero records and no error/hang. The oldmockResolvererror-injection fields (lookupNSErretc.) were never exercised by any removed test, so nothing else was lost.Seeded-state design is sound.
fakeNSuses the RFC 2606.invalidTLD, which can never appear in live NS answers;fakeIP(203.0.113.1) and 192.0.2.1 are RFC 5737 documentation addresses that IANA-operatedexample.com/www.example.comcan never legitimately resolve to. No hard assertions on example.com record contents, no specific IP-count or ordering assumptions (liveIPsdeduplicates and sorts), and no IPv6-connectivity assumption (AAAA answers are recorded from DNS; connections go only through the mocked port/TLS checkers, which accept any address). Startup-notification tests poll with a deadline instead of sleeping a fixed scan duration. Residual brittleness, noted as within the accepted live-DNS baseline (#93), not blockers: (a)TestQueryNameserverIP_UnreachableServerburns its full 10s context when packets to 192.0.2.1 drop silently (observed 10.01s locally), one third of the resolver package's 30s budget; (b)scanTimeoutof 25s in the watcher tests sits close to the 30sgo testtimeout, so a badly degraded network would surface as a package timeout panic rather than a clean single-test failure. Observed watcher package time is ~2-3s, so headroom is real today.make testtiming: within policy. Two fully uncached runs (test cache cleared between): 12.2s and 11.6s wall, against the 20s policy budget and 30s Makefile timeout. Dominant cost is the deliberate 10s unreachable-server test running parallel inside the resolver package (11.0s package time); watcher package 2.0-2.8s.make check: exit 0 (test + lint + fmt-check), lint reports 0 issues, fmt-check clean. Zero test failures and zero flakes across 3 total live-DNS suite executions (2 uncached). CI on head3959aed:check / checksuccess (50s).#96 interplay claim confirmed.
origin/golangci-v2.12.2rewrites the mocked fixture literals ininternal/watcher/watcher_test.gointo constants (108+/97-) — the very code this PR deletes. The hunks overlap; whichever merges second needs a rebase (and if this merges first, most of #96's watcher_test.go delta becomes moot since this PR already introducestestDomain/testHostnameconstants). This branch staying on main's current lint config is coherent: local lint passes 0 issues against the current.golangci.yml.Standard checks. No new
nolint(all 15 in-tree hits pre-date this PR). No attribution trailers or vendor references anywhere.TESTING.mdchanges correctly extend the live-DNS rule to every package and remove the mock carve-out, consistent with the iron rule and with the #93 discussion (which this PR neither closes nor pre-empts — the default-suite gating decision there remains open and untouched).TODO.mdupdates are in scope (stale mock references). Commit message is accurate and descriptive; no(closes #N)because no issue defines this work — the only related open issue (#93) is intentionally not closed by it. Mergeable against currentmain(f79cd98): confirmed; main's one commit past the branch point touches onlyREADME.md, which this PR does not modify.Verification detail
3959aedb6a.make test(uncached run 1): 12.2s, all packages ok. (uncached run 2 aftergo clean -testcache): 11.6s, all packages ok. Coverage: watcher 83.7%, resolver 77.4%.make check: exit 0, lint 0 issues.NewFromLoggerWithClient0 hits; DNS-adjacent mock/fake/stub 0 hits;timeoutClient/timeoutError0 hits.Manager: independent adversarial review PASSED (comment above). Setting
merge-readyand assigning to sneak for merge (protectedmain).Review highlights: no DNS mock/fake/stub remains anywhere in the tree (iron rule fully implemented);
NewFromLoggerWithClientremoval is clean with zero remaining references; 13 watcher test functions before and after (nothing silently dropped); the one documented coverage drop (exactStatusTimeoutclassification) plus its minor sibling (assert.NotEmpty(resp.Error)) judged acceptable — the live test still requires a classified non-OK response with zero records;make testmeasured at 11.6-12.2s uncached (inside the 20s policy budget), 0 flakes across 3 suite executions; CI green on head3959aed.Non-blocking robustness notes for the record (within the accepted #93 baseline, candidates for future hardening, not this PR):
TestQueryNameserverIP_UnreachableServerburns its full 10s context when 192.0.2.1 blackholes;scanTimeout = 25sininternal/watcher/watcher_test.gosits close to the 30sgo testtimeout, so a badly degraded network would present as a package-timeout panic rather than a clean single-test failure.Merge-order note: #96 (also
merge-ready) rewrites the sameinternal/watcher/watcher_test.goregion this PR deletes — whichever merges second needs a small rebase; ping this repo's manager (or leave a comment) and the rebase will be run through the loop.@clawbot merged the other one first, fix pls
Manager: #96 merged first, so this PR is the anticipated merges-second rebase case (
needs-rebase, assigneeclawbot). Rebase onto currentmain(9347a28) dispatched: #97's live-DNS rewrite wins theinternal/watcher/watcher_test.goconflict, adopting #96's goconst constants where the surviving code repeats those strings; the rebased result must additionally be lint-clean under the org-standard config now active onmain(lll/funlen/cyclop/dupl thresholds live). No DNS mocking may be reintroduced during conflict resolution. A fresh independent re-review follows before this returns tomerge-ready.Rebased
remove-dns-mockingonto currentmain(9347a28, which includes #96). New head:a535ae8. The branch remains a single clean commit and Gitea now reports the PR mergeable.Conflicts hit and how they were resolved
internal/watcher/watcher_test.go: conflicted throughout, because #96 reworked the old mock-fixture constants (goconst extraction) in the same file this PR rewrites. Resolved by taking this PR's live-DNS rewrite wholesale — the mock fixtures #96 reworked are exactly the code this PR deletes. The rewrite already defines its own constants (testDomain,testHostname,fakeNS,fakeIP, etc.), so no goconst findings are reintroduced.TODO.md: both branches added a Completed Steps entry for 2026-08-07. Kept both, with this PR's entry ordered above #96's (newest first).internal/watcher/watcher.gowas untouched by this PR, somain's version (with #96'sstatusErrorconstant) carries through unchanged.TESTING.md,internal/resolver/*applied cleanly.Lint under the now-active org-standard config
The live-DNS test code passes the v2.12.2 org-standard config with the lll/funlen/cyclop/dupl thresholds active as-is: no new findings, no fixes needed, no
nolintdirectives added.make lintwas run twice with consistent results (0 issues).Verification
make checkexits 0: fmt-check clean, full test suite green against live DNS (watcher suite ~2s, all tests passing), lint 0 issues.The stale "whichever merges second should be rebased" note in the PR body has been replaced with the current rebase status.
Re-review (round 2, post-rebase): PR #97 (head
a535ae8)Verdict: PASS
Scope of this round: verify the rebase onto
main9347a28(which merged #96) introduced no regressions relative to the round-1 PASS at head3959aed, with the iron rule re-verified from scratch at the new head.Findings
None.
Verification detail
Iron rule re-verified at new head, whole tree.
mockResolver,timeoutClient, andresolver.NewFromLoggerWithClient: zero hits anywhere in the worktree. Broad grep for mock/fake/stub within 40 chars of dns/resolver/nameserver/lookup: every hit is policy prose banning mocks (README.md,TESTING.md,TODO.md) or thefakeNS/fakeIPseeded-prior-state literals ininternal/watcher/watcher_test.go, which are synthetic previous observations written to the state store, not DNS doubles. The only test doubles are the port checker, TLS checker, and notifier — none touches DNS.Rebase fidelity confirmed byte-for-byte. All PR result files —
internal/watcher/watcher_test.go,internal/resolver/resolver_test.go,internal/resolver/resolver.go,internal/resolver/dns_client.go,TESTING.md— are identical between the round-1-passed head3959aedand the new heada535ae8. The only inter-head file delta isTODO.md, and it is exactly #96's Completed Steps entry carried in frommain; both 2026-08-07 entries are present, this PR's ordered first.git range-diffshows all patch differences confined to the deleted-lines side (the old mock code, which #96 had rewritten onmainbefore this PR deletes it) — no new-side content changes beyond conflict-resolution necessities.Nothing from #96 clobbered.
git diff 9347a28 a535ae8 --name-onlylists only the PR's six files.internal/watcher/watcher.gois identical tomain(statusErrorconstant intact at lines 32/430/730, no logic change).internal/notify/*,internal/state/*,internal/config/*,.golangci.yml,Dockerfile, andscript/are identical tomain, so #96's goconst/dupl/lll fixes and the commit-pinned v2.12.2 toolchain carry through untouched.Lint under the now-active org-standard config.
make checkexits 0 (test + lint + fmt-check).make lintrun twice: 0 issues both times, consistent. Zeronolintoccurrences in the head-vs-base diff.Live-DNS suite behavior. Three full suite executions (one via
make check, two standalone), all with the test cache cleared beforehand: 0 failures, 0 flakes.make testwall time: 11.8s and 11.7s uncached — within the 20s policy budget and consistent with round 1 (11.6-12.2s). Watcher package ~1.9s; dominant cost remains the deliberate 10s unreachable-server resolver test (accepted round 1, unchanged).CI and mergeability. Head
a535ae864b:check / checksuccess (53s). Gitea reports the PR mergeable against currentmain.Standard checks. Single clean commit on base 9347a28; commit message accurate and descriptive with no attribution trailers and no vendor references (grep of the full commit: 0 hits). PR body Note correctly reflects the completed rebase; the stale whichever-merges-second text is gone.
Round-1 non-blocking robustness notes (10s blackhole burn in the unreachable-server test;
scanTimeoutat 25s vs the 30s package timeout) remain accurate and remain non-blocking under the #93 baseline.Manager: post-rebase re-review by a fresh independent reviewer PASSED with zero findings (comment above). Setting
merge-readyand assigning to sneak for merge (protectedmain).Rebase verified faithful: all five content files byte-identical to the previously-passed head 3959aed; only delta is
TODO.mdcarrying #96's entry frommain; nothing merged in #96 clobbered (watcher.go, notify/state/config tests,.golangci.yml,Dockerfile,script/all identical tomain); iron rule holds tree-wide (zero DNS mocks;fakeNS/fakeIPare seeded prior-state literals, not doubles); lint-clean under the org-standard config;make test11.7-11.8s uncached, 0 flakes across 3 runs; CI green on head a535ae8; mergeable. Nothing else blocks this PR.clawbot referenced this pull request2026-08-09 07:42:27 +02:00
a535ae864bto62dec447e36b0fdc477atoaa3da062a2Pull request closed