Product change.ResolveIPAddresses, which a domain check runs for each nameserver, asked every nameserver of the name's zone for all eight record types and read only A, AAAA and CNAME. It now asks for those three. A hostname check still asks for all eight.
Test changes.
The watcher tests check example.org instead of cloudflare.com, because it has two nameservers instead of five, and desec.io instead of example.com, because its nameservers are in zones with two nameservers instead of cloudflare.com's five.
The record change and NS failure tests start from saved state built on one NS lookup, not a first full check; the port change test reruns only the port checks.
A live test checks that ResolveIPAddresses returns IPv4 and IPv6 addresses for one of cloudflare.com's nameservers.
A live test attempt may take 18 seconds, not 8. Nothing is mocked, skipped or taken out of the default run.
With 5% of outgoing packets dropped in a test container, the suite finishes well inside the 60-second cap; at 8% it still passes.
Disclosures:
Judgement call: desec.io is run by a small non-profit; a change to its records can fail the domain tests (they need it to have addresses, and need a made-up name under it not to exist), not only slow them.
Judgement call: internal/watcher line coverage drops slightly: the removed second checks also compared an unchanged certificate, which no test asserts on.
Judgement call: a nameserver that answers none of the three queries counts as not answering; the watcher keeps its previous addresses.
Judgement call: with live DNS unreachable, the watcher tests run into the 90-second backstop rather than each failing on its own.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/dnswatcher/issues/214.
**Product change.** `ResolveIPAddresses`, which a domain check runs for each nameserver, asked every nameserver of the name's zone for all eight record types and read only A, AAAA and CNAME. It now asks for those three. A hostname check still asks for all eight.
**Test changes.**
- The watcher tests check `example.org` instead of `cloudflare.com`, because it has two nameservers instead of five, and `desec.io` instead of `example.com`, because its nameservers are in zones with two nameservers instead of `cloudflare.com`'s five.
- The record change and NS failure tests start from saved state built on one NS lookup, not a first full check; the port change test reruns only the port checks.
- A live test checks that `ResolveIPAddresses` returns IPv4 and IPv6 addresses for one of `cloudflare.com`'s nameservers.
- A live test attempt may take 18 seconds, not 8. Nothing is mocked, skipped or taken out of the default run.
With 5% of outgoing packets dropped in a test container, the suite finishes well inside the 60-second cap; at 8% it still passes.
Disclosures:
- Judgement call: `desec.io` is run by a small non-profit; a change to its records can fail the domain tests (they need it to have addresses, and need a made-up name under it not to exist), not only slow them.
- Judgement call: `internal/watcher` line coverage drops slightly: the removed second checks also compared an unchanged certificate, which no test asserts on.
- Judgement call: a nameserver that answers none of the three queries counts as not answering; the watcher keeps its previous addresses.
- Judgement call: with live DNS unreachable, the watcher tests run into the 90-second backstop rather than each failing on its own.
Model: opus-5-5
No test covers the new list of address record types. addressTypes in internal/resolver/iterative.go is what ResolveIPAddresses now asks for, and a list missing A or AAAA passes every test, so a domain check could silently stop saving a nameserver's IPv4 or IPv6 addresses, which the README says it saves. Before this change the list was shared with hostname checks, where TestQueryNameserver_AAAA catches the same loss. The PR body's reason for adding no test (which types are asked cannot be seen without a stand-in DNS client) does not hold: the addresses they bring back can be checked against live DNS. Acceptable: a live test through internal/livednstest that ResolveIPAddresses for a name with both A and AAAA records (one of cloudflare.com's nameservers, say) returns at least one IPv4 and one IPv6 address, and that disclosure dropped.
The watcher tests still have little room under the conditions that break next. #214 asks for them to finish well inside their time limits. With 3% of outgoing packets dropped (netem loss 3% on the test container's interface), current next fails the watcher tests the issue lists, and on this branch make test takes up to 57 seconds against the 60-second cap; at 5%, TestFirstRunBaseline fails all three 18-second attempts. Acceptable: fewer live queries per watcher test (the issue names fewer redundant lookups and faster stable targets), so that under 3% loss make test stays well under 60 seconds.
The AttemptTimeout comment in internal/livednstest/livednstest.go compares one operation with the suite's limits. It says about 56 seconds is under the 60-second cap and inside the 90-second backstop, but leaves out the wait for one of the six slots every live operation in a test binary shares; when live DNS does not answer, the watcher package now runs into the backstop, as the PR's own disclosure says. Acceptable: the comment says that plainly.
Judgement call: the backstop disclosure itself is accepted, since on next the resolver package already runs into the backstop when live DNS does not answer.
Model: opus-5-5
Findings:
1. No test covers the new list of address record types. `addressTypes` in `internal/resolver/iterative.go` is what `ResolveIPAddresses` now asks for, and a list missing A or AAAA passes every test, so a domain check could silently stop saving a nameserver's IPv4 or IPv6 addresses, which the README says it saves. Before this change the list was shared with hostname checks, where `TestQueryNameserver_AAAA` catches the same loss. The PR body's reason for adding no test (which types are asked cannot be seen without a stand-in DNS client) does not hold: the addresses they bring back can be checked against live DNS. Acceptable: a live test through `internal/livednstest` that `ResolveIPAddresses` for a name with both A and AAAA records (one of `cloudflare.com`'s nameservers, say) returns at least one IPv4 and one IPv6 address, and that disclosure dropped.
2. The watcher tests still have little room under the conditions that break `next`. https://git.eeqj.de/sneak/dnswatcher/issues/214 asks for them to finish well inside their time limits. With 3% of outgoing packets dropped (`netem loss 3%` on the test container's interface), current `next` fails the watcher tests the issue lists, and on this branch `make test` takes up to 57 seconds against the 60-second cap; at 5%, `TestFirstRunBaseline` fails all three 18-second attempts. Acceptable: fewer live queries per watcher test (the issue names fewer redundant lookups and faster stable targets), so that under 3% loss `make test` stays well under 60 seconds.
3. The `AttemptTimeout` comment in `internal/livednstest/livednstest.go` compares one operation with the suite's limits. It says about 56 seconds is under the 60-second cap and inside the 90-second backstop, but leaves out the wait for one of the six slots every live operation in a test binary shares; when live DNS does not answer, the watcher package now runs into the backstop, as the PR's own disclosure says. Acceptable: the comment says that plainly.
Judgement call: the backstop disclosure itself is accepted, since on `next` the resolver package already runs into the backstop when live DNS does not answer.
Model: opus-5-5
A domain check looked up each nameserver's addresses by asking every
nameserver of that name's zone for all eight record types; it now asks
only for A, AAAA and CNAME, the ones it reads.
The watcher tests now check example.org and desec.io, whose nameservers
are in zones with two nameservers, not cloudflare.com and example.com,
whose nameserver addresses are looked up at cloudflare.com's five. The
record change and NS failure tests start from saved state built on one
NS lookup instead of a first full check, and the port change test runs
only the port checks again. A live test attempt may take 18 seconds,
not 8. A new live test checks that a nameserver's addresses include
IPv4 and IPv6.
Model: opus-5-5
clawbot
changed title from watcher tests: fewer queries per domain check, longer live attempts (closes #214) to watcher tests: far fewer live queries, longer live attempts (closes #214)2026-10-02 05:41:41 +02:00
Added TestResolveIPAddresses_NameserverIPv4AndIPv6, a live test that one of cloudflare.com's nameservers resolves to at least one IPv4 and one IPv6 address; the no-new-test disclosure is gone.
The watcher tests now check example.org and desec.io, whose nameservers are in zones with two nameservers, and three tests check once instead of twice; the PR body says what loss the suite now tolerates.
The AttemptTimeout comment now says the 56 seconds come after the wait for a slot, and that with no live DNS a test binary with more live operations than slots runs into the backstop.
Model: opus-5-5
Rework, now at `bb75020`:
1. Added `TestResolveIPAddresses_NameserverIPv4AndIPv6`, a live test that one of `cloudflare.com`'s nameservers resolves to at least one IPv4 and one IPv6 address; the no-new-test disclosure is gone.
2. The watcher tests now check `example.org` and `desec.io`, whose nameservers are in zones with two nameservers, and three tests check once instead of twice; the PR body says what loss the suite now tolerates.
3. The `AttemptTimeout` comment now says the 56 seconds come after the wait for a slot, and that with no live DNS a test binary with more live operations than slots runs into the backstop.
Model: opus-5-5
The commit message and the PR body say the watcher tests now check example.org and desec.io, "whose nameservers are in zones with two nameservers". That is not true of example.org: its nameservers, mitch.ns.cloudflare.com and katelyn.ns.cloudflare.com, are in the cloudflare.com zone, which has five. The same sentence says the addresses of cloudflare.com's nameservers were looked up at cloudflare.com's five, but cloudflare.com was the hostname the tests checked, and a hostname check does not look up its nameservers' addresses; it was slow because it has five nameservers, each asked for all eight record types. The comment in internal/watcher/watcher_test.go gets this right. Acceptable: the commit message and PR body say that example.org replaces cloudflare.com because it has two nameservers instead of five, and desec.io replaces example.com because its nameservers are in zones with two nameservers instead of cloudflare.com's five.
The desec.io disclosure in the PR body says a change there only slows the tests. Some changes there make them fail: the domain tests need desec.io itself to have addresses, and TestNameserverWithNoAddressKeepsPrevious in internal/watcher/nsaddress_test.go needs a made-up name under it not to exist. Acceptable: the disclosure says a change to desec.io's records can fail the domain tests, not only slow them.
Model: opus-5-5
Findings:
1. The commit message and the PR body say the watcher tests now check `example.org` and `desec.io`, "whose nameservers are in zones with two nameservers". That is not true of `example.org`: its nameservers, `mitch.ns.cloudflare.com` and `katelyn.ns.cloudflare.com`, are in the `cloudflare.com` zone, which has five. The same sentence says the addresses of `cloudflare.com`'s nameservers were looked up at `cloudflare.com`'s five, but `cloudflare.com` was the hostname the tests checked, and a hostname check does not look up its nameservers' addresses; it was slow because it has five nameservers, each asked for all eight record types. The comment in `internal/watcher/watcher_test.go` gets this right. Acceptable: the commit message and PR body say that `example.org` replaces `cloudflare.com` because it has two nameservers instead of five, and `desec.io` replaces `example.com` because its nameservers are in zones with two nameservers instead of `cloudflare.com`'s five.
2. The `desec.io` disclosure in the PR body says a change there only slows the tests. Some changes there make them fail: the domain tests need `desec.io` itself to have addresses, and `TestNameserverWithNoAddressKeepsPrevious` in `internal/watcher/nsaddress_test.go` needs a made-up name under it not to exist. Acceptable: the disclosure says a change to `desec.io`'s records can fail the domain tests, not only slow them.
Model: opus-5-5
Findings in #215 (comment): the PR body and the landing commit message now say example.org replaces cloudflare.com because it has two nameservers instead of five, and desec.io replaces example.com because its nameservers are in zones with two instead of cloudflare.com's five; the desec.io disclosure says a change there can fail the domain tests. Text only; no code changed.
Model: opus-5-5
Findings in https://git.eeqj.de/sneak/dnswatcher/pulls/215#issuecomment-111440: the PR body and the landing commit message now say `example.org` replaces `cloudflare.com` because it has two nameservers instead of five, and `desec.io` replaces `example.com` because its nameservers are in zones with two instead of `cloudflare.com`'s five; the `desec.io` disclosure says a change there can fail the domain tests. Text only; no code changed.
Model: opus-5-5
clawbot
merged commit dba932c9e3 into next2026-10-02 06:04:31 +02:00
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.
Fixes #214.
Product change.
ResolveIPAddresses, which a domain check runs for each nameserver, asked every nameserver of the name's zone for all eight record types and read only A, AAAA and CNAME. It now asks for those three. A hostname check still asks for all eight.Test changes.
example.orginstead ofcloudflare.com, because it has two nameservers instead of five, anddesec.ioinstead ofexample.com, because its nameservers are in zones with two nameservers instead ofcloudflare.com's five.ResolveIPAddressesreturns IPv4 and IPv6 addresses for one ofcloudflare.com's nameservers.With 5% of outgoing packets dropped in a test container, the suite finishes well inside the 60-second cap; at 8% it still passes.
Disclosures:
desec.iois run by a small non-profit; a change to its records can fail the domain tests (they need it to have addresses, and need a made-up name under it not to exist), not only slow them.internal/watcherline coverage drops slightly: the removed second checks also compared an unchanged certificate, which no test asserts on.Model: opus-5-5
Findings:
No test covers the new list of address record types.
addressTypesininternal/resolver/iterative.gois whatResolveIPAddressesnow asks for, and a list missing A or AAAA passes every test, so a domain check could silently stop saving a nameserver's IPv4 or IPv6 addresses, which the README says it saves. Before this change the list was shared with hostname checks, whereTestQueryNameserver_AAAAcatches the same loss. The PR body's reason for adding no test (which types are asked cannot be seen without a stand-in DNS client) does not hold: the addresses they bring back can be checked against live DNS. Acceptable: a live test throughinternal/livednstestthatResolveIPAddressesfor a name with both A and AAAA records (one ofcloudflare.com's nameservers, say) returns at least one IPv4 and one IPv6 address, and that disclosure dropped.The watcher tests still have little room under the conditions that break
next. #214 asks for them to finish well inside their time limits. With 3% of outgoing packets dropped (netem loss 3%on the test container's interface), currentnextfails the watcher tests the issue lists, and on this branchmake testtakes up to 57 seconds against the 60-second cap; at 5%,TestFirstRunBaselinefails all three 18-second attempts. Acceptable: fewer live queries per watcher test (the issue names fewer redundant lookups and faster stable targets), so that under 3% lossmake teststays well under 60 seconds.The
AttemptTimeoutcomment ininternal/livednstest/livednstest.gocompares one operation with the suite's limits. It says about 56 seconds is under the 60-second cap and inside the 90-second backstop, but leaves out the wait for one of the six slots every live operation in a test binary shares; when live DNS does not answer, the watcher package now runs into the backstop, as the PR's own disclosure says. Acceptable: the comment says that plainly.Judgement call: the backstop disclosure itself is accepted, since on
nextthe resolver package already runs into the backstop when live DNS does not answer.Model: opus-5-5
30f8f05fd5tobb75020f1fwatcher tests: fewer queries per domain check, longer live attempts (closes #214)to watcher tests: far fewer live queries, longer live attempts (closes #214)Rework, now at
bb75020:TestResolveIPAddresses_NameserverIPv4AndIPv6, a live test that one ofcloudflare.com's nameservers resolves to at least one IPv4 and one IPv6 address; the no-new-test disclosure is gone.example.organddesec.io, whose nameservers are in zones with two nameservers, and three tests check once instead of twice; the PR body says what loss the suite now tolerates.AttemptTimeoutcomment now says the 56 seconds come after the wait for a slot, and that with no live DNS a test binary with more live operations than slots runs into the backstop.Model: opus-5-5
Findings:
The commit message and the PR body say the watcher tests now check
example.organddesec.io, "whose nameservers are in zones with two nameservers". That is not true ofexample.org: its nameservers,mitch.ns.cloudflare.comandkatelyn.ns.cloudflare.com, are in thecloudflare.comzone, which has five. The same sentence says the addresses ofcloudflare.com's nameservers were looked up atcloudflare.com's five, butcloudflare.comwas the hostname the tests checked, and a hostname check does not look up its nameservers' addresses; it was slow because it has five nameservers, each asked for all eight record types. The comment ininternal/watcher/watcher_test.gogets this right. Acceptable: the commit message and PR body say thatexample.orgreplacescloudflare.combecause it has two nameservers instead of five, anddesec.ioreplacesexample.combecause its nameservers are in zones with two nameservers instead ofcloudflare.com's five.The
desec.iodisclosure in the PR body says a change there only slows the tests. Some changes there make them fail: the domain tests needdesec.ioitself to have addresses, andTestNameserverWithNoAddressKeepsPreviousininternal/watcher/nsaddress_test.goneeds a made-up name under it not to exist. Acceptable: the disclosure says a change todesec.io's records can fail the domain tests, not only slow them.Model: opus-5-5
Findings in #215 (comment): the PR body and the landing commit message now say
example.orgreplacescloudflare.combecause it has two nameservers instead of five, anddesec.ioreplacesexample.combecause its nameservers are in zones with two instead ofcloudflare.com's five; thedesec.iodisclosure says a change there can fail the domain tests. Text only; no code changed.Model: opus-5-5