From 7ffacb662028acb573053515987f2d8c688ffc53 Mon Sep 17 00:00:00 2001 From: 0x1d Date: Wed, 5 Nov 2025 21:11:14 +0100 Subject: [PATCH] fix: add mutex to mockLogger in errorbus tests to prevent race conditions The mockLogger's errors slice was being accessed concurrently from multiple goroutines (the error bus consumer and the test goroutine), causing race conditions when running tests with the race detector. Added sync.Mutex to protect the errors slice and proper locking when accessing it in test assertions. --- AGENTS.md | 6 ++++-- Makefile | 4 ++-- internal/errorbus/channel_bus_test.go | 19 ++++++++++++++++--- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 70a0eff..0d29a5b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,8 +150,10 @@ When working on this project, follow this workflow: - Use descriptive branch names (e.g., `feature/epic1-http-server`, `bugfix/auth-token-expiry`, `enhancement/rate-limiting`) - Branch names should follow the pattern: `{type}/{epic}-{short-description}` or `{type}/{story-id}-{short-description}` - **ALWAYS create a commit** after successfully implementing a feature that: - - ✅ Builds successfully (`go build` passes) - - ✅ Tests pass (`go test` passes) + - ✅ Builds successfully (`make build` passes) + - ✅ Tests pass (`make test` passes) + - ✅ Lint pass (`make lint` passes) + - ✅ fmt-check pass (`make fmt-check` passes) - ✅ Meets all acceptance criteria from the story - Commit messages should be clear and descriptive, referencing the story/epic when applicable - Never commit directly to `main` branch diff --git a/Makefile b/Makefile index 4d579f0..bff4f9f 100644 --- a/Makefile +++ b/Makefile @@ -49,11 +49,11 @@ help: # Development commands test: @echo "Running tests..." - CGO_ENABLED=1 $(GO) test -v ./... + CGO_ENABLED=1 $(GO) test -v -race ./... test-coverage: @echo "Running tests with coverage..." - CGO_ENABLED=1 $(GO) test -v -coverprofile=coverage.out ./... + CGO_ENABLED=1 $(GO) test -v -race -coverprofile=coverage.out ./... $(GO) tool cover -html=coverage.out -o coverage.html @echo "Coverage report generated: coverage.html" diff --git a/internal/errorbus/channel_bus_test.go b/internal/errorbus/channel_bus_test.go index 6708542..5d86472 100644 --- a/internal/errorbus/channel_bus_test.go +++ b/internal/errorbus/channel_bus_test.go @@ -3,6 +3,7 @@ package errorbus import ( "context" "errors" + "sync" "testing" "time" @@ -61,7 +62,10 @@ func TestChannelBus_Publish(t *testing.T) { time.Sleep(100 * time.Millisecond) // Verify error was logged - if len(mockLogger.errors) == 0 { + mockLogger.mu.Lock() + errorCount := len(mockLogger.errors) + mockLogger.mu.Unlock() + if errorCount == 0 { t.Error("Expected error to be logged") } @@ -84,7 +88,10 @@ func TestChannelBus_Publish_NilError(t *testing.T) { time.Sleep(50 * time.Millisecond) // Verify nil error was not logged - if len(mockLogger.errors) > 0 { + mockLogger.mu.Lock() + errorCount := len(mockLogger.errors) + mockLogger.mu.Unlock() + if errorCount > 0 { t.Error("Expected nil error to be ignored") } @@ -107,7 +114,10 @@ func TestChannelBus_Publish_WithContext(t *testing.T) { time.Sleep(100 * time.Millisecond) // Verify error was logged with context - if len(mockLogger.errors) == 0 { + mockLogger.mu.Lock() + errorCount := len(mockLogger.errors) + mockLogger.mu.Unlock() + if errorCount == 0 { t.Error("Expected error to be logged") } @@ -181,12 +191,15 @@ func TestChannelBus_ChannelFull(t *testing.T) { // mockLogger implements logger.Logger for testing. type mockLogger struct { errors []string + mu sync.Mutex } func (m *mockLogger) Debug(msg string, fields ...logger.Field) {} func (m *mockLogger) Info(msg string, fields ...logger.Field) {} func (m *mockLogger) Warn(msg string, fields ...logger.Field) {} func (m *mockLogger) Error(msg string, fields ...logger.Field) { + m.mu.Lock() + defer m.mu.Unlock() m.errors = append(m.errors, msg) }