From fe4fd0094fc9caffcac9096403dfd44b7cad32ad Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Tue, 21 Apr 2026 16:38:47 -0500 Subject: [PATCH 1/2] Fix instability in diode_test on darwin --- diode/diode_test.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/diode/diode_test.go b/diode/diode_test.go index cbef60b2..ae2bf59a 100644 --- a/diode/diode_test.go +++ b/diode/diode_test.go @@ -16,15 +16,37 @@ import ( "github.com/rs/zerolog/internal/cbor" ) +type signalBuffer struct { + bytes.Buffer + wrote chan struct{} +} + +func (b *signalBuffer) Write(p []byte) (n int, err error) { + n, err = b.Buffer.Write(p) + if err == nil { + select { + case b.wrote <- struct{}{}: + default: + } + } + return n, err +} + func TestNewWriter(t *testing.T) { - buf := bytes.Buffer{} + buf := signalBuffer{wrote: make(chan struct{}, 1)} w := diode.NewWriter(&buf, 1000, 0, func(missed int) { fmt.Printf("Dropped %d messages\n", missed) }) log := zerolog.New(w) log.Print("test") - w.Close() + select { + case <-buf.wrote: + case <-time.After(time.Second): + t.Fatal("timed out waiting for diode writer to flush") + } + + _ = w.Close() want := "{\"level\":\"debug\",\"message\":\"test\"}\n" got := cbor.DecodeIfBinaryToString(buf.Bytes()) if got != want { From 2cfa1b875faf6f901fb10a1d762a0f071c9a6cf1 Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Tue, 21 Apr 2026 17:24:10 -0500 Subject: [PATCH 2/2] Addressed review comments Applied same pattern to the other diode_test methods because now they race. Simplified to remove the sub-process now that zerolog supports replaceable FatalExitFunc --- diode/diode_test.go | 152 ++++++++++++++++---------------------------- 1 file changed, 55 insertions(+), 97 deletions(-) diff --git a/diode/diode_test.go b/diode/diode_test.go index ae2bf59a..19234306 100644 --- a/diode/diode_test.go +++ b/diode/diode_test.go @@ -6,8 +6,6 @@ import ( "io" "log" "os" - "os/exec" - "sync" "testing" "time" @@ -16,16 +14,16 @@ import ( "github.com/rs/zerolog/internal/cbor" ) -type signalBuffer struct { - bytes.Buffer +type signalWriter struct { + w io.Writer wrote chan struct{} } -func (b *signalBuffer) Write(p []byte) (n int, err error) { - n, err = b.Buffer.Write(p) +func (w *signalWriter) Write(p []byte) (n int, err error) { + n, err = w.w.Write(p) if err == nil { select { - case b.wrote <- struct{}{}: + case w.wrote <- struct{}{}: default: } } @@ -33,16 +31,18 @@ func (b *signalBuffer) Write(p []byte) (n int, err error) { } func TestNewWriter(t *testing.T) { - buf := signalBuffer{wrote: make(chan struct{}, 1)} - w := diode.NewWriter(&buf, 1000, 0, func(missed int) { + var buf bytes.Buffer + sw := signalWriter{w: &buf, wrote: make(chan struct{}, 1)} + w := diode.NewWriter(&sw, 1000, 0, func(missed int) { fmt.Printf("Dropped %d messages\n", missed) }) log := zerolog.New(w) log.Print("test") select { - case <-buf.wrote: - case <-time.After(time.Second): + case <-sw.wrote: + case <-time.After(2 * time.Second): + w.Close() t.Fatal("timed out waiting for diode writer to flush") } @@ -63,110 +63,68 @@ func TestClose(t *testing.T) { } func TestFatal(t *testing.T) { - if os.Getenv("TEST_FATAL") == "1" { - w := diode.NewWriter(os.Stderr, 1000, 0, func(missed int) { - fmt.Printf("Dropped %d messages\n", missed) - }) - defer w.Close() - log := zerolog.New(w) - log.Fatal().Msg("test") - return - } + var buf bytes.Buffer + w := diode.NewWriter(&buf, 1000, 0, func(missed int) { + fmt.Printf("Dropped %d messages\n", missed) + }) + log := zerolog.New(w) - cmd := exec.Command(os.Args[0], "-test.run=TestFatal") - cmd.Env = append(os.Environ(), "TEST_FATAL=1") - stderr, err := cmd.StderrPipe() - if err != nil { - t.Fatal(err) - } - err = cmd.Start() - if err != nil { - t.Fatal(err) - } + oldExitFunc := zerolog.FatalExitFunc + zerolog.FatalExitFunc = func() { panic("fatal exit") } + defer func() { zerolog.FatalExitFunc = oldExitFunc }() - var stderrBuf bytes.Buffer - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - if _, err := io.Copy(&stderrBuf, stderr); err != nil { - t.Errorf("failed to copy stderr: %v", err) + defer func() { + if r := recover(); r == nil || r != "fatal exit" { + t.Fatalf("expected panic %q from log.Fatal(), got %v", "fatal exit", r) + } + want := "{\"level\":\"fatal\",\"message\":\"test\"}\n" + got := cbor.DecodeIfBinaryToString(buf.Bytes()) + if got != want { + t.Errorf("Diode Fatal Test failed. got:%s, want:%s!", got, want) } }() - err = cmd.Wait() - if err == nil { - t.Error("Expected log.Fatal to exit with non-zero status") - } - - wg.Wait() // Wait for the goroutine to finish copying - slurp := stderrBuf.Bytes() - - want := "{\"level\":\"fatal\",\"message\":\"test\"}\n" - got := cbor.DecodeIfBinaryToString(slurp) - if got != want { - t.Errorf("Diode Fatal Test failed. got:%s, want:%s!", got, want) - } + log.Fatal().Msg("test") } -type SlowWriter struct{} +type SlowWriter struct{ w io.Writer } func (rw *SlowWriter) Write(p []byte) (n int, err error) { time.Sleep(200 * time.Millisecond) - fmt.Print(string(p)) - return len(p), nil + return rw.w.Write(p) } func TestFatalWithFilteredLevelWriter(t *testing.T) { - if os.Getenv("TEST_FATAL_SLOW") == "1" { - slowWriter := SlowWriter{} - diodeWriter := diode.NewWriter(&slowWriter, 500, 0, func(missed int) { - fmt.Printf("Missed %d logs\n", missed) - }) - leveledDiodeWriter := zerolog.LevelWriterAdapter{ - Writer: &diodeWriter, - } - filteredDiodeWriter := zerolog.FilteredLevelWriter{ - Writer: &leveledDiodeWriter, - Level: zerolog.InfoLevel, - } - logger := zerolog.New(&filteredDiodeWriter) - logger.Fatal().Msg("test") - return - } - - cmd := exec.Command(os.Args[0], "-test.run=TestFatalWithFilteredLevelWriter") - cmd.Env = append(os.Environ(), "TEST_FATAL_SLOW=1") - stdout, err := cmd.StdoutPipe() - if err != nil { - t.Fatal(err) + var buf bytes.Buffer + slowWriter := SlowWriter{w: &buf} + diodeWriter := diode.NewWriter(&slowWriter, 500, 0, func(missed int) { + fmt.Printf("Missed %d logs\n", missed) + }) + leveledDiodeWriter := zerolog.LevelWriterAdapter{ + Writer: &diodeWriter, } - err = cmd.Start() - if err != nil { - t.Fatal(err) + filteredDiodeWriter := zerolog.FilteredLevelWriter{ + Writer: &leveledDiodeWriter, + Level: zerolog.InfoLevel, } + logger := zerolog.New(&filteredDiodeWriter) - var stdoutBuf bytes.Buffer - var wg sync.WaitGroup - wg.Add(1) - go func() { - defer wg.Done() - _, _ = io.Copy(&stdoutBuf, stdout) - }() + oldExitFunc := zerolog.FatalExitFunc + zerolog.FatalExitFunc = func() { panic("fatal exit") } + defer func() { zerolog.FatalExitFunc = oldExitFunc }() - err = cmd.Wait() - if err == nil { - t.Error("Expected log.Fatal to exit with non-zero status") - } - - wg.Wait() // Wait for the goroutine to finish copying - slurp := stdoutBuf.Bytes() + defer func() { + if r := recover(); r == nil || r != "fatal exit" { + t.Fatalf("expected panic %q from log.Fatal(), got %v", "fatal exit", r) + } + want := "{\"level\":\"fatal\",\"message\":\"test\"}\n" + got := cbor.DecodeIfBinaryToString(buf.Bytes()) + if got != want { + t.Errorf("Expected output %q, got: %q", want, got) + } + }() - got := cbor.DecodeIfBinaryToString(slurp) - want := "{\"level\":\"fatal\",\"message\":\"test\"}\n" - if got != want { - t.Errorf("Expected output %q, got: %q", want, got) - } + logger.Fatal().Msg("test") } func Benchmark(b *testing.B) {