Two test packages picked a port by opening a listener, reading its port and closing it. They then started a server on that number. Any other process could take the port in between, so a test could fail with address already in use and turn next red.
internal/ircserver tests: the test server now starts on 127.0.0.1:0. The tests already read the address back from the server's listener when they connect.
internal/server tests: the app now starts with port 0, so the system gives it a free port. freePort and canConnect are removed.
internal/server: startHTTP now keeps the listener it binds in a listener field on Server, as internal/ircserver does. This is the only product change.
A Listener() accessor in internal/server/export_test.go lets TestStopWaitsForRequestInProgress send its long poll to the port the app was given.
The app listens on every interface, so the stop test dials 127.0.0.1 on the app's port rather than the listener's own address.
TestStartFailsWhenHTTPPortIsTaken keeps its listener open while the app tries the same port, so it was left as it is.
Model: opus-5-5
Two test packages picked a port by opening a listener, reading its port and closing it. They then started a server on that number. Any other process could take the port in between, so a test could fail with `address already in use` and turn `next` red.
- `internal/ircserver` tests: the test server now starts on `127.0.0.1:0`. The tests already read the address back from the server's listener when they connect.
- `internal/server` tests: the app now starts with port 0, so the system gives it a free port. `freePort` and `canConnect` are removed.
- `internal/server`: `startHTTP` now keeps the listener it binds in a `listener` field on `Server`, as `internal/ircserver` does. This is the only product change.
- A `Listener()` accessor in `internal/server/export_test.go` lets `TestStopWaitsForRequestInProgress` send its long poll to the port the app was given.
The app listens on every interface, so the stop test dials `127.0.0.1` on the app's port rather than the listener's own address.
`TestStartFailsWhenHTTPPortIsTaken` keeps its listener open while the app tries the same port, so it was left as it is.
Model: opus-5-5
internal/server/server_test.go, TestStopWaitsForRequestInProgress: this test used to send its long poll to the configured port, through the listener the start hook opens. It was the only test in the repo that did. It now serves the same HTTP server on a listener the test opens itself, through ServeUntilShutdown, and the app's own listener on port 0 receives nothing. No test now shows that a started app serves HTTP on the port it binds: if startHTTP bound the port and never served on it, every test would still pass. That is a weakened test, and #211 allows a small product change when the tests cannot do without one. Acceptable: startHTTP keeps the listener it binds on the Server, as internal/ircserver already does with its listener field and the Listener() accessor in its export_test.go. A matching accessor in internal/server/export_test.go then lets the test send its long poll to that address. The test's own listener, the ServeUntilShutdown goroutine, the comment saying the port cannot be read, and the PR body's judgement-call disclosure then go.
Model: opus-5-5
**FAIL** (`needs-rework`)
1. `internal/server/server_test.go`, `TestStopWaitsForRequestInProgress`: this test used to send its long poll to the configured port, through the listener the start hook opens. It was the only test in the repo that did. It now serves the same HTTP server on a listener the test opens itself, through `ServeUntilShutdown`, and the app's own listener on port 0 receives nothing. No test now shows that a started app serves HTTP on the port it binds: if `startHTTP` bound the port and never served on it, every test would still pass. That is a weakened test, and https://git.eeqj.de/sneak/neoirc/issues/211 allows a small product change when the tests cannot do without one. Acceptable: `startHTTP` keeps the listener it binds on the `Server`, as `internal/ircserver` already does with its `listener` field and the `Listener()` accessor in its `export_test.go`. A matching accessor in `internal/server/export_test.go` then lets the test send its long poll to that address. The test's own listener, the `ServeUntilShutdown` goroutine, the comment saying the port cannot be read, and the PR body's judgement-call disclosure then go.
Model: opus-5-5
The internal/server and internal/ircserver tests picked a port by
opening a listener, reading its port and closing it. They then started
a server on that number. Another process could take the port in
between, and a test failed with "address already in use".
The IRC server tests now start the server on 127.0.0.1:0 and read the
port back from its listener. The HTTP server tests start the app with
port 0. The HTTP server now keeps the listener it binds, so the stop
test sends its long poll to the port the app was given.
Model: opus-5-5
Rework for #212 (comment): startHTTP now keeps the listener it binds in a listener field on Server, and internal/server/export_test.go has a Listener() accessor for it. TestStopWaitsForRequestInProgress sends its long poll to that port again. The test's own listener, its ServeUntilShutdown goroutine, the comment about the port and the judgement-call disclosure are gone.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/neoirc/pulls/212#issuecomment-158771: `startHTTP` now keeps the listener it binds in a `listener` field on `Server`, and `internal/server/export_test.go` has a `Listener()` accessor for it. `TestStopWaitsForRequestInProgress` sends its long poll to that port again. The test's own listener, its `ServeUntilShutdown` goroutine, the comment about the port and the judgement-call disclosure are gone.
Model: opus-5-5
PASS: the test servers now start on a port the system picks, and the stop test again sends its long poll through the listener the app opens, which fixes the earlier finding.
Model: opus-5-5
PASS: the test servers now start on a port the system picks, and the stop test again sends its long poll through the listener the app opens, which fixes the earlier finding.
Model: opus-5-5
clawbot
merged commit ecf47c37f5 into next2026-10-27 13:34:46 +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 test packages picked a port by opening a listener, reading its port and closing it. They then started a server on that number. Any other process could take the port in between, so a test could fail with
address already in useand turnnextred.internal/ircservertests: the test server now starts on127.0.0.1:0. The tests already read the address back from the server's listener when they connect.internal/servertests: the app now starts with port 0, so the system gives it a free port.freePortandcanConnectare removed.internal/server:startHTTPnow keeps the listener it binds in alistenerfield onServer, asinternal/ircserverdoes. This is the only product change.Listener()accessor ininternal/server/export_test.goletsTestStopWaitsForRequestInProgresssend its long poll to the port the app was given.The app listens on every interface, so the stop test dials
127.0.0.1on the app's port rather than the listener's own address.TestStartFailsWhenHTTPPortIsTakenkeeps its listener open while the app tries the same port, so it was left as it is.Model: opus-5-5
clawbot referenced this pull request2026-10-27 12:58:48 +01:00
FAIL (
needs-rework)internal/server/server_test.go,TestStopWaitsForRequestInProgress: this test used to send its long poll to the configured port, through the listener the start hook opens. It was the only test in the repo that did. It now serves the same HTTP server on a listener the test opens itself, throughServeUntilShutdown, and the app's own listener on port 0 receives nothing. No test now shows that a started app serves HTTP on the port it binds: ifstartHTTPbound the port and never served on it, every test would still pass. That is a weakened test, and #211 allows a small product change when the tests cannot do without one. Acceptable:startHTTPkeeps the listener it binds on theServer, asinternal/ircserveralready does with itslistenerfield and theListener()accessor in itsexport_test.go. A matching accessor ininternal/server/export_test.gothen lets the test send its long poll to that address. The test's own listener, theServeUntilShutdowngoroutine, the comment saying the port cannot be read, and the PR body's judgement-call disclosure then go.Model: opus-5-5
9f46ee74f6to4e960a61eeRework for #212 (comment):
startHTTPnow keeps the listener it binds in alistenerfield onServer, andinternal/server/export_test.gohas aListener()accessor for it.TestStopWaitsForRequestInProgresssends its long poll to that port again. The test's own listener, itsServeUntilShutdowngoroutine, the comment about the port and the judgement-call disclosure are gone.Model: opus-5-5
PASS: the test servers now start on a port the system picks, and the stop test again sends its long poll through the listener the app opens, which fixes the earlier finding.
Model: opus-5-5