Skip to content

Speed up the local Go test runner with package-scoped, timing-balanced shards - #235

Merged
bigboateng merged 4 commits into
mainfrom
speed-up-local-test-runner
Aug 21, 2026
Merged

Speed up the local Go test runner with package-scoped, timing-balanced shards#235
bigboateng merged 4 commits into
mainfrom
speed-up-local-test-runner

Conversation

@bigboateng

Copy link
Copy Markdown
Contributor

Summary

Full local suite wall-clock on a 14-core Mac: 278.4s → 218.3s (~22% faster), 603 tests, all passing, with all safety invariants kept (-count=1, process isolation, MAX_DEFAULT_JOBS=10, exact-partition refusal, CI workflows untouched).

  • Package-scoped shards: ci_shard.read_test_packages keeps the test-to-package mapping already present in go test -list output; each shard now runs go test -count=1 -v -run <regex> against only the packages owning its tests instead of ./.... This removes ~300 no-op test-binary spawns per run (measured at 18–30s each under 10-way contention on this spawn-latency-bound suite). A verified_package_scope gate refuses the run if any enumerated test lacks an owning package or falls outside its shard's scope, so scoping can never silently skip a test.
  • Timings-fed LPT balancing: per-test durations are parsed from shard output and persisted to ~/.cache/boatstack/test-timings.json (user cache, never the repo), then fed into the existing-but-unused timings hook of assign_shards. Missing or malformed timings degrade to the previous count-balanced behavior.
  • Truthful per-shard elapsed: shard finish times are recorded when each worker process exits instead of after the global join, which previously printed identical times for every shard. The fixed report exposed the real bottleneck: TestExactProductDeliveryFlowReachesPublishedPRWithFakeProvider runs ~210s alone and is now the suite's floor (a candidate for a follow-up, deliberately out of scope here).
  • Failed-shard output strips go test -v pass narration so failures stay readable.

Rejected alternatives (each verified locally, with supervisory-control analysis via Locus): dropping -count=1 (stale cache reuse), in-process t.Parallel (races the ~14 mutable global function seams), and raising the jobs cap (a real 30s subprocess deadline test flaked at --jobs 16 under oversubscription).

Test plan

  • python3 -m unittest discover -s .github/tests -p 'test_*.py' passes (18 runner tests, incl. new cases for package mapping, coverage refusal, timings round-trip, duration parsing)
  • python3 .github/scripts/run_go_tests.py full suite green twice (231.4s cold cache, 218.3s with measured weights), 603 tests enumerated and executed exactly once
  • go vet ./... clean

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex automated review

Verdict: patch is incorrect
Confidence: 0.98

The new package-scoping gate can be defeated by valid TestMain output that resembles a Go package summary, producing a false-success test run. Model-level verification is not recommended; this is a deterministic parser/attribution defect.

Comment on lines +86 to +91
summary = PACKAGE_SUMMARY.match(stripped)
if summary:
package = summary.group(1)
for name in pending:
owners.setdefault(name, set()).add(package)
pending = []

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not derive package ownership from unstructured test output

Invariant: a successful scoped run must execute every enumerated test in every owning package. A valid package p can define TestAlpha and a TestMain that calls m.Run() and then emits ok example.com/mod/q 0.01s; enumeration then produces TestAlpha, that line, and Go's real ok .../p summary. .github/scripts/ci_shard.py assigns TestAlpha to q, clears it before the real summary, and the coverage check accepts the mapping. If q exists without TestAlpha, the scoped worker reports success with no tests run, while p/TestAlpha is skipped. This is introduced by replacing the previous ./... sweep with ownership inferred from ambiguous stdout. The runner can therefore falsely report the complete suite passed. Use package-labelled output or per-package enumeration. The smallest regression test is an integration fixture with packages p and q and the above TestMain, asserting that p/TestAlpha executes or enumeration refuses.

Confidence: 0.98

@bigboateng
bigboateng merged commit 77c1f3b into main Aug 21, 2026
18 checks passed
@bigboateng
bigboateng deleted the speed-up-local-test-runner branch August 21, 2026 08:33
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