Live-DNS test harness never retries an answer that is missing the record type under test, so TestQueryNameserver_TXT flakes #143

New Issue

2026-09-04T02:23:38+02:00

clawbot commented

2026-09-04 02:23:38 +02:00

Found by the round-2 reviewer of PR #97. Not caused by that PR — the failing test is untouched and byte-identical on next, so it was not blocked on; blocking would have fixed nothing.

Problem

liveQueryNameserver in internal/resolver/livedns_test.go retries only when the response status is StatusTimeout or StatusError:

if resp.Status == resolver.StatusTimeout ||
    resp.Status == resolver.StatusError {
    return fmt.Errorf(...)   // retry
}
out = resp                   // accepted as final

A response that arrives but carries no records of the type the caller cares about is therefore accepted as final. The helper's doc comment states this deliberately — "NXDOMAIN and NODATA are answers and are returned to the caller to assert on" — but that design collides with the callers that assert on a specific record type.

TestQueryNameserver_TXT calls it and then does require.NotEmpty(t, resp.Records["TXT"]). When the authoritative server answers without TXT records, the retry harness does not fire and the assertion fails instead.

Evidence

make check returned exit 2 on a clean checkout. Attribution was measured over 19 alternating full-suite runs: 2 failures in 10 at PR #97's head, 0 in 9 on next (p ≈ 0.47, both failures inside one early window). Not statistically significant, and the defect is present on next regardless.

Expected

Callers asserting on a particular record type should get a retry when that type is absent, rather than a hard failure on the first empty answer. The fix should keep NXDOMAIN/NODATA meaningful for the callers that genuinely want to assert on them — probably a separate helper, or a caller-supplied "answer is satisfactory" predicate, rather than widening the retry condition for everyone.

Notes

This is the last known nondeterministic failure in internal/resolver, the subject of #93. #93's own definition of done requires make check green "across repeated consecutive runs"; that is not currently true on next.
Related but distinct from #141 (tryExchange downgrading StatusTimeout to StatusNoData), which is a production-code defect. This one is confined to the test harness.
Related to #139: a repeat green from Go's test cache would hide this entirely.

Found by the round-2 reviewer of [PR #97](https://git.eeqj.de/sneak/dnswatcher/pulls/97). **Not caused by that PR** — the failing test is untouched and byte-identical on `next`, so it was not blocked on; blocking would have fixed nothing. ## Problem `liveQueryNameserver` in `internal/resolver/livedns_test.go` retries only when the response status is `StatusTimeout` or `StatusError`: ```go if resp.Status == resolver.StatusTimeout || resp.Status == resolver.StatusError { return fmt.Errorf(...) // retry } out = resp // accepted as final ``` A response that *arrives* but carries no records of the type the caller cares about is therefore accepted as final. The helper's doc comment states this deliberately — "NXDOMAIN and NODATA are answers and are returned to the caller to assert on" — but that design collides with the callers that assert on a specific record type. `TestQueryNameserver_TXT` calls it and then does `require.NotEmpty(t, resp.Records["TXT"])`. When the authoritative server answers without TXT records, the retry harness does not fire and the assertion fails instead. ## Evidence `make check` returned exit 2 on a clean checkout. Attribution was measured over 19 alternating full-suite runs: **2 failures in 10 at PR #97's head, 0 in 9 on `next`** (p ≈ 0.47, both failures inside one early window). Not statistically significant, and the defect is present on `next` regardless. ## Expected Callers asserting on a particular record type should get a retry when that type is absent, rather than a hard failure on the first empty answer. The fix should keep NXDOMAIN/NODATA meaningful for the callers that genuinely want to assert on them — probably a separate helper, or a caller-supplied "answer is satisfactory" predicate, rather than widening the retry condition for everyone. ## Notes - This is the last known nondeterministic failure in `internal/resolver`, the subject of [#93](https://git.eeqj.de/sneak/dnswatcher/issues/93). #93's own definition of done requires `make check` green "across repeated consecutive runs"; that is not currently true on `next`. - Related but distinct from [#141](https://git.eeqj.de/sneak/dnswatcher/issues/141) (`tryExchange` downgrading `StatusTimeout` to `StatusNoData`), which is a production-code defect. This one is confined to the test harness. - Related to [#139](https://git.eeqj.de/sneak/dnswatcher/issues/139): a repeat green from Go's test cache would hide this entirely.

~~clawbot referenced this issue 2026-09-04 02:24:42 +02:00~~

Remove DNS mocking from tests #97

~~clawbot referenced this issue 2026-09-04 02:42:17 +02:00~~

Remove DNS mocking from tests #97

clawbot commented

2026-09-05 11:32:51 +02:00

Closed at sneak's instruction, 2026-09-05: this was opened by an agent running on another machine outside the managed fleet, under superseded rules. Reopen if the content is wanted.

Model: fable-5-1

Closed at sneak's instruction, 2026-09-05: this was opened by an agent running on another machine outside the managed fleet, under superseded rules. Reopen if the content is wanted. Model: fable-5-1

clawbot closed this issue

2026-09-05 11:32:51 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/dnswatcher#143