The timeout tests asserted on newHTTPServer alone, so a Run that built its http.Server inline would drop every timeout with the suite still green.
TestRunWiresSocketTimeouts wires a *server.Server through fx as cmd/dnswatcher does, minus the watcher and resolver so no live DNS is touched. Given an unbindable port (-1), Run stores its http.Server, ListenAndServe fails at once, and Run returns without listening. The test checks that stored server for all four timeouts and both relationships (WriteTimeout > handler budget, ReadTimeout >= ReadHeaderTimeout). It replaces the four constructor-only tests; the addr/handler test, which could not fail, is dropped and its handler check folded in.
The ReadTimeout note in the test now matches the net/http source of go1.25.7, the Dockerfile toolchain: a request whose headers arrive after ReadTimeout but within ReadHeaderTimeout gets a read deadline that has already passed, so reading its body fails at once.
No timeout value changed; nothing measures elapsed time.
Mutation proof
With Run building &http.Server{Addr: listenAddr, Handler: s}, the test fails with ReadTimeout must be non-zero, got 0s, the same for the other three timeouts, and the handler-budget error.
Disclosures
Judgement call: only the test comment carried the wrong wording. The body of #118 and the plan comment on #99 say "unreachable" only about WriteTimeout, which is correct, so they are unchanged.
Judgement call: the test cannot use t.Parallel; it sets an env var and resets viper's global state.
Closes https://git.eeqj.de/sneak/dnswatcher/issues/120.
## What changed
The timeout tests asserted on `newHTTPServer` alone, so a `Run` that built its `http.Server` inline would drop every timeout with the suite still green.
`TestRunWiresSocketTimeouts` wires a `*server.Server` through `fx` as `cmd/dnswatcher` does, minus the watcher and resolver so no live DNS is touched. Given an unbindable port (`-1`), `Run` stores its `http.Server`, `ListenAndServe` fails at once, and `Run` returns without listening. The test checks that stored server for all four timeouts and both relationships (`WriteTimeout` > handler budget, `ReadTimeout` >= `ReadHeaderTimeout`). It replaces the four constructor-only tests; the addr/handler test, which could not fail, is dropped and its handler check folded in.
The `ReadTimeout` note in the test now matches the `net/http` source of go1.25.7, the `Dockerfile` toolchain: a request whose headers arrive after `ReadTimeout` but within `ReadHeaderTimeout` gets a read deadline that has already passed, so reading its body fails at once.
No timeout value changed; nothing measures elapsed time.
## Mutation proof
With `Run` building `&http.Server{Addr: listenAddr, Handler: s}`, the test fails with `ReadTimeout must be non-zero, got 0s`, the same for the other three timeouts, and the handler-budget error.
## Disclosures
- Judgement call: only the test comment carried the wrong wording. The body of https://git.eeqj.de/sneak/dnswatcher/pulls/118 and the plan comment on https://git.eeqj.de/sneak/dnswatcher/issues/99 say "unreachable" only about `WriteTimeout`, which is correct, so they are unchanged.
- Judgement call: the test cannot use `t.Parallel`; it sets an env var and resets viper's global state.
Model: opus-4-8 (implementation); opus-5-5 (rework)
internal/server/server_test.go lines 72–75: the corrected read-deadline note makes a new wrong claim. In the pinned toolchain (go1.25.7, from the Dockerfile golang image), readRequest sets the whole-request read deadline to ReadTimeout counted from the start of the request. That deadline has already passed only when the headers took longer than ReadTimeout to arrive. It is not already passed just because ReadTimeout is the smaller value. It also does not sever the request. For a request without a body (every route here), net/http clears the read deadline before it runs the handler, so only reading a request body fails. Acceptable: say that when a request's headers arrive after ReadTimeout but within ReadHeaderTimeout, it gets a read deadline that has already passed, so reading its body fails at once. Also drop t0, a name that exists only inside the net/http source.
internal/server/server_test.go lines 58–61 and 76–78: the test's doc comment carries history (what the earlier tests did, plus the issue link) and a note saying what it was verified against. That note also cites go.mod's go 1.25.5, which is a minimum version, not the pinned toolchain. Acceptable: keep only what the test guards and why Run is given an unbindable port, and drop both passages.
TODO.md lines 29 and 37: the change adds blank lines inside the Completed Steps list. One of them sits between two existing entries that this change is not about (the go mod tidy entry and the 2026-08-10 entry). Acceptable: add the new entry in the same style as its neighbours and leave the existing entries unchanged.
PR body: it runs to about 300 words, over the limit of about 250. It also refers to #118 and #99 by number only. Acceptable: trim it below about 250 words and write both as full links.
Model: opus-5-5
1. `internal/server/server_test.go` lines 72–75: the corrected read-deadline note makes a new wrong claim. In the pinned toolchain (go1.25.7, from the `Dockerfile` golang image), `readRequest` sets the whole-request read deadline to `ReadTimeout` counted from the start of the request. That deadline has already passed only when the headers took longer than `ReadTimeout` to arrive. It is not already passed just because `ReadTimeout` is the smaller value. It also does not sever the request. For a request without a body (every route here), `net/http` clears the read deadline before it runs the handler, so only reading a request body fails. Acceptable: say that when a request's headers arrive after `ReadTimeout` but within `ReadHeaderTimeout`, it gets a read deadline that has already passed, so reading its body fails at once. Also drop `t0`, a name that exists only inside the `net/http` source.
2. `internal/server/server_test.go` lines 58–61 and 76–78: the test's doc comment carries history (what the earlier tests did, plus the issue link) and a note saying what it was verified against. That note also cites `go.mod`'s `go 1.25.5`, which is a minimum version, not the pinned toolchain. Acceptable: keep only what the test guards and why `Run` is given an unbindable port, and drop both passages.
3. `TODO.md` lines 29 and 37: the change adds blank lines inside the Completed Steps list. One of them sits between two existing entries that this change is not about (the `go mod tidy` entry and the 2026-08-10 entry). Acceptable: add the new entry in the same style as its neighbours and leave the existing entries unchanged.
4. PR body: it runs to about 300 words, over the limit of about 250. It also refers to https://git.eeqj.de/sneak/dnswatcher/pulls/118 and https://git.eeqj.de/sneak/dnswatcher/issues/99 by number only. Acceptable: trim it below about 250 words and write both as full links.
Model: opus-5-5
The timeout tests called newHTTPServer directly, so a Run that built
its http.Server inline would drop every timeout with the suite still
green. TestRunWiresSocketTimeouts wires a Server as cmd/dnswatcher
does, minus the watcher and resolver so no live DNS is touched, drives
Run with an unbindable port so it stores its http.Server and returns
without listening, and checks that server carries all four timeouts
and both required relationships. The addr/handler test, which could
not fail, is dropped.
The ReadTimeout note now says what net/http does: a request whose
headers arrive after ReadTimeout but within ReadHeaderTimeout gets a
read deadline that has already passed, so reading its body fails at
once.
Model: opus-4-8 (implementation); opus-5-5 (rework)
Findings 1 and 2: the test's doc comment now says only what the test guards, why Run gets an unbindable port, and the ReadTimeout behaviour the go1.25.7 net/http source shows.
Findings 3 and 4: the TODO.md entry matches its neighbours with no added blank lines; the PR body is trimmed and uses full links.
Model: opus-5-5
Findings 1 and 2: the test's doc comment now says only what the test guards, why `Run` gets an unbindable port, and the `ReadTimeout` behaviour the go1.25.7 `net/http` source shows.
Findings 3 and 4: the `TODO.md` entry matches its neighbours with no added blank lines; the PR body is trimmed and uses full links.
Model: opus-5-5
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.
Closes #120.
What changed
The timeout tests asserted on
newHTTPServeralone, so aRunthat built itshttp.Serverinline would drop every timeout with the suite still green.TestRunWiresSocketTimeoutswires a*server.Serverthroughfxascmd/dnswatcherdoes, minus the watcher and resolver so no live DNS is touched. Given an unbindable port (-1),Runstores itshttp.Server,ListenAndServefails at once, andRunreturns without listening. The test checks that stored server for all four timeouts and both relationships (WriteTimeout> handler budget,ReadTimeout>=ReadHeaderTimeout). It replaces the four constructor-only tests; the addr/handler test, which could not fail, is dropped and its handler check folded in.The
ReadTimeoutnote in the test now matches thenet/httpsource of go1.25.7, theDockerfiletoolchain: a request whose headers arrive afterReadTimeoutbut withinReadHeaderTimeoutgets a read deadline that has already passed, so reading its body fails at once.No timeout value changed; nothing measures elapsed time.
Mutation proof
With
Runbuilding&http.Server{Addr: listenAddr, Handler: s}, the test fails withReadTimeout must be non-zero, got 0s, the same for the other three timeouts, and the handler-budget error.Disclosures
WriteTimeout, which is correct, so they are unchanged.t.Parallel; it sets an env var and resets viper's global state.Model: opus-4-8 (implementation); opus-5-5 (rework)
71b8d46f7btod07311df99internal/server/server_test.golines 72–75: the corrected read-deadline note makes a new wrong claim. In the pinned toolchain (go1.25.7, from theDockerfilegolang image),readRequestsets the whole-request read deadline toReadTimeoutcounted from the start of the request. That deadline has already passed only when the headers took longer thanReadTimeoutto arrive. It is not already passed just becauseReadTimeoutis the smaller value. It also does not sever the request. For a request without a body (every route here),net/httpclears the read deadline before it runs the handler, so only reading a request body fails. Acceptable: say that when a request's headers arrive afterReadTimeoutbut withinReadHeaderTimeout, it gets a read deadline that has already passed, so reading its body fails at once. Also dropt0, a name that exists only inside thenet/httpsource.internal/server/server_test.golines 58–61 and 76–78: the test's doc comment carries history (what the earlier tests did, plus the issue link) and a note saying what it was verified against. That note also citesgo.mod'sgo 1.25.5, which is a minimum version, not the pinned toolchain. Acceptable: keep only what the test guards and whyRunis given an unbindable port, and drop both passages.TODO.mdlines 29 and 37: the change adds blank lines inside the Completed Steps list. One of them sits between two existing entries that this change is not about (thego mod tidyentry and the 2026-08-10 entry). Acceptable: add the new entry in the same style as its neighbours and leave the existing entries unchanged.PR body: it runs to about 300 words, over the limit of about 250. It also refers to #118 and #99 by number only. Acceptable: trim it below about 250 words and write both as full links.
Model: opus-5-5
d07311df99toa3cfec09dfFindings 1 and 2: the test's doc comment now says only what the test guards, why
Rungets an unbindable port, and theReadTimeoutbehaviour the go1.25.7net/httpsource shows.Findings 3 and 4: the
TODO.mdentry matches its neighbours with no added blank lines; the PR body is trimmed and uses full links.Model: opus-5-5
Review passed on
a3cfec0.Model: opus-5-5