From d24a8a42bcfd18557da235ec33e35f80fd6f47fd Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 4 Jun 2026 13:30:48 +0000 Subject: [PATCH] fix(internal/syncwriter): write diagnostics in a single os.Stderr write The error handler used the builtin println, which writes to fd 2 unsynchronized with the test stream and emits the message and its trailing newline as two separate syscalls. Under concurrency a test framework's "--- PASS:"/"--- FAIL:" line can land between those two writes, so test2json never records that test's terminal action and tools like gotestsum report an innocent, passing test as unknown or failed. Format the message and its newline into one fmt.Fprintf to os.Stderr so the write can't be split, which keeps the result marker at the start of a line. Fixes #225. --- internal/syncwriter/syncwriter.go | 4 +++- internal/syncwriter/syncwriter_test.go | 29 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/internal/syncwriter/syncwriter.go b/internal/syncwriter/syncwriter.go index acf886c..03e0b12 100644 --- a/internal/syncwriter/syncwriter.go +++ b/internal/syncwriter/syncwriter.go @@ -24,7 +24,9 @@ func New(w io.Writer) *Writer { w: w, errorf: func(f string, v ...interface{}) { - println(fmt.Sprintf(f, v...)) + // Avoid the builtin println, which writes to fd 2 + // unsynchronized and can corrupt concurrent go test output. + fmt.Fprintf(os.Stderr, f+"\n", v...) }, } } diff --git a/internal/syncwriter/syncwriter_test.go b/internal/syncwriter/syncwriter_test.go index 9516d32..9e010ed 100644 --- a/internal/syncwriter/syncwriter_test.go +++ b/internal/syncwriter/syncwriter_test.go @@ -3,6 +3,7 @@ package syncwriter import ( "io" "os" + "strings" "testing" "cdr.dev/slog/v3/internal/assert" @@ -74,6 +75,34 @@ func TestWriter_Sync(t *testing.T) { }) } +// The default handler must report through the os.Stderr variable, not the +// builtin println that bypasses it. Reverting to println fails this test. +// +// Not parallel: it swaps the process-wide os.Stderr. +func TestWriter_defaultErrorReportsThroughStderr(t *testing.T) { + r, w, err := os.Pipe() + assert.Success(t, "pipe", err) + + tw := New(syncWriter{ + wf: func([]byte) (int, error) { return 0, io.EOF }, + sf: func() error { return nil }, + }) + + orig := os.Stderr + os.Stderr = w + tw.Write("sinkname", []byte("entry")) + os.Stderr = orig + assert.Success(t, "close", w.Close()) + + out, err := io.ReadAll(r) + assert.Success(t, "read", err) + + got := string(out) + assert.True(t, "reported via stderr", strings.Contains(got, "sinkname: failed to write entry")) + assert.True(t, "single trailing newline", strings.HasSuffix(got, "\n")) + assert.Equal(t, "newline count", 1, strings.Count(got, "\n")) +} + func Test_errorsIsAny(t *testing.T) { t.Parallel()