From 2ea9e77f3701a142cea1d6f1cd56ba7c60426d64 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 29 May 2026 18:16:03 -0400 Subject: [PATCH] fix(handlers): stop flaky coverage CI from global-logger race in parallel tests TestDiscoveryLoggingIncludesOptionalRequestID and TestExecutionCleanupService_StartStopBranches both call setupExecutionCleanupTestLogger, which swaps the process-global logger.Logger, while also calling t.Parallel(). When they ran alongside another parallel test, a sibling reassigned the global logger mid-run, so the discovery test's captured buffer came back empty and its assertions ("discovery request failed", `"request_id":"req-123"`, `"format":"compact"`) failed. This broke the `coverage (control-plane)` job and cascaded into `coverage-summary` (missing control-plane.total.txt). The non-test build is unaffected, so it slipped past the required gates and only showed up in the coverage job's full parallel run. Drop t.Parallel() from both tests so they run in the serial phase, where nothing else mutates the global logger concurrently -- matching the existing non-parallel logger-swapping tests in execution_cleanup_test.go. Verified by running the previously-failing command 20x clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../handlers/coverage_handlers_92_target_test.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/control-plane/internal/handlers/coverage_handlers_92_target_test.go b/control-plane/internal/handlers/coverage_handlers_92_target_test.go index ac366881..1bb461c2 100644 --- a/control-plane/internal/handlers/coverage_handlers_92_target_test.go +++ b/control-plane/internal/handlers/coverage_handlers_92_target_test.go @@ -207,7 +207,11 @@ func TestAgentConcurrencyLimiter_MaxPerAgentNilAndConfigured(t *testing.T) { } func TestExecutionCleanupService_StartStopBranches(t *testing.T) { - t.Parallel() + // NOT t.Parallel(): this test swaps the process-global logger.Logger via + // setupExecutionCleanupTestLogger. Running it in parallel let a sibling test + // reassign the global logger mid-run, contaminating log buffers and flaking + // the coverage CI job. Keep it in the serial phase, like the other + // logger-swapping tests in execution_cleanup_test.go. t.Run("disabled start is no-op", func(t *testing.T) { service := NewExecutionCleanupService(&cleanupStoreMock{}, config.ExecutionCleanupConfig{Enabled: false}) @@ -341,7 +345,10 @@ func TestIsLightweightRequestQueryVariants(t *testing.T) { } func TestDiscoveryLoggingIncludesOptionalRequestID(t *testing.T) { - t.Parallel() + // NOT t.Parallel(): this test swaps the process-global logger.Logger via + // setupExecutionCleanupTestLogger and then asserts on the captured buffer. + // In parallel, a sibling test reassigning the global logger emptied this + // buffer and flaked the coverage CI job. Keep it in the serial phase. gin.SetMode(gin.TestMode) logBuffer := setupExecutionCleanupTestLogger(t)