ci: shard the full-suite coverage run across a matrix - #4815
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4815 +/- ##
=======================================
Coverage 94.23% 94.23%
=======================================
Files 442 442
Lines 38845 38845
Branches 14150 14150
=======================================
Hits 36607 36607
Misses 1577 1577
Partials 661 661
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-11 00:38:03 UTC
⏸️ Suggested Action - Manual Review
Review summary Blockers
Nits — 6 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
0fbb548 to
71646d3
Compare
validate-code's "Test with coverage" step was ~9-10 of the ~11 minutes a typical backend PR spent in CI: vitest schedules whole test files atomically to workers, so a single job with --maxWorkers=4 (a standard runner's real CPU budget) is capped at 4-way parallelism no matter how many test files exist. Split it into a new validate-tests job, matrixed 4 ways over vitest's own --shard=<i>/<n> flag, giving N*4-way parallelism instead. Each shard uploads its own lcov.info + junit report to Codecov; multiple uploads for the same commit/PR are additive there, which is the standard pattern for matrix-split suites. Sets COVERAGE_NO_THRESHOLDS=true per shard, finally wiring up vitest.config.ts's existing (previously dormant) support for it -- a shard only exercises part of the tree, so the global 90% backstop threshold would otherwise always false-fail per shard. validate-code keeps everything else (drift checks, typecheck, build engine, worker runtime tests, MCP/miner build+pack, REES, UI toolchain) as a single job, since those are fast and don't benefit from sharing a matrix. The public repo's standard runners are free regardless of job count, so this costs nothing extra -- only wall-clock time drops. Updates the two workflow-introspection tests (codecov-policy, workflow-runner-labels) that asserted the old single-job shape.
…erage The AI review agent correctly flagged that sharding disabled vitest.config.ts's global 90% coverage-threshold check everywhere in CI (COVERAGE_NO_THRESHOLDS=true on every shard, unconditionally), leaving only Codecov's patch gate -- which only covers changed lines, not a whole-repo regression outside the diff (e.g. a deleted test file). Add a validate-tests-merge job: each shard now also writes a vitest blob report (--reporter=blob) and uploads it as a build artifact; validate-tests-merge downloads all 4, merges them via vitest's own --mergeReports, and re-runs the global threshold check (without COVERAGE_NO_THRESHOLDS) against the combined whole-suite result -- restoring the exact backstop the old single unsharded job had.
71646d3 to
fde6376
Compare
merge-multiple download-artifact flattens all 4 shards into one directory; every shard wrote to blob-report/report.blob, so they collided/corrupted on download (vitest's readBlobs failed with a JSON syntax error mid-file). Suffix each shard's blob filename with its shard index so they land in all-blob-reports/ without colliding.
…nner CPU count (#4951) validate-code's Codecov upload steps moved to validate-tests when coverage was split out (#4815) -- it no longer needs fetch-depth: 0 (measured: shallow checkout is ~3s faster than full-history on this repo). Also adds a temporary nproc diagnostic to validate-tests to check whether --maxWorkers=4 (pinned to the assumed standard-runner vCPU count) actually matches the real runner spec, or whether there's free parallelism available without adding more shards.
Summary
validate-code's "Test with coverage" step was ~9-10 of the ~11 minutes a typical backend PR spent in CI (confirmed via real timing pulled from ~15 recent CI runs): vitest schedules whole test files atomically to workers, so a single job with--maxWorkers=4(a standard GitHub-hosted runner's real CPU budget) is capped at 4-way parallelism no matter how many test files exist.validate-testsjob, matrixed 4 ways over vitest's own--shard=<i>/<n>flag, giving N*4-way parallelism instead of 4-way.lcov.info+ junit report to Codecov; multiple uploads for the same commit/PR are additive there (standard pattern for matrix-split suites).COVERAGE_NO_THRESHOLDS=trueper shard, wiring upvitest.config.ts's existing (previously dormant/unused) support for it -- a shard only exercises part of the tree, so the global 90% backstop threshold would otherwise always false-fail per shard.validate-codekeeps everything else (drift checks, typecheck, build engine, worker runtime tests, MCP/miner build+pack, REES, UI toolchain) as a single job, since those are fast (~1 min total) and don't benefit from a matrix.codecov-policy.test.ts,workflow-runner-labels.test.ts) that asserted the old single-job shape.Why
Investigated at the user's request: CI
validatewas taking 10+ minutes per PR. Real timing data from recent runs showed ~85-95% of that was the single unsharded coverage step; everything else (path-filtered drift checks, typecheck, UI toolchain) was already fast thanks to the existingchanges/paths-filterjob.Best paired with #4814 (splitting the two oversized test files) for balanced shards, but works standalone too.
Test plan
npx tsc --noEmitcleannpm run actionlintcleanpython3 -c "import yaml; yaml.safe_load(...)")--shard=<i>/<n>+COVERAGE_NO_THRESHOLDS=truelocally: threshold failure reproduces without the flag, disappears with it, and still produces a valid coverage reportcodecov-policy.test.ts/workflow-runner-labels.test.tsassertions now pass against the new job shape