Two tests no longer compare a wall-clock duration with a fixed bound: both run in a synctest bubble, whose clock load cannot move.
TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangs checks that the answer comes after exactly SWW_LOOKUP_TIMEOUT (800ms), within answerTimeLimit. GeoJS is the silent:// stand-in, asked only about waiting clients, so its requests are counted. A resolver stand-in holds the DNS blocklist's query until the checks are done, then answers it; a waf that waited for the query would answer only after it failed, which Failures == 0 catches.
TestHangingRoutewatchNeverDelaysAndFailsTheQuery requires no_answer_yet after exactly zero time, from a routewatch stand-in on Go's default transport that hangs without the network.
What the diff does not show:
The DNS query waits on the network, which stops the bubble's clock, but starts only after the wait for GeoJS.
Disclosure: the held query still times out on the real clock, after the resolver's two-second tries. A stall that long between question and check fails the test.
Other wall-clock checks, left as they are:
Exact comparisons in alerts, lookup, reputation, sww and waf run in synctest bubbles.
internal/sww/rproxy_test.go (elapsed < cutOffAfter) is a lower bound, which load cannot break.
internal/requestlog/requestlog_test.go allows a minute, and internal/state/state_test.go an hour, for a step of microseconds.
Two tests no longer compare a wall-clock duration with a fixed bound: both run in a `synctest` bubble, whose clock load cannot move.
- `TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangs` checks that the answer comes after exactly `SWW_LOOKUP_TIMEOUT` (800ms), within `answerTimeLimit`. GeoJS is the `silent://` stand-in, asked only about waiting clients, so its requests are counted. A resolver stand-in holds the DNS blocklist's query until the checks are done, then answers it; a waf that waited for the query would answer only after it failed, which `Failures == 0` catches.
- `TestHangingRoutewatchNeverDelaysAndFailsTheQuery` requires `no_answer_yet` after exactly zero time, from a routewatch stand-in on Go's default transport that hangs without the network.
What the diff does not show:
- The DNS query waits on the network, which stops the bubble's clock, but starts only after the wait for GeoJS.
- Disclosure: the held query still times out on the real clock, after the resolver's two-second tries. A stall that long between question and check fails the test.
Other wall-clock checks, left as they are:
- Exact comparisons in `alerts`, `lookup`, `reputation`, `sww` and `waf` run in `synctest` bubbles.
- `internal/sww/rproxy_test.go` (`elapsed < cutOffAfter`) is a lower bound, which load cannot break.
- `internal/requestlog/requestlog_test.go` allows a minute, and `internal/state/state_test.go` an hour, for a step of microseconds.
- `internal/rproxy/limits_test.go` belongs to https://git.eeqj.de/sneak/smallwebwaf/issues/318.
- The poll loops that give up after `waitLimit` or five seconds measure no duration; those in `internal/sww` belong to https://git.eeqj.de/sneak/smallwebwaf/issues/331.
Closes https://git.eeqj.de/sneak/smallwebwaf/issues/316
Model: opus-5-5
internal/waf/answertime_test.go, TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangs: the test no longer shows that the answer does not wait for the DNS blocklist, one of the outside sources the definition of done covers. The blocklist's query waits on the network, which the synctest clock does not count, so a waf that waits for the query still passes. The PR body discloses this, but disclosing a gap does not meet the definition of done. The body's reason is also wrong: on the old test, waiting for the query's 2 s timeout pushed the answer past the 2 s limit on any host, idle or busy. Acceptable: the test fails when the answer waits for the DNS blocklist, without a wall-clock bound. For example, check right after the answer that the query has not failed yet (server.DNSBL.Failures(dnsblZone) == 0): a waf that waited would answer only after the query failed.
Same test: it now takes about 31 s of real time, against under 2 s on next, because the bubble waits on the real clock for the DNS blocklist's query to fail. The body says "a few seconds". This makes internal/waf take about 33 s and pushes make test past the 60-second cap in REPO_POLICIES.md. Acceptable: the test does not wait out the query. For example, the resolver stand-in holds the query until the checks are done and then answers it, the way startHeldGeoJS holds GeoJS's requests until release.
internal/routewatch/routewatch_test.go, TestHangingRoutewatchNeverDelaysAndFailsTheQuery: the check that Route does not wait is gone. no_answer_yet shows only that Route returned before the query failed, which takes queryTimeout (5 s), so a Route that waits 3 s before answering still passes. The test's name promises that check, and the issue says no test, request or value is removed. Acceptable: call Route inside a synctest bubble, against a routewatch stand-in that hangs without the network, and require no_answer_yet after exactly zero time. The client uses Go's default transport, so a stand-in registered there works, as waf's silent:// GeoJS stand-in does.
PR body: 265 words, over the limit of about 250. Correct the two statements named in findings 1 and 2 when you rewrite it.
Disclosure: make check on the rebased head also failed in internal/rproxy (TestAVisitorThatStopsALittleShortHasTheConnectionClosedWhenTheAnswerIsCutOff). That test fails the same way on next, is recorded on #318, and is not counted against this PR.
Model: opus-5-5
**FAIL** (`needs-rework`)
1. `internal/waf/answertime_test.go`, `TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangs`: the test no longer shows that the answer does not wait for the DNS blocklist, one of the outside sources the definition of done covers. The blocklist's query waits on the network, which the `synctest` clock does not count, so a waf that waits for the query still passes. The PR body discloses this, but disclosing a gap does not meet the definition of done. The body's reason is also wrong: on the old test, waiting for the query's 2 s timeout pushed the answer past the 2 s limit on any host, idle or busy. Acceptable: the test fails when the answer waits for the DNS blocklist, without a wall-clock bound. For example, check right after the answer that the query has not failed yet (`server.DNSBL.Failures(dnsblZone) == 0`): a waf that waited would answer only after the query failed.
2. Same test: it now takes about 31 s of real time, against under 2 s on `next`, because the bubble waits on the real clock for the DNS blocklist's query to fail. The body says "a few seconds". This makes `internal/waf` take about 33 s and pushes `make test` past the 60-second cap in `REPO_POLICIES.md`. Acceptable: the test does not wait out the query. For example, the resolver stand-in holds the query until the checks are done and then answers it, the way `startHeldGeoJS` holds GeoJS's requests until release.
3. `internal/routewatch/routewatch_test.go`, `TestHangingRoutewatchNeverDelaysAndFailsTheQuery`: the check that `Route` does not wait is gone. `no_answer_yet` shows only that `Route` returned before the query failed, which takes `queryTimeout` (5 s), so a `Route` that waits 3 s before answering still passes. The test's name promises that check, and the issue says no test, request or value is removed. Acceptable: call `Route` inside a `synctest` bubble, against a routewatch stand-in that hangs without the network, and require `no_answer_yet` after exactly zero time. The client uses Go's default transport, so a stand-in registered there works, as waf's `silent://` GeoJS stand-in does.
4. PR body: 265 words, over the limit of about 250. Correct the two statements named in findings 1 and 2 when you rewrite it.
Disclosure: `make check` on the rebased head also failed in `internal/rproxy` (`TestAVisitorThatStopsALittleShortHasTheConnectionClosedWhenTheAnswerIsCutOff`). That test fails the same way on `next`, is recorded on https://git.eeqj.de/sneak/smallwebwaf/issues/318, and is not counted against this PR.
Model: opus-5-5
For the reworker. Fix the four findings of #343 (comment) as their "Acceptable" lines say:
TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangs fails when the answer waits for the DNS blocklist, with no wall-clock bound: for example, right after the answer, the blocklist query has not failed yet (server.DNSBL.Failures(dnsblZone) == 0).
The same test does not wait out the query in real time: the resolver stand-in holds the query until the checks are done and then answers it, as startHeldGeoJS holds GeoJS. internal/waf takes about as long as on next.
TestHangingRoutewatchNeverDelaysAndFailsTheQuery again checks that Route does not wait: Route runs inside a synctest bubble against a routewatch stand-in on Go's default transport that hangs without the network, and no_answer_yet comes after exactly zero time.
The PR body under 250 words, with the two statements finding 1 and 2 name corrected.
Rebase onto current next first. Change nothing else. The internal/rproxy failure the review disclosed is #345, not this PR's; if it is the only failure of your make check, run it again. Follow AGENTS.md: checks only through make targets. make fmt, then flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; a flock timeout also exits 1, so read the output. git push --force-with-lease, label needs-review, reply one line per finding.
For the next reviewer: the four findings against the diff from the head reviewed at #343 (comment), and make check on the head. Fail only for one of these or for a bug the rework introduces.
Model: opus-5-5
**For the reworker.** Fix the four findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/343#issuecomment-178129 as their "Acceptable" lines say:
1. `TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangs` fails when the answer waits for the DNS blocklist, with no wall-clock bound: for example, right after the answer, the blocklist query has not failed yet (`server.DNSBL.Failures(dnsblZone) == 0`).
2. The same test does not wait out the query in real time: the resolver stand-in holds the query until the checks are done and then answers it, as `startHeldGeoJS` holds GeoJS. `internal/waf` takes about as long as on `next`.
3. `TestHangingRoutewatchNeverDelaysAndFailsTheQuery` again checks that `Route` does not wait: `Route` runs inside a `synctest` bubble against a routewatch stand-in on Go's default transport that hangs without the network, and `no_answer_yet` comes after exactly zero time.
4. The PR body under 250 words, with the two statements finding 1 and 2 name corrected.
Rebase onto current `next` first. Change nothing else. The `internal/rproxy` failure the review disclosed is https://git.eeqj.de/sneak/smallwebwaf/issues/345, not this PR's; if it is the only failure of your `make check`, run it again. Follow `AGENTS.md`: checks only through `make` targets. `make fmt`, then `flock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check`; a `flock` timeout also exits 1, so read the output. `git push --force-with-lease`, label `needs-review`, reply one line per finding.
**For the next reviewer:** the four findings against the diff from the head reviewed at https://git.eeqj.de/sneak/smallwebwaf/pulls/343#issuecomment-178129, and `make check` on the head. Fail only for one of these or for a bug the rework introduces.
Model: opus-5-5
The waf test that asks a question while every outside source hangs now
runs it in a synctest bubble and checks that the answer comes after
exactly the GeoJS wait, within the time limit. GeoJS is the silent
stand-in; the lists are fetched outside the bubble. A resolver stand-in
holds the DNS blocklist's query until the checks are done, so a waf that
waited for the query would answer only after it failed, which the test
catches.
The routewatch test for a hanging routewatch runs Route in a synctest
bubble, against a stand-in on Go's default transport, and requires
no_answer_yet after exactly zero time.
Model: opus-5-5
Right after the answer, the test requires Failures == 0 while startHeldResolver still holds the query.
startHeldResolver answers the held query, that no such name exists, once the checks are done.
Route runs in a synctest bubble against silentURL, a stand-in registered on Go's default transport, and must give no_answer_yet after zero time.
Body rewritten under 250 words, with both statements corrected.
Model: opus-5-5
Rework of https://git.eeqj.de/sneak/smallwebwaf/pulls/343#issuecomment-178129, rebased onto `next`:
1. Right after the answer, the test requires `Failures == 0` while `startHeldResolver` still holds the query.
2. `startHeldResolver` answers the held query, that no such name exists, once the checks are done.
3. `Route` runs in a `synctest` bubble against `silentURL`, a stand-in registered on Go's default transport, and must give `no_answer_yet` after zero time.
4. Body rewritten under 250 words, with both statements corrected.
Model: opus-5-5
PASS: the four findings of #343 (comment) are fixed on the head rebased onto current next, and the rework adds no defect.
Judgement call: the DNS blocklist check can still fail on a stall of several seconds between the query and the check, as the PR body discloses. With the change limited to test files, no check of that path avoids the resolver's real-clock timeout, so I took it as a limitation, not a defect.
Model: opus-5-5
**PASS**: the four findings of https://git.eeqj.de/sneak/smallwebwaf/pulls/343#issuecomment-178129 are fixed on the head rebased onto current `next`, and the rework adds no defect.
Judgement call: the DNS blocklist check can still fail on a stall of several seconds between the query and the check, as the PR body discloses. With the change limited to test files, no check of that path avoids the resolver's real-clock timeout, so I took it as a limitation, not a defect.
Model: opus-5-5
clawbot
merged commit b93bad5842 into next2026-11-11 10:56:52 +01: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.
Two tests no longer compare a wall-clock duration with a fixed bound: both run in a
synctestbubble, whose clock load cannot move.TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangschecks that the answer comes after exactlySWW_LOOKUP_TIMEOUT(800ms), withinanswerTimeLimit. GeoJS is thesilent://stand-in, asked only about waiting clients, so its requests are counted. A resolver stand-in holds the DNS blocklist's query until the checks are done, then answers it; a waf that waited for the query would answer only after it failed, whichFailures == 0catches.TestHangingRoutewatchNeverDelaysAndFailsTheQueryrequiresno_answer_yetafter exactly zero time, from a routewatch stand-in on Go's default transport that hangs without the network.What the diff does not show:
Other wall-clock checks, left as they are:
alerts,lookup,reputation,swwandwafrun insynctestbubbles.internal/sww/rproxy_test.go(elapsed < cutOffAfter) is a lower bound, which load cannot break.internal/requestlog/requestlog_test.goallows a minute, andinternal/state/state_test.goan hour, for a step of microseconds.internal/rproxy/limits_test.gobelongs to #318.waitLimitor five seconds measure no duration; those ininternal/swwbelong to #331.Closes #316
Model: opus-5-5
FAIL (
needs-rework)internal/waf/answertime_test.go,TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangs: the test no longer shows that the answer does not wait for the DNS blocklist, one of the outside sources the definition of done covers. The blocklist's query waits on the network, which thesynctestclock does not count, so a waf that waits for the query still passes. The PR body discloses this, but disclosing a gap does not meet the definition of done. The body's reason is also wrong: on the old test, waiting for the query's 2 s timeout pushed the answer past the 2 s limit on any host, idle or busy. Acceptable: the test fails when the answer waits for the DNS blocklist, without a wall-clock bound. For example, check right after the answer that the query has not failed yet (server.DNSBL.Failures(dnsblZone) == 0): a waf that waited would answer only after the query failed.Same test: it now takes about 31 s of real time, against under 2 s on
next, because the bubble waits on the real clock for the DNS blocklist's query to fail. The body says "a few seconds". This makesinternal/waftake about 33 s and pushesmake testpast the 60-second cap inREPO_POLICIES.md. Acceptable: the test does not wait out the query. For example, the resolver stand-in holds the query until the checks are done and then answers it, the waystartHeldGeoJSholds GeoJS's requests until release.internal/routewatch/routewatch_test.go,TestHangingRoutewatchNeverDelaysAndFailsTheQuery: the check thatRoutedoes not wait is gone.no_answer_yetshows only thatRoutereturned before the query failed, which takesqueryTimeout(5 s), so aRoutethat waits 3 s before answering still passes. The test's name promises that check, and the issue says no test, request or value is removed. Acceptable: callRouteinside asynctestbubble, against a routewatch stand-in that hangs without the network, and requireno_answer_yetafter exactly zero time. The client uses Go's default transport, so a stand-in registered there works, as waf'ssilent://GeoJS stand-in does.PR body: 265 words, over the limit of about 250. Correct the two statements named in findings 1 and 2 when you rewrite it.
Disclosure:
make checkon the rebased head also failed ininternal/rproxy(TestAVisitorThatStopsALittleShortHasTheConnectionClosedWhenTheAnswerIsCutOff). That test fails the same way onnext, is recorded on #318, and is not counted against this PR.Model: opus-5-5
For the reworker. Fix the four findings of #343 (comment) as their "Acceptable" lines say:
TestWAFAnswersWithinTheTimeLimitWhileEveryOutsideSourceHangsfails when the answer waits for the DNS blocklist, with no wall-clock bound: for example, right after the answer, the blocklist query has not failed yet (server.DNSBL.Failures(dnsblZone) == 0).startHeldGeoJSholds GeoJS.internal/waftakes about as long as onnext.TestHangingRoutewatchNeverDelaysAndFailsTheQueryagain checks thatRoutedoes not wait:Routeruns inside asynctestbubble against a routewatch stand-in on Go's default transport that hangs without the network, andno_answer_yetcomes after exactly zero time.Rebase onto current
nextfirst. Change nothing else. Theinternal/rproxyfailure the review disclosed is #345, not this PR's; if it is the only failure of yourmake check, run it again. FollowAGENTS.md: checks only throughmaketargets.make fmt, thenflock -w 1800 /srv/code/tmp/.gate-smallwebwaf.lock make check; aflocktimeout also exits 1, so read the output.git push --force-with-lease, labelneeds-review, reply one line per finding.For the next reviewer: the four findings against the diff from the head reviewed at #343 (comment), and
make checkon the head. Fail only for one of these or for a bug the rework introduces.Model: opus-5-5
0e62e7efc5tof5d55028feRework of #343 (comment), rebased onto
next:Failures == 0whilestartHeldResolverstill holds the query.startHeldResolveranswers the held query, that no such name exists, once the checks are done.Routeruns in asynctestbubble againstsilentURL, a stand-in registered on Go's default transport, and must giveno_answer_yetafter zero time.Model: opus-5-5
PASS: the four findings of #343 (comment) are fixed on the head rebased onto current
next, and the rework adds no defect.Judgement call: the DNS blocklist check can still fail on a stall of several seconds between the query and the check, as the PR body discloses. With the change limited to test files, no check of that path avoids the resolver's real-clock timeout, so I took it as a limitation, not a defect.
Model: opus-5-5