Skip to content

fix(ci): resolve go test 10-minute timeout failures in internal/mcp#251

Merged
zzet merged 3 commits into
mainfrom
fix/mcp-test-ci-timeout
Jul 4, 2026
Merged

fix(ci): resolve go test 10-minute timeout failures in internal/mcp#251
zzet merged 3 commits into
mainfrom
fix/mcp-test-ci-timeout

Conversation

@zzet

@zzet zzet commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Summary

  • CI's test and benchmark jobs never passed -timeout to go test, so they relied on Go's silent 10-minute-per-package default. internal/mcp had grown past it, failing PR fix(mcp): stop the stdio proxy gating tools/call in defer mode #247 and several recent main runs with panic: test timed out after 10m0s.
  • Root cause: 50 test-local helpers across the package each called parser.NewRegistry() + languages.RegisterAll() fresh, recompiling all ~30 tree-sitter grammar queries every time. Under -race that cost is amplified enough to dominate the package's runtime. Replaced with one shared, lazily-built registry (internal/mcp/testregistry_test.go).
  • Also closed a real (if minor) leak: TestSuppressFinding_SuppressesAcrossReviews opened an on-disk SQLite sidecar and never closed it.

Verified locally (same machine, before/after):

go test -race ./internal/mcp/...
Before 305.65s
After ~50-75s

Test plan

  • go build ./...
  • go vet ./...
  • go test -race -timeout=20m -count=1 ./internal/mcp/... — passes in ~52s, coverage unchanged (71.8%)

zzet added 3 commits July 4, 2026 22:23
Both steps relied on go test's silent 10-minute default, which internal/mcp
was starting to exceed on ubuntu-latest, failing CI runs with no actionable
error beyond a goroutine dump.
50 test-local helpers across the package each called parser.NewRegistry()
followed by languages.RegisterAll(), recompiling all ~30 tree-sitter grammar
queries from scratch every time. Under -race that cost is amplified enough
that the package's local runtime drops from ~305s to ~45-75s once every
helper shares one lazily-built registry instead. No test in the package
mutates the registry after RegisterAll or runs in parallel, so sharing one
instance is safe.
TestSuppressFinding_SuppressesAcrossReviews wires a real on-disk sidecar via
InitSuppressions(t.TempDir(), dir) but never closed it, leaking a *sql.DB
and its connectionOpener goroutine for the life of the test binary.
@zzet zzet merged commit 7d309e7 into main Jul 4, 2026
10 checks passed
@zzet zzet deleted the fix/mcp-test-ci-timeout branch July 4, 2026 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant