From dbb486dd7f78aa3cfd55251679dc2615396bd4af Mon Sep 17 00:00:00 2001 From: Victor Solano Date: Thu, 6 Aug 2026 07:37:55 +0200 Subject: [PATCH] fix(test): stop the serve tests racing on the shared log buffer The serve tests capture log output by pointing the process-wide logger at a local buffer with log.SetOutput. Four tests do that, and three of them run with t.Parallel(), so they all run at once against whichever buffer was installed last: one test's request goroutines write through log.Printf while another test reads the same buffer with String(). The race detector catches it. make cov (go test -race) fails on master in 11 of 20 runs here, always with the same pair of stacks -- a bytes.Buffer grow from outputs.logTrace under TestServeHandlesConcurrentRequests against a bytes.Buffer String from TestServeNegotiatingContent or TestServeWithNoContentNegotiation. Guard the buffer with a mutex. The logger stays global and no test changes what it asserts; only the buffer becomes safe to touch from two goroutines. Same 12 runs of the same command are clean afterwards. Fixing the routing properly -- so a parallel test reads only its own log lines -- means giving each test its own logger, which is a larger change to how outputs and serve get their logger. This just stops CI failing. Signed-off-by: Victor Solano --- logbuffer_test.go | 36 ++++++++++++++++++++++++++++++++++++ serve_test.go | 9 ++++----- 2 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 logbuffer_test.go diff --git a/logbuffer_test.go b/logbuffer_test.go new file mode 100644 index 000000000..efd710311 --- /dev/null +++ b/logbuffer_test.go @@ -0,0 +1,36 @@ +package goss + +import ( + "bytes" + "sync" +) + +// syncBuffer is a bytes.Buffer that is safe to write and read concurrently. +// +// The serve tests capture log output by pointing the process-wide logger at a +// buffer with log.SetOutput. That destination is global, so a parallel test +// still writes into whichever buffer was installed last while its owner reads +// it — a data race on the buffer even though each test declares its own. +// Guarding the buffer removes the race without giving up the shared logger. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +func (b *syncBuffer) Reset() { + b.mu.Lock() + defer b.mu.Unlock() + b.buf.Reset() +} diff --git a/serve_test.go b/serve_test.go index c7f64a265..90042c03a 100644 --- a/serve_test.go +++ b/serve_test.go @@ -1,7 +1,6 @@ package goss import ( - "bytes" "log" "net/http" "net/http/httptest" @@ -44,7 +43,7 @@ func TestServeWithNoContentNegotiation(t *testing.T) { for testName := range tests { tc := tests[testName] t.Run(testName, func(t *testing.T) { - var logOutput bytes.Buffer + var logOutput syncBuffer log.SetOutput(&logOutput) config, err := util.NewConfig( @@ -158,7 +157,7 @@ func TestServeNegotiatingContent(t *testing.T) { for testName := range tests { tc := tests[testName] t.Run(testName, func(t *testing.T) { - var logOutput bytes.Buffer + var logOutput syncBuffer log.SetOutput(&logOutput) config, err := util.NewConfig( @@ -189,7 +188,7 @@ func TestServeNegotiatingContent(t *testing.T) { } func TestServeCacheWithNoContentNegotiation(t *testing.T) { - var logOutput bytes.Buffer + var logOutput syncBuffer log.SetOutput(&logOutput) const cache = time.Duration(time.Millisecond * 100) config, err := util.NewConfig( @@ -236,7 +235,7 @@ func TestServeCacheWithNoContentNegotiation(t *testing.T) { } func TestServeCacheNegotiatingContent(t *testing.T) { - var logOutput bytes.Buffer + var logOutput syncBuffer log.SetOutput(&logOutput) const cache = time.Duration(time.Millisecond * 100) config, err := util.NewConfig(