diff --git a/internal/worker/logger_test.go b/internal/worker/logger_test.go deleted file mode 100644 index 6dd0f2f4..00000000 --- a/internal/worker/logger_test.go +++ /dev/null @@ -1,23 +0,0 @@ -package worker_test - -import ( - "testing" - - "github.com/hookdeck/outpost/internal/util/testutil" - "github.com/hookdeck/outpost/internal/worker" -) - -// TestLoggingLoggerImplementsInterface verifies that *logging.Logger -// from internal/logging satisfies the worker.Logger interface. -func TestLoggingLoggerImplementsInterface(t *testing.T) { - logger := testutil.CreateTestLogger(t) - - // This will fail to compile if *logging.Logger doesn't implement worker.Logger - var _ worker.Logger = logger - - // Also verify we can actually use it with WorkerSupervisor - supervisor := worker.NewWorkerSupervisor(logger) - if supervisor == nil { - t.Fatal("expected non-nil supervisor") - } -} diff --git a/internal/worker/supervisor.go b/internal/worker/supervisor.go index 9be4aab1..44001d5b 100644 --- a/internal/worker/supervisor.go +++ b/internal/worker/supervisor.go @@ -7,23 +7,16 @@ import ( "sync" "time" + "github.com/hookdeck/outpost/internal/logging" "go.uber.org/zap" ) -// Logger is a minimal logging interface for structured logging with zap. -type Logger interface { - Info(msg string, fields ...zap.Field) - Error(msg string, fields ...zap.Field) - Debug(msg string, fields ...zap.Field) - Warn(msg string, fields ...zap.Field) -} - // WorkerSupervisor manages and supervises multiple workers. // It tracks their health and handles graceful shutdown. type WorkerSupervisor struct { workers map[string]Worker health *HealthTracker - logger Logger + logger *logging.Logger shutdownTimeout time.Duration // 0 means no timeout } @@ -40,7 +33,7 @@ func WithShutdownTimeout(timeout time.Duration) SupervisorOption { } // NewWorkerSupervisor creates a new WorkerSupervisor. -func NewWorkerSupervisor(logger Logger, opts ...SupervisorOption) *WorkerSupervisor { +func NewWorkerSupervisor(logger *logging.Logger, opts ...SupervisorOption) *WorkerSupervisor { r := &WorkerSupervisor{ workers: make(map[string]Worker), health: NewHealthTracker(), diff --git a/internal/worker/worker_test.go b/internal/worker/worker_test.go index eab0dc71..f505b7f4 100644 --- a/internal/worker/worker_test.go +++ b/internal/worker/worker_test.go @@ -8,9 +8,9 @@ import ( "testing" "time" + "github.com/hookdeck/outpost/internal/util/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/zap" ) // Mock worker for testing @@ -50,55 +50,6 @@ func (m *mockWorker) WasStarted() bool { return m.started } -// Mock logger for testing -type mockLogger struct { - mu sync.Mutex - messages []string -} - -func newMockLogger() *mockLogger { - return &mockLogger{ - messages: []string{}, - } -} - -func (l *mockLogger) log(level, msg string, fields ...zap.Field) { - l.mu.Lock() - defer l.mu.Unlock() - l.messages = append(l.messages, fmt.Sprintf("[%s] %s", level, msg)) -} - -func (l *mockLogger) Info(msg string, fields ...zap.Field) { - l.log("INFO", msg, fields...) -} - -func (l *mockLogger) Error(msg string, fields ...zap.Field) { - l.log("ERROR", msg, fields...) -} - -func (l *mockLogger) Debug(msg string, fields ...zap.Field) { - l.log("DEBUG", msg, fields...) -} - -func (l *mockLogger) Warn(msg string, fields ...zap.Field) { - l.log("WARN", msg, fields...) -} - -func (l *mockLogger) Contains(substr string) bool { - l.mu.Lock() - defer l.mu.Unlock() - for _, msg := range l.messages { - if contains(msg, substr) { - return true - } - } - return false -} - -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || s[1:len(s)-1] != s[1:len(s)-1] && contains(s[1:], substr))) -} - // Tests func TestHealthTracker_MarkHealthy(t *testing.T) { @@ -174,18 +125,17 @@ func TestHealthTracker_NoErrorExposed(t *testing.T) { } func TestWorkerSupervisor_RegisterWorker(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) worker := newMockWorker("test-worker", nil) supervisor.Register(worker) assert.Len(t, supervisor.workers, 1) - assert.True(t, logger.Contains("worker registered")) } func TestWorkerSupervisor_RegisterDuplicateWorker(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) worker1 := newMockWorker("test-worker", nil) @@ -199,7 +149,7 @@ func TestWorkerSupervisor_RegisterDuplicateWorker(t *testing.T) { } func TestWorkerSupervisor_Run_HealthyWorkers(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) worker1 := newMockWorker("worker-1", func(ctx context.Context) error { @@ -250,7 +200,7 @@ func TestWorkerSupervisor_Run_HealthyWorkers(t *testing.T) { } func TestWorkerSupervisor_Run_FailedWorker(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) healthyWorker := newMockWorker("healthy", func(ctx context.Context) error { @@ -301,7 +251,7 @@ func TestWorkerSupervisor_Run_FailedWorker(t *testing.T) { } func TestWorkerSupervisor_Run_AllWorkersExit(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) // Both workers exit on their own (not from context cancellation) @@ -339,13 +289,10 @@ func TestWorkerSupervisor_Run_AllWorkersExit(t *testing.T) { workers := status["workers"].(map[string]WorkerHealth) assert.Equal(t, WorkerStatusFailed, workers["worker-1"].Status) assert.Equal(t, WorkerStatusFailed, workers["worker-2"].Status) - - // Verify log message - assert.True(t, logger.Contains("all workers have exited")) } func TestWorkerSupervisor_Run_ContextCanceled(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) worker := newMockWorker("worker-1", func(ctx context.Context) error { @@ -373,7 +320,7 @@ func TestWorkerSupervisor_Run_ContextCanceled(t *testing.T) { } func TestWorkerSupervisor_Run_NoWorkers(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) ctx := context.Background() @@ -381,7 +328,6 @@ func TestWorkerSupervisor_Run_NoWorkers(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "no workers registered") - assert.True(t, logger.Contains("no workers registered")) } func TestHealthTracker_ConcurrentAccess(t *testing.T) { @@ -425,7 +371,7 @@ func TestHealthTracker_ConcurrentAccess(t *testing.T) { } func TestWorkerSupervisor_Run_VariableShutdownTiming(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) // Worker that shuts down quickly (50ms) @@ -483,7 +429,7 @@ func TestWorkerSupervisor_Run_VariableShutdownTiming(t *testing.T) { } func TestWorkerSupervisor_Run_VerySlowShutdown_NoTimeout(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) // No timeout // Worker that takes a very long time to shutdown (2 seconds) @@ -528,7 +474,7 @@ func TestWorkerSupervisor_Run_VerySlowShutdown_NoTimeout(t *testing.T) { } func TestWorkerSupervisor_Run_ShutdownTimeout(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) // Set shutdown timeout to 500ms supervisor := NewWorkerSupervisor(logger, WithShutdownTimeout(500*time.Millisecond)) @@ -573,7 +519,7 @@ func TestWorkerSupervisor_Run_ShutdownTimeout(t *testing.T) { } func TestWorkerSupervisor_Run_ShutdownTimeout_FastWorkers(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) // Set shutdown timeout to 2s supervisor := NewWorkerSupervisor(logger, WithShutdownTimeout(2*time.Second)) @@ -617,7 +563,7 @@ func TestWorkerSupervisor_Run_ShutdownTimeout_FastWorkers(t *testing.T) { } func TestWorkerSupervisor_Run_StuckWorker(t *testing.T) { - logger := newMockLogger() + logger := testutil.CreateTestLogger(t) supervisor := NewWorkerSupervisor(logger) // Worker that never shuts down (ignores context cancellation)