Skip to content

Commit 08c064d

Browse files
authored
fix(internal/syncwriter): write diagnostics in a single os.Stderr write (#226)
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.
1 parent 284c50f commit 08c064d

2 files changed

Lines changed: 32 additions & 1 deletion

File tree

internal/syncwriter/syncwriter.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ func New(w io.Writer) *Writer {
2424
w: w,
2525

2626
errorf: func(f string, v ...interface{}) {
27-
println(fmt.Sprintf(f, v...))
27+
// Avoid the builtin println, which writes to fd 2
28+
// unsynchronized and can corrupt concurrent go test output.
29+
fmt.Fprintf(os.Stderr, f+"\n", v...)
2830
},
2931
}
3032
}

internal/syncwriter/syncwriter_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package syncwriter
33
import (
44
"io"
55
"os"
6+
"strings"
67
"testing"
78

89
"cdr.dev/slog/v3/internal/assert"
@@ -74,6 +75,34 @@ func TestWriter_Sync(t *testing.T) {
7475
})
7576
}
7677

78+
// The default handler must report through the os.Stderr variable, not the
79+
// builtin println that bypasses it. Reverting to println fails this test.
80+
//
81+
// Not parallel: it swaps the process-wide os.Stderr.
82+
func TestWriter_defaultErrorReportsThroughStderr(t *testing.T) {
83+
r, w, err := os.Pipe()
84+
assert.Success(t, "pipe", err)
85+
86+
tw := New(syncWriter{
87+
wf: func([]byte) (int, error) { return 0, io.EOF },
88+
sf: func() error { return nil },
89+
})
90+
91+
orig := os.Stderr
92+
os.Stderr = w
93+
tw.Write("sinkname", []byte("entry"))
94+
os.Stderr = orig
95+
assert.Success(t, "close", w.Close())
96+
97+
out, err := io.ReadAll(r)
98+
assert.Success(t, "read", err)
99+
100+
got := string(out)
101+
assert.True(t, "reported via stderr", strings.Contains(got, "sinkname: failed to write entry"))
102+
assert.True(t, "single trailing newline", strings.HasSuffix(got, "\n"))
103+
assert.Equal(t, "newline count", 1, strings.Count(got, "\n"))
104+
}
105+
77106
func Test_errorsIsAny(t *testing.T) {
78107
t.Parallel()
79108

0 commit comments

Comments
 (0)