From 6e096412c1755783c8471a2590d73b1e01bf8bfe Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 13 Feb 2026 18:11:19 +0100 Subject: [PATCH] Replace time.Sleep in tests with deterministic synchronization Use require.Eventually, time.AfterFunc, and channel synchronization instead of fixed time.Sleep calls to make tests faster and less flaky. - telemetry: poll mockHTTP.GetRequestCount() with require.Eventually - fake/proxy: use channel write and require.Eventually for sync points - runtime/fallback: use time.AfterFunc for context cancel, require.Eventually for cooldown expiry - tui/styles: poll atomic callback counter with require.Eventually Assisted-By: cagent --- pkg/fake/proxy_test.go | 10 +++--- pkg/runtime/fallback_test.go | 13 +++----- pkg/runtime/runtime_test.go | 3 +- pkg/telemetry/telemetry_test.go | 50 ++++++++++++---------------- pkg/tui/styles/theme_watcher_test.go | 23 +++++-------- 5 files changed, 41 insertions(+), 58 deletions(-) diff --git a/pkg/fake/proxy_test.go b/pkg/fake/proxy_test.go index e99ae5c72..5927cb637 100644 --- a/pkg/fake/proxy_test.go +++ b/pkg/fake/proxy_test.go @@ -234,8 +234,8 @@ func TestStreamCopy_ContextCancellation(t *testing.T) { done <- StreamCopy(c, resp) }() - // Give StreamCopy time to start and block on the read - time.Sleep(50 * time.Millisecond) + // Write some data to ensure StreamCopy is actively reading before we cancel + slowBody.data <- []byte("initial") // Cancel the context - this should cause StreamCopy to return immediately cancel() @@ -328,8 +328,10 @@ func TestSimulatedStreamCopy_ContextCancellation(t *testing.T) { done <- SimulatedStreamCopy(c, resp, 10*time.Millisecond) }() - // Wait for data to be written - time.Sleep(50 * time.Millisecond) + // Wait until at least the first chunk has been written to the recorder + require.Eventually(t, func() bool { + return rec.Body.Len() > 0 + }, time.Second, 5*time.Millisecond, "expected first chunk to be written") // Cancel the context and close the body (simulating client disconnect) cancel() diff --git a/pkg/runtime/fallback_test.go b/pkg/runtime/fallback_test.go index 9119393d2..a2da62810 100644 --- a/pkg/runtime/fallback_test.go +++ b/pkg/runtime/fallback_test.go @@ -243,10 +243,7 @@ func TestSleepWithContext(t *testing.T) { ctx, cancel := context.WithCancel(t.Context()) // Cancel context after a short delay - go func() { - time.Sleep(10 * time.Millisecond) - cancel() - }() + time.AfterFunc(10*time.Millisecond, cancel) start := time.Now() completed := sleepWithContext(ctx, 1*time.Second) @@ -699,11 +696,9 @@ func TestFallbackCooldownState(t *testing.T) { assert.Equal(t, 0, state.fallbackIndex) // Wait for cooldown to expire - time.Sleep(150 * time.Millisecond) - - // Should no longer be in cooldown - state = rt.getCooldownState(agentName) - assert.Nil(t, state, "cooldown should have expired") + require.Eventually(t, func() bool { + return rt.getCooldownState(agentName) == nil + }, time.Second, 10*time.Millisecond, "cooldown should have expired") // Set cooldown again and then clear it rt.setCooldownState(agentName, 1, 1*time.Hour) diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index 344b987d3..5b1a88933 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -577,7 +577,8 @@ func TestStartBackgroundRAGInit_StopsForwardingAfterContextCancel(t *testing.T) // Cancel the context and ensure no further events are forwarded. cancel() - // Give the forwarder time to observe cancellation. + // Brief yield to allow the forwarder goroutine to observe cancellation. + // This is a timing-based negative test: we verify no event is forwarded. time.Sleep(10 * time.Millisecond) // Emit another event; it should NOT be forwarded. diff --git a/pkg/telemetry/telemetry_test.go b/pkg/telemetry/telemetry_test.go index 5d35c13b8..28d8c968b 100644 --- a/pkg/telemetry/telemetry_test.go +++ b/pkg/telemetry/telemetry_test.go @@ -128,12 +128,11 @@ func TestSessionTracking(t *testing.T) { // Multiple ends should be safe client.RecordSessionEnd(ctx) - // Wait for events to be processed - time.Sleep(20 * time.Millisecond) + require.Eventually(t, func() bool { + return mockHTTP.GetRequestCount() > 0 + }, time.Second, 5*time.Millisecond, "Expected HTTP requests to be made for session tracking events") requestCount := mockHTTP.GetRequestCount() - assert.Positive(t, requestCount, "Expected HTTP requests to be made for session tracking events") - t.Logf("Session tracking HTTP requests captured: %d", requestCount) requests := mockHTTP.GetRequests() @@ -165,12 +164,11 @@ func TestCommandTracking(t *testing.T) { require.NoError(t, err) assert.True(t, executed) - // Wait for events to be processed - time.Sleep(20 * time.Millisecond) + require.Eventually(t, func() bool { + return mockHTTP.GetRequestCount() > 0 + }, time.Second, 5*time.Millisecond, "Expected HTTP requests to be made for command tracking") requestCount := mockHTTP.GetRequestCount() - assert.Positive(t, requestCount, "Expected HTTP requests to be made for command tracking") - t.Logf("Command tracking HTTP requests captured: %d", requestCount) requests := mockHTTP.GetRequests() @@ -200,12 +198,11 @@ func TestCommandTrackingWithError(t *testing.T) { assert.Equal(t, testErr, err) - // Wait for events to be processed - time.Sleep(20 * time.Millisecond) + require.Eventually(t, func() bool { + return mockHTTP.GetRequestCount() > 0 + }, time.Second, 5*time.Millisecond, "Expected HTTP requests to be made for command error tracking") requestCount := mockHTTP.GetRequestCount() - assert.Positive(t, requestCount, "Expected HTTP requests to be made for command error tracking") - t.Logf("Command error tracking HTTP requests captured: %d", requestCount) } @@ -493,11 +490,11 @@ func TestAllEventTypes(t *testing.T) { // End session client.RecordSessionEnd(ctx) - // Wait for events to be processed - time.Sleep(20 * time.Millisecond) + require.Eventually(t, func() bool { + return mockHTTP.GetRequestCount() > 0 + }, time.Second, 5*time.Millisecond, "Expected HTTP requests to be made for telemetry events") requestCount := mockHTTP.GetRequestCount() - assert.Positive(t, requestCount, "Expected HTTP requests to be made for telemetry events") t.Logf("Total HTTP requests captured: %d", requestCount) @@ -609,14 +606,12 @@ func TestHTTPRequestVerification(t *testing.T) { client.Track(ctx, event) - // Give time for background processing - time.Sleep(20 * time.Millisecond) + require.Eventually(t, func() bool { + return mockHTTP.GetRequestCount() > 0 + }, time.Second, 5*time.Millisecond, "Expected HTTP request to be made") - // Debug output t.Logf("HTTP requests captured: %d", mockHTTP.GetRequestCount()) - assert.Positive(t, mockHTTP.GetRequestCount(), "Expected HTTP request to be made") - requests := mockHTTP.GetRequests() req := requests[0] @@ -699,11 +694,9 @@ func TestNon2xxHTTPResponseHandling(t *testing.T) { client.Track(t.Context(), &CommandEvent{Action: "error-test", Success: true}) - // Give time for background processing - time.Sleep(20 * time.Millisecond) - - requestCount := mockHTTP.GetRequestCount() - assert.Positive(t, requestCount, "Expected HTTP request to be made despite error response") + require.Eventually(t, func() bool { + return mockHTTP.GetRequestCount() > 0 + }, time.Second, 5*time.Millisecond, "Expected HTTP request to be made despite error response") mockHTTP.SetResponse(&http.Response{ StatusCode: http.StatusNotFound, @@ -714,8 +707,7 @@ func TestNon2xxHTTPResponseHandling(t *testing.T) { client.Track(t.Context(), &CommandEvent{Action: "not-found-test", Success: true}) - time.Sleep(20 * time.Millisecond) - - finalRequestCount := mockHTTP.GetRequestCount() - assert.GreaterOrEqual(t, finalRequestCount, 2, "Expected at least 2 HTTP requests (500 + 404)") + require.Eventually(t, func() bool { + return mockHTTP.GetRequestCount() >= 2 + }, time.Second, 5*time.Millisecond, "Expected at least 2 HTTP requests (500 + 404)") } diff --git a/pkg/tui/styles/theme_watcher_test.go b/pkg/tui/styles/theme_watcher_test.go index 0d78bc7c5..727baafa6 100644 --- a/pkg/tui/styles/theme_watcher_test.go +++ b/pkg/tui/styles/theme_watcher_test.go @@ -47,9 +47,6 @@ colors: err := watcher.Watch("test-theme") require.NoError(t, err) - // Give the watcher time to start - time.Sleep(100 * time.Millisecond) - // Modify the theme file updatedContent := `version: 1 name: Updated Theme @@ -58,11 +55,10 @@ colors: ` require.NoError(t, os.WriteFile(themePath, []byte(updatedContent), 0o644)) - // Wait for the debounce timer and callback - time.Sleep(1 * time.Second) - - // Verify callback was called with the correct themeRef - assert.GreaterOrEqual(t, callbackCount.Load(), int32(1), "callback should have been called at least once") + // Wait for the watcher to detect the change and fire the callback + require.Eventually(t, func() bool { + return callbackCount.Load() >= 1 + }, 5*time.Second, 50*time.Millisecond, "callback should have been called at least once") ref, ok := lastThemeRef.Load().(string) assert.True(t, ok) assert.Equal(t, "test-theme", ref, "callback should receive the correct theme ref") @@ -205,14 +201,11 @@ func TestThemeWatcher_SignalsOnAnyFileChange(t *testing.T) { err := watcher.Watch("signal-test") require.NoError(t, err) - time.Sleep(100 * time.Millisecond) - // Write any content (even invalid YAML) - watcher just signals, doesn't validate require.NoError(t, os.WriteFile(themePath, []byte("this is: [not: valid yaml"), 0o644)) - // Wait for debounce - time.Sleep(1 * time.Second) - - // Callback SHOULD be called because watcher only signals - validation is TUI's job - assert.GreaterOrEqual(t, callbackCount.Load(), int32(1), "callback should be called for any file change") + // Wait for the watcher to detect the change and fire the callback + require.Eventually(t, func() bool { + return callbackCount.Load() >= 1 + }, 5*time.Second, 50*time.Millisecond, "callback should be called for any file change") }