From 841004fa37d0b4393f9cefadd3e2e77e5b1f726b Mon Sep 17 00:00:00 2001 From: jdtw Date: Sat, 1 Aug 2026 18:55:28 -0700 Subject: [PATCH] Fix a race in TestClient The test picked a free port by binding a listener and closing it, started ListenAndServe in a goroutine, and then immediately dialed without waiting for the server to come up. Nothing ordered the bind before the first request, so on a loaded runner the client could win and the test failed with "connection refused". Closing the listener before rebinding also left a window for another process to take the port. httptest.NewServer binds before returning and hands back the address it actually bound, which removes both races and the localhost IPv4/IPv6 mismatch in the failure. pkg/frontend already tests this way. This was pre-existing flakiness rather than a regression; it happened to surface on the merge of an unrelated change. Co-Authored-By: Claude Opus 5 --- pkg/client/client_test.go | 45 ++++++++++----------------------------- 1 file changed, 11 insertions(+), 34 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index db6620c..82d8bb8 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -1,51 +1,28 @@ package client import ( - "context" "errors" - "fmt" - "net" - "net/http" - "sync" + "net/http/httptest" "testing" "jdtw.dev/links/pkg/links" "jdtw.dev/links/pkg/tokentest" ) -func getFreePort(t *testing.T) int { - t.Helper() - addr, err := net.ResolveTCPAddr("tcp", "localhost:0") - if err != nil { - t.Fatalf("net.ResolveTCPAddr failed: %v", err) - } - - l, err := net.ListenTCP("tcp", addr) - if err != nil { - t.Fatalf("net.ListenTCP failed: %v", err) - } - defer l.Close() - return l.Addr().(*net.TCPAddr).Port -} - func TestClient(t *testing.T) { ks, signer := tokentest.GenerateKey(t, "test") store := links.NewMemStore() - var wg sync.WaitGroup - wg.Add(1) - addr := fmt.Sprintf("localhost:%d", getFreePort(t)) - s := &http.Server{Addr: addr, Handler: links.NewHandler(store, ks, 0)} - go func() { - defer wg.Done() - s.ListenAndServe() - }() - ctx := context.Background() - t.Cleanup(func() { - s.Shutdown(ctx) - wg.Wait() - }) - c := New("http://"+addr, signer) + // httptest.NewServer binds before it returns, so the client below cannot + // race the listener. The previous version picked a port, closed it, then + // started the server in a goroutine and immediately dialed -- which meant + // the request could arrive before the server was listening (and left a + // window for another process to take the port). That raced on loaded CI + // runners. + s := httptest.NewServer(links.NewHandler(store, ks, 0)) + t.Cleanup(s.Close) + + c := New(s.URL, signer) if _, err := c.Get("foo"); !errors.Is(err, ErrNotFound) { t.Errorf("Get(foo) returned %v; want err %v", err, ErrNotFound) }