Skip to content

Fix race condition in TestOllamaAdapter_ContextCancellation causing CI hangs #272

Description

@fullsend-ai-retro

What happened

During the CI run for PR #242 on 2026-09-21, the Go 1.24 Unit + Integration Tests job (run 35624800904) timed out after 15 minutes. The stack trace pointed to TestOllamaAdapter_ContextCancellation in internal/aireport/adapter_ollama_test.go. The CI log shows httptest.Server blocked in Close after 5 seconds, waiting for connections with an active TCP connection that never drained. The subsequent run (35627742889) on the same commit passed, confirming this is a flaky test, not a real regression.

What could go better

The test has two structural issues causing intermittent hangs:

  1. Missing synchronization: The test calls cancel() on the line immediately after spawning the goroutine that calls adapter.Format(ctx, ...). There is no signal to confirm the HTTP request has arrived at the server before cancellation fires. Under CI resource pressure, the scheduler timing can cause the server handler's <-r.Context().Done() to never see the cancellation propagate through the HTTP transport, leaving the handler goroutine blocked.

  2. Missing test-level timeout: The <-done channel receive has no timeout. If the goroutine hangs, the test hangs indefinitely until the Go test binary's -timeout flag kills the entire binary — which in CI is 15 minutes.

The failure occurred only on Go 1.24 (not 1.25), suggesting a Go version difference in how httptest.Server.Close() propagates connection shutdown to handler goroutines. This is a test-fixture resilience issue, not a production code bug.

Confidence: HIGH — the root cause is clearly visible in the test structure and confirmed by the CI log showing the blocked server connection.

Proposed change

In internal/aireport/adapter_ollama_test.go, apply two fixes to TestOllamaAdapter_ContextCancellation:

  1. Add a synchronization channel to ensure the HTTP request is in-flight before cancelling:

    • Add started := make(chan struct{}) before the server handler.
    • In the handler, close(started) before <-r.Context().Done().
    • After spawning the goroutine, <-started before calling cancel().
  2. Add a timeout on the done channel receive to fail fast with a clear message instead of hanging:

    • Replace err := <-done with a select that includes a case <-time.After(5 * time.Second): t.Fatal("Format did not return after context cancellation").

Both fixes should be applied together — the synchronization ensures deterministic behavior, and the timeout ensures the test fails fast (in 5 seconds with a clear message) rather than hanging the CI job for 15 minutes if something still goes wrong.

Validation criteria

  1. Run go test -race -count=10 -run TestOllamaAdapter_ContextCancellation ./internal/aireport/ on both Go 1.24 and Go 1.25 — all 10 iterations should pass without hanging.
  2. The next 3 CI runs on main should not show any timeout in the Unit + Integration Tests job for internal/aireport.
  3. If the test fails, it should fail within 5 seconds with the message Format did not return after context cancellation, not after 15 minutes with a generic timeout.

Generated by retro agent from #242

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingready-for-triageTriggers triage agent dispatchready-to-codeTriggers code agent dispatch

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions