diff --git a/internal/api/auth_denial_signal_test.go b/internal/api/auth_denial_signal_test.go index 2c8da24e7..da66a4e92 100644 --- a/internal/api/auth_denial_signal_test.go +++ b/internal/api/auth_denial_signal_test.go @@ -1,48 +1,20 @@ package api import ( - "bytes" "encoding/json" "net/http/httptest" "strings" "sync" "testing" "time" - - "github.com/rs/zerolog" - "github.com/rs/zerolog/log" ) -// lockedLogBuffer keeps log capture safe when another package goroutine logs -// while an auth-denial test owns the process-wide zerolog logger. bytes.Buffer -// is not safe for concurrent writes or for a read racing a write. -type lockedLogBuffer struct { - mu sync.Mutex - buf bytes.Buffer -} - -func (b *lockedLogBuffer) Write(p []byte) (int, error) { - b.mu.Lock() - defer b.mu.Unlock() - return b.buf.Write(p) -} - -func (b *lockedLogBuffer) String() string { - b.mu.Lock() - defer b.mu.Unlock() - return b.buf.String() -} - -// captureAuthDenialLogs swaps the global logger for a buffer at debug level and -// resets the denial tracker, so each case starts from a known window. +// captureAuthDenialLogs resets the denial tracker and captures logs through the +// process-wide synchronized test sink, so each case starts from a known window. func captureAuthDenialLogs(t *testing.T) *lockedLogBuffer { t.Helper() - var buf lockedLogBuffer - prevLogger := log.Logger - prevLevel := zerolog.GlobalLevel() - log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) - zerolog.SetGlobalLevel(zerolog.DebugLevel) + buf := captureTestLogs(t) authDenialMu.Lock() authDenials = make(map[string]*authDenialCounter) @@ -50,15 +22,13 @@ func captureAuthDenialLogs(t *testing.T) *lockedLogBuffer { prevNow := authDenialNow t.Cleanup(func() { - log.Logger = prevLogger - zerolog.SetGlobalLevel(prevLevel) authDenialNow = prevNow authDenialMu.Lock() authDenials = make(map[string]*authDenialCounter) authDenialMu.Unlock() }) - return &buf + return buf } func countLogEvents(t *testing.T, buf *lockedLogBuffer, level, message string) int { diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 132b7c12d..3f0017a16 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -66,7 +66,6 @@ import ( "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" "github.com/rcourtman/pulse-go-rewrite/pkg/reporting" "github.com/rs/zerolog" - "github.com/rs/zerolog/log" tmock "github.com/stretchr/testify/mock" ) @@ -24487,19 +24486,13 @@ func TestContract_AuthorizationRefusalKeepsStatusWhileLoggingAtDebug(t *testing. // Capture only the request phase — router construction emits unrelated // startup warnings that have nothing to do with authorization. - var logBuf bytes.Buffer - prevLogger := log.Logger - prevLevel := zerolog.GlobalLevel() - log.Logger = zerolog.New(&logBuf).Level(zerolog.DebugLevel) - zerolog.SetGlobalLevel(zerolog.DebugLevel) + logBuf := captureTestLogs(t) authDenialMu.Lock() authDenials = make(map[string]*authDenialCounter) authDenialMu.Unlock() t.Cleanup(func() { - log.Logger = prevLogger - zerolog.SetGlobalLevel(prevLevel) authDenialMu.Lock() authDenials = make(map[string]*authDenialCounter) authDenialMu.Unlock() diff --git a/internal/api/main_test.go b/internal/api/main_test.go index 55e1e9330..eaaea4bb1 100644 --- a/internal/api/main_test.go +++ b/internal/api/main_test.go @@ -19,7 +19,8 @@ func TestMain(m *testing.M) { _ = os.Setenv("PULSE_UPDATE_SERVER", "http://127.0.0.1:1") _ = os.Setenv("PULSE_DATA_DIR", dataDir) allowLoopbackSSOFetch = true - log.Logger = zerolog.Nop() + log.Logger = zerolog.New(testLogSink).Level(zerolog.DebugLevel) + zerolog.SetGlobalLevel(zerolog.DebugLevel) code := m.Run() _ = os.RemoveAll(dataDir) os.Exit(code) diff --git a/internal/api/router_bench_test.go b/internal/api/router_bench_test.go index 87e4ac59e..b808f208c 100644 --- a/internal/api/router_bench_test.go +++ b/internal/api/router_bench_test.go @@ -16,17 +16,12 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" - "github.com/rs/zerolog" - "github.com/rs/zerolog/log" ) -// suppressBenchLogs disables zerolog for the duration of a benchmark to prevent -// log I/O from skewing results. +// TestMain installs a discard-by-default sink, so benchmarks avoid log I/O +// without mutating zerolog's process-global logger. func suppressBenchLogs(b *testing.B) { b.Helper() - orig := log.Logger - log.Logger = zerolog.Nop() - b.Cleanup(func() { log.Logger = orig }) } // setBenchUnexportedField sets an unexported field on a struct via reflection. diff --git a/internal/api/router_webhook_cidr_reload_test.go b/internal/api/router_webhook_cidr_reload_test.go index 901d5cc55..5b8ebf0a5 100644 --- a/internal/api/router_webhook_cidr_reload_test.go +++ b/internal/api/router_webhook_cidr_reload_test.go @@ -8,8 +8,6 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" - "github.com/rs/zerolog" - "github.com/rs/zerolog/log" ) // TestReloadSystemSettings_AppliesWebhookCIDRsToNewMonitor verifies the fix @@ -99,22 +97,11 @@ func TestReloadSystemSettings_AppliesWebhookCIDRsToNewMonitor(t *testing.T) { } } -// captureRouterSettingsLogs swaps the global logger for a locked buffer so +// captureRouterSettingsLogs uses the synchronized process-wide test sink so // tests can assert on warnings emitted by system-settings load failures. func captureRouterSettingsLogs(t *testing.T) *lockedLogBuffer { t.Helper() - - var buf lockedLogBuffer - prevLogger := log.Logger - prevLevel := zerolog.GlobalLevel() - log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel) - zerolog.SetGlobalLevel(zerolog.DebugLevel) - t.Cleanup(func() { - log.Logger = prevLogger - zerolog.SetGlobalLevel(prevLevel) - }) - - return &buf + return captureTestLogs(t) } // unreadableSystemSettings returns a ConfigPersistence whose LoadSystemSettings diff --git a/internal/api/test_log_capture_test.go b/internal/api/test_log_capture_test.go new file mode 100644 index 000000000..17df2030d --- /dev/null +++ b/internal/api/test_log_capture_test.go @@ -0,0 +1,74 @@ +package api + +import ( + "bytes" + "sync" + "testing" +) + +// lockedLogBuffer permits assertions while unrelated package goroutines are +// still emitting log records. +type lockedLogBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *lockedLogBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *lockedLogBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +// synchronizedTestLogSink remains installed for the lifetime of the test +// process. Tests change only its protected destination, never zerolog's +// process-global logger, so background monitor goroutines can log safely. +type synchronizedTestLogSink struct { + mu sync.Mutex + target *lockedLogBuffer +} + +func (s *synchronizedTestLogSink) Write(p []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.target == nil { + return len(p), nil + } + return s.target.Write(p) +} + +var ( + testLogSink = &synchronizedTestLogSink{} + testLogCaptureGate = func() chan struct{} { + gate := make(chan struct{}, 1) + gate <- struct{}{} + return gate + }() +) + +// captureTestLogs serializes the small number of tests that need exact log +// assertions, while all other test logging continues through the stable sink. +func captureTestLogs(t testing.TB) *lockedLogBuffer { + t.Helper() + <-testLogCaptureGate + + buf := &lockedLogBuffer{} + testLogSink.mu.Lock() + testLogSink.target = buf + testLogSink.mu.Unlock() + + t.Cleanup(func() { + testLogSink.mu.Lock() + if testLogSink.target == buf { + testLogSink.target = nil + } + testLogSink.mu.Unlock() + testLogCaptureGate <- struct{}{} + }) + return buf +}