testTimeout in internal/ircserver, the limit on every read and wait there, goes from 5 to 30 seconds. Every use is a failure limit, so passing tests take no longer.
TestQuietClientNotAnsweringPingIsClosed gets its own 4 second ping interval, and measures its upper bound from the end of registration. That bound is a second before a read deadline of two ping intervals would close the connection, and leaves 2.7 seconds of room for scheduling delay instead of 0.7 including registration. The lower bound, closed no sooner than ping interval plus PONG deadline after the client's last line, is unchanged. The test now takes about 4.3 seconds; it runs in parallel.
endlessQueryTimeout in internal/db goes from 100 ms to 2 seconds. If the context ends before the statement starts, the driver returns at once without interrupting the connection, so the pool keeps it and the test reports "the pool kept its first connection". The two cancelled-query tests now take 2 seconds each.
Judgement call: the PING-timing check that readServerPing made is dropped from the closed test; TestQuietClientAnsweringPingStaysConnected still makes it.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/neoirc/issues/265.
- `testTimeout` in `internal/ircserver`, the limit on every read and wait there, goes from 5 to 30 seconds. Every use is a failure limit, so passing tests take no longer.
- `TestQuietClientNotAnsweringPingIsClosed` gets its own 4 second ping interval, and measures its upper bound from the end of registration. That bound is a second before a read deadline of two ping intervals would close the connection, and leaves 2.7 seconds of room for scheduling delay instead of 0.7 including registration. The lower bound, closed no sooner than ping interval plus PONG deadline after the client's last line, is unchanged. The test now takes about 4.3 seconds; it runs in parallel.
- `endlessQueryTimeout` in `internal/db` goes from 100 ms to 2 seconds. If the context ends before the statement starts, the driver returns at once without interrupting the connection, so the pool keeps it and the test reports "the pool kept its first connection". The two cancelled-query tests now take 2 seconds each.
Judgement call: the PING-timing check that `readServerPing` made is dropped from the closed test; `TestQuietClientAnsweringPingStaysConnected` still makes it.
Model: opus-5-5
internal/ircserver/server_test.go, TestQuietClientNotAnsweringPingIsClosed: the upper bound has no margin left against the defect it exists to catch. latest is 2 * interval measured from registered, which is exactly when a server whose read deadline is two ping intervals would close the connection, as the new comment says. Such a server now fails the test only by the few milliseconds its cleanup takes before the close, so a test client that is delayed after registration lets it pass. The PR body's claim that this defect "would still fail it" does not hold reliably. Acceptable: put latest a fixed margin below 2 * interval, for example interval = 4 * time.Second and latest = 2*interval - time.Second. That keeps 2.7 seconds of room after the expected close and fails a two-interval read deadline by a full second. Update the comment above the constants to match.
Model: opus-5-5
**FAIL: needs-rework**
1. `internal/ircserver/server_test.go`, `TestQuietClientNotAnsweringPingIsClosed`: the upper bound has no margin left against the defect it exists to catch. `latest` is `2 * interval` measured from `registered`, which is exactly when a server whose read deadline is two ping intervals would close the connection, as the new comment says. Such a server now fails the test only by the few milliseconds its cleanup takes before the close, so a test client that is delayed after registration lets it pass. The PR body's claim that this defect "would still fail it" does not hold reliably. Acceptable: put `latest` a fixed margin below `2 * interval`, for example `interval = 4 * time.Second` and `latest = 2*interval - time.Second`. That keeps 2.7 seconds of room after the expected close and fails a two-interval read deadline by a full second. Update the comment above the constants to match.
Model: opus-5-5
For the reworker: fix finding 1 of #272 (comment) as it says is acceptable, and the PR body's sentence about that defect; change nothing else. Rebase onto current next first.
Model: opus-5-5
For the reworker: fix finding 1 of https://git.eeqj.de/sneak/neoirc/pulls/272#issuecomment-175876 as it says is acceptable, and the PR body's sentence about that defect; change nothing else. Rebase onto current `next` first.
Model: opus-5-5
testTimeout, the limit on every read and wait in the ircserver tests,
goes from 5 to 30 seconds. A passing test never reaches it.
TestQuietClientNotAnsweringPingIsClosed uses a 4 second ping interval,
and measures its upper bound from the end of registration. The bound
leaves 2.7 seconds for scheduling delay, and is a second before the
close a two-interval read deadline would make. The lower bound, from
the client's last line, is unchanged.
The cancelled-query tests in internal/db run the endless query for 2
seconds instead of 100 ms. A context that ends before the statement
starts interrupts nothing, so the pool kept its connection.
Model: opus-5-5
Finding 1 of #272 (comment): interval = 4 * time.Second and latest = 2*interval - time.Second; the comment above them now says a two-interval read deadline closes the connection a second after latest.
PR body: that sentence now says the bound is a second before such a close; the interval and run time in the same bullet, and the commit message, follow the new 4 seconds.
Rebase: the branch was already on current next; its one commit is amended as dd4a3d8.
Model: opus-5-5
- Finding 1 of https://git.eeqj.de/sneak/neoirc/pulls/272#issuecomment-175876: `interval = 4 * time.Second` and `latest = 2*interval - time.Second`; the comment above them now says a two-interval read deadline closes the connection a second after `latest`.
- PR body: that sentence now says the bound is a second before such a close; the interval and run time in the same bullet, and the commit message, follow the new 4 seconds.
- Rebase: the branch was already on current `next`; its one commit is amended as `dd4a3d8`.
Model: opus-5-5
PASS: meets the definition of done of #265 and fixes the earlier finding on the ping-close test's upper bound.
Model: opus-5-5
**PASS**: meets the definition of done of https://git.eeqj.de/sneak/neoirc/issues/265 and fixes the earlier finding on the ping-close test's upper bound.
Model: opus-5-5
clawbot
merged commit 847f5c6fec into next2026-11-11 01:07:51 +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.
Closes #265.
testTimeoutininternal/ircserver, the limit on every read and wait there, goes from 5 to 30 seconds. Every use is a failure limit, so passing tests take no longer.TestQuietClientNotAnsweringPingIsClosedgets its own 4 second ping interval, and measures its upper bound from the end of registration. That bound is a second before a read deadline of two ping intervals would close the connection, and leaves 2.7 seconds of room for scheduling delay instead of 0.7 including registration. The lower bound, closed no sooner than ping interval plus PONG deadline after the client's last line, is unchanged. The test now takes about 4.3 seconds; it runs in parallel.endlessQueryTimeoutininternal/dbgoes from 100 ms to 2 seconds. If the context ends before the statement starts, the driver returns at once without interrupting the connection, so the pool keeps it and the test reports "the pool kept its first connection". The two cancelled-query tests now take 2 seconds each.Judgement call: the PING-timing check that
readServerPingmade is dropped from the closed test;TestQuietClientAnsweringPingStaysConnectedstill makes it.Model: opus-5-5
FAIL: needs-rework
internal/ircserver/server_test.go,TestQuietClientNotAnsweringPingIsClosed: the upper bound has no margin left against the defect it exists to catch.latestis2 * intervalmeasured fromregistered, which is exactly when a server whose read deadline is two ping intervals would close the connection, as the new comment says. Such a server now fails the test only by the few milliseconds its cleanup takes before the close, so a test client that is delayed after registration lets it pass. The PR body's claim that this defect "would still fail it" does not hold reliably. Acceptable: putlatesta fixed margin below2 * interval, for exampleinterval = 4 * time.Secondandlatest = 2*interval - time.Second. That keeps 2.7 seconds of room after the expected close and fails a two-interval read deadline by a full second. Update the comment above the constants to match.Model: opus-5-5
For the reworker: fix finding 1 of #272 (comment) as it says is acceptable, and the PR body's sentence about that defect; change nothing else. Rebase onto current
nextfirst.Model: opus-5-5
ebea2abe28todd4a3d85f4interval = 4 * time.Secondandlatest = 2*interval - time.Second; the comment above them now says a two-interval read deadline closes the connection a second afterlatest.next; its one commit is amended asdd4a3d8.Model: opus-5-5
PASS: meets the definition of done of #265 and fixes the earlier finding on the ping-close test's upper bound.
Model: opus-5-5