From 4e960a61ee98752d914e83794c04a9f850e73b31 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 27 Oct 2026 11:34:36 +0000 Subject: [PATCH] Start test servers on a port the system picks (closes #211) 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 --- internal/ircserver/server_test.go | 18 ++-------- internal/server/export_test.go | 6 ++++ internal/server/server.go | 3 ++ internal/server/server_test.go | 56 +++++++------------------------ 4 files changed, 25 insertions(+), 58 deletions(-) diff --git a/internal/ircserver/server_test.go b/internal/ircserver/server_test.go index f30b826..1cc4316 100644 --- a/internal/ircserver/server_test.go +++ b/internal/ircserver/server_test.go @@ -137,20 +137,6 @@ func newTestEnvWithResolver( brk := broker.New() - var listenConfig net.ListenConfig - - listener, err := listenConfig.Listen(t.Context(), "tcp", "127.0.0.1:0") - if err != nil { - t.Fatalf("listen: %v", err) - } - - addr := listener.Addr().String() - - err = listener.Close() - if err != nil { - t.Fatalf("close listener: %v", err) - } - log := slog.New(slog.NewTextHandler( os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}, //nolint:exhaustruct @@ -160,7 +146,9 @@ func newTestEnvWithResolver( log, cfg, database, brk, interval, deadline, resolve, ) - err = srv.Start(addr) + // Port 0 makes the system pick a free port. dial reads it + // back from the server's listener. + err = srv.Start("127.0.0.1:0") if err != nil { t.Fatalf("start irc server: %v", err) } diff --git a/internal/server/export_test.go b/internal/server/export_test.go index 3908b62..ef72b69 100644 --- a/internal/server/export_test.go +++ b/internal/server/export_test.go @@ -15,6 +15,12 @@ func NewTestSPAServer() *Server { return srv } +// Listener returns the listener the start hook bound for the +// HTTP server. +func (srv *Server) Listener() net.Listener { + return srv.listener +} + // ServeUntilShutdown runs serveUntilShutdown, which serves the // HTTP server on listener, and returns when it does. func (srv *Server) ServeUntilShutdown(listener net.Listener) { diff --git a/internal/server/server.go b/internal/server/server.go index 0bd8951..c5715ab 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -50,6 +50,7 @@ type Server struct { sentryEnabled bool log *slog.Logger httpServer *http.Server + listener net.Listener router *chi.Mux params Params mw *middleware.Middleware @@ -179,6 +180,8 @@ func (srv *Server) startHTTP(ctx context.Context) error { return fmt.Errorf("http listen: %w", err) } + srv.listener = listener + srv.httpServer = &http.Server{ //nolint:exhaustruct // optional fields Addr: listenAddr, ReadTimeout: httpReadTimeout, diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 874231e..6a44326 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -45,10 +45,9 @@ const ( // for a long poll the HTTP server is serving to finish, and // that the poll gets its response. func TestStopWaitsForRequestInProgress(t *testing.T) { - port := freePort(t) - addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(port)) + var srv *server.Server - app, brk, database := startApp(t, port) + app, brk, database := startApp(t, fx.Populate(&srv)) sessionID, _, token, err := database.CreateSession( t.Context(), "poller", "poller", "", "", @@ -57,9 +56,12 @@ func TestStopWaitsForRequestInProgress(t *testing.T) { t.Fatalf("create session: %v", err) } - waitUntil(t, "the HTTP server to listen", func() bool { - return canConnect(t, addr) - }) + tcpAddr, ok := srv.Listener().Addr().(*net.TCPAddr) + if !ok { + t.Fatalf("listener address %v is not TCP", srv.Listener().Addr()) + } + + addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(tcpAddr.Port)) statuses := make(chan int, 1) @@ -145,7 +147,7 @@ func TestStartFailsWhenHTTPPortIsTaken(t *testing.T) { func TestServeFailureStopsApp(t *testing.T) { var srv *server.Server - app, _, _ := startApp(t, freePort(t), fx.Populate(&srv)) + app, _, _ := startApp(t, fx.Populate(&srv)) shutdowns := app.Wait() @@ -168,7 +170,7 @@ func TestServeFailureStopsApp(t *testing.T) { func TestServeAfterStopDoesNotStopApp(t *testing.T) { var srv *server.Server - app, _, _ := startApp(t, freePort(t), fx.Populate(&srv)) + app, _, _ := startApp(t, fx.Populate(&srv)) shutdowns := app.Wait() @@ -186,16 +188,15 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) { } } -// startApp starts the app newApp builds, with the HTTP server -// listening on port. +// startApp starts the app newApp builds, with the HTTP port +// set to 0, so the system gives the server a free port. func startApp( t *testing.T, - port int, options ...fx.Option, ) (*fxtest.App, *broker.Broker, *db.Database) { t.Helper() - app, brk, database := newApp(t, port, options...) + app, brk, database := newApp(t, 0, options...) app.RequireStart() @@ -254,21 +255,6 @@ func newApp( return app, brk, database } -// freePort returns a TCP port that nothing was listening on a -// moment ago. -func freePort(t *testing.T) int { - t.Helper() - - listener := closedListener(t) - - tcpAddr, ok := listener.Addr().(*net.TCPAddr) - if !ok { - t.Fatalf("listener address %v is not TCP", listener.Addr()) - } - - return tcpAddr.Port -} - // closedListener returns a TCP listener that has already been // closed, so that accepting a connection on it fails. func closedListener(t *testing.T) net.Listener { @@ -291,22 +277,6 @@ func closedListener(t *testing.T) net.Listener { return listener } -// canConnect reports whether addr accepts a TCP connection. -func canConnect(t *testing.T, addr string) bool { - t.Helper() - - var dialer net.Dialer - - conn, err := dialer.DialContext(t.Context(), "tcp", addr) - if err != nil { - return false - } - - _ = conn.Close() - - return true -} - // waitUntil calls done every checkStep until it returns true, // and fails the test if it does not within testTimeout. func waitUntil(t *testing.T, what string, done func() bool) { -- 2.54.0