Skip to content

Commit f08667d

Browse files
committed
ci: duration-aware test sharding
Replaces vitest's own --shard (splits test files by COUNT only, no duration awareness) with an explicit, historically-duration-balanced file list per shard for the full-suite case. Confirmed this session, via real per-shard timing pulled from multiple CI runs: shard 4 consistently runs ~20-30% longer than shard 5 (the fastest), every run sampled -- a structural artifact of hash-sort-and-slice sharding, not noise. Since validate-tests-merge waits on all 6 shards, the whole job's wall-clock is set by the slowest one. scripts/fetch-test-timing.mjs pulls real per-test-file historical duration from Codecov's Test Analytics API (test-results/), which already pools JUnit uploads across every push-to-main run -- no need to build a self-tracked history from scratch. Requires a Codecov personal API access token (CODECOV_API_TOKEN, distinct from the existing upload-only CODECOV_TOKEN secret), fetched on a schedule (.github/workflows/test-timing-refresh.yml, every 6 hours) rather than per-PR, since Codecov doesn't publish a rate limit for this endpoint and this repo's PR volume made "query fresh on every PR" a real risk. scripts/compute-test-shards.mjs bin-packs the full test/**/*.test.ts set into 6 balanced shards via greedy LPT (sort files descending by duration, assign each to the currently-lightest shard). New/untracked files (no timing data yet) fall back to the median of known durations rather than 0 -- treating an unknown file as free would let newly-added heavy test files land unbalanced. Verified against this repo's real ~1000 test files and against synthetic realistic-shaped durations (a few heavy outliers, mostly small files): balances to within ~0.1s across all 6 shards. Ships with a hard invariant, checked before any output is written: the union of every shard's file list must equal EXACTLY the discovered file set, no file missing, no file in more than one shard. A violation refuses to write output at all rather than risk it -- a bug here would otherwise mean a test file silently never runs in CI, the exact failure class this session's earlier Codecov-enforcement work exists to prevent. Verified the check actually catches a missing/duplicated/phantom file, not just that it exists. A real bug was caught by this verification, not shipped: the initial greedy "pick the lightest shard" reduce always resolved ties in favor of shard 1 (equal totals never satisfy `<`), which is the common case, not a theoretical one -- it's exactly what happens on this feature's own first run, before any timing data has ever been fetched, when every file falls back to the same weight. Without the fix, 100% of ~1000 files landed in shard 1 while shards 2-6 stayed empty, verified by running the script against this repo's real file set before wiring it into ci.yml. Fixed by rotating the tie-break starting shard after every assignment. Deliberately scoped to the full-suite case only. A PR using scoped test selection (--changed=origin/main, for miner/mcp/discoveryIndex-only diffs) keeps vitest's own native --shard unchanged: that file set isn't known until vitest resolves --changed itself, so a precomputed assignment can't cover it without duplicating vitest's own dependency-graph resolution here. Verified end-to-end locally (not just each piece in isolation): the exact combined shell logic from ci.yml's "Test with coverage" step -- bin-pack, extract this shard's files via the same mapfile/node one-liner, run vitest against them, write JUnit -- run for real against a slice of this repo's actual test files. Full local test/unit suite (979 files, 18,487 tests) passes after all changes, including the new test/unit/compute-test-shards.test.ts (round-robin fallback distribution, exact-set-match with no duplicates, weighted-balance quality, and the invariant's two failure-mode error messages).
1 parent 6c834ab commit f08667d

5 files changed

Lines changed: 443 additions & 1 deletion

File tree

.github/workflows/ci.yml

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,21 @@ jobs:
739739
key: turbo-tests-${{ hashFiles('package-lock.json') }}-${{ github.run_id }}
740740
- name: Prepare test reports dir
741741
run: mkdir -p reports/junit
742+
# Restore-only -- this job never saves it, test-timing-refresh.yml is the sole writer, on its own
743+
# schedule (pulling from Codecov's Test Analytics API, which already pools per-test durations
744+
# across every push-to-main run). key never actually matches (no save ever uses this literal
745+
# string); it exists only so the step always falls through to the restore-keys prefix match,
746+
# picking whatever the most recent refresh wrote. A cache MISS here is always safe: see
747+
# scripts/compute-test-shards.mjs's fallback -- it splits evenly across shards when no timing
748+
# data is available for a file (or none at all), the same balance vitest's own --shard already
749+
# gives today, so this can only make shard balance better than today's baseline, never worse.
750+
- name: Restore test timing cache
751+
uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
752+
with:
753+
path: test-timing.json
754+
key: test-timing-unused
755+
restore-keys: |
756+
test-timing-
742757
- name: Test with coverage (shard ${{ matrix.shard }}/6)
743758
id: coverage
744759
env:
@@ -795,8 +810,22 @@ jobs:
795810
# ~2,900-file suite is 0% covered, polluting Codecov's project trend on every scoped PR. Real
796811
# coverage for files actually exercised is unaffected either way.
797812
SCOPE_ARGS+=(--changed=origin/main --coverage.all=false)
813+
npm run test:coverage -- --maxWorkers=4 --shard=${{ matrix.shard }}/6 --reporter=default --reporter=blob --reporter=junit --outputFile.blob=blob-report/report-${{ matrix.shard }}.blob --outputFile.junit=reports/junit/vitest.xml "${EXCLUDE_ARGS[@]}" "${SCOPE_ARGS[@]}"
814+
else
815+
# Duration-aware sharding (#ci-duration-aware-sharding), full-suite case only: vitest's own
816+
# --shard splits by file COUNT alone, no duration awareness -- confirmed via real per-shard CI
817+
# timing data to produce a consistent ~20-30% gap between the slowest and fastest of the 6
818+
# shards, every run sampled. Deliberately not applied to the scoped-selection branch above:
819+
# that file set isn't known until vitest resolves --changed itself, so a precomputed
820+
# assignment can't cover it without duplicating vitest's own dependency-graph resolution here.
821+
# compute-test-shards.mjs enforces its own hard invariant (the union of all 6 shards' files
822+
# must exactly equal the discovered file set, no file missing or duplicated) and refuses to
823+
# write output at all if that's ever violated, so a bug here fails this step loudly rather
824+
# than silently dropping a test file from CI.
825+
node scripts/compute-test-shards.mjs --shards=6 --timing=test-timing.json --output=shard-assignment.json
826+
mapfile -t SHARD_FILES < <(node -e "console.log(JSON.parse(require('fs').readFileSync('shard-assignment.json','utf8'))['${{ matrix.shard }}'].join('\n'))")
827+
npm run test:coverage -- --maxWorkers=4 "${SHARD_FILES[@]}" --reporter=default --reporter=blob --reporter=junit --outputFile.blob=blob-report/report-${{ matrix.shard }}.blob --outputFile.junit=reports/junit/vitest.xml "${EXCLUDE_ARGS[@]}"
798828
fi
799-
npm run test:coverage -- --maxWorkers=4 --shard=${{ matrix.shard }}/6 --reporter=default --reporter=blob --reporter=junit --outputFile.blob=blob-report/report-${{ matrix.shard }}.blob --outputFile.junit=reports/junit/vitest.xml "${EXCLUDE_ARGS[@]}" "${SCOPE_ARGS[@]}"
800829
- name: Test failure guidance
801830
if: ${{ failure() && steps.coverage.conclusion == 'failure' }}
802831
run: |
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
name: Test timing refresh
2+
3+
# Pulls per-test-file historical duration data from Codecov's Test Analytics API (see
4+
# scripts/fetch-test-timing.mjs) and caches it for validate-tests' duration-aware shard bin-packer
5+
# (scripts/compute-test-shards.mjs, ci.yml's "Test with coverage" step) to consume. Runs on a schedule
6+
# rather than per-PR: Codecov doesn't publish a numeric rate limit for this read endpoint, and this
7+
# repo's PR volume (hundreds/day) makes "query fresh on every PR" a real risk of hitting one, for data
8+
# that doesn't meaningfully change run-to-run anyway.
9+
10+
on:
11+
schedule:
12+
- cron: "0 */6 * * *" # every 6 hours
13+
workflow_dispatch:
14+
15+
permissions:
16+
contents: read
17+
18+
concurrency:
19+
group: test-timing-refresh
20+
cancel-in-progress: true
21+
22+
jobs:
23+
refresh:
24+
name: refresh
25+
runs-on: ubuntu-latest
26+
timeout-minutes: 10
27+
steps:
28+
- name: Checkout
29+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7
30+
- name: Setup Node
31+
uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6
32+
with:
33+
node-version-file: .nvmrc
34+
- name: Fetch test timing from Codecov
35+
env:
36+
CODECOV_API_TOKEN: ${{ secrets.CODECOV_API_TOKEN }}
37+
run: node scripts/fetch-test-timing.mjs --output=test-timing.json
38+
# Run_id-suffixed key (always a fresh entry, never a re-save of an existing one) + prefix
39+
# restore-keys fallback on the consuming side (ci.yml's "Restore test timing cache" step) -- same
40+
# accumulating-cache pattern already used for .turbo/cache and .tsbuildinfo elsewhere in this repo,
41+
# since this data changes every run rather than being wholesale-replaced per some fixed input hash.
42+
- name: Save test timing cache
43+
if: ${{ !cancelled() }}
44+
uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0
45+
with:
46+
path: test-timing.json
47+
key: test-timing-${{ github.run_id }}

scripts/compute-test-shards.mjs

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
#!/usr/bin/env node
2+
// Bin-packs the full test/**/*.test.ts file set into N balanced shards by historical duration (greedy
3+
// LPT -- Longest Processing Time first: sort files descending by duration, repeatedly assign the next
4+
// file to whichever shard currently has the smallest total), replacing vitest's own --shard (file
5+
// COUNT only, no duration awareness -- confirmed this session via real per-shard CI timing data to
6+
// produce a consistent ~20-30% gap between the slowest and fastest of 6 shards, every sampled run).
7+
//
8+
// Deliberately scoped to the full-suite case only -- see ci.yml's "Test with coverage" step for how
9+
// this output is (and is NOT) consumed. A PR using scoped test selection (--changed=origin/main)
10+
// keeps vitest's own native --shard: that file set isn't known until vitest resolves --changed
11+
// itself, so a precomputed assignment can't apply to it without duplicating vitest's own dependency-
12+
// graph resolution here.
13+
//
14+
// HARD INVARIANT, checked before any output is written: the union of every shard's file list must
15+
// equal EXACTLY the discovered test file set, with no file in more than one shard. A violation means
16+
// this script would make CI silently never run some test file at all -- exactly the failure class that
17+
// this whole session's Codecov-enforcement work exists to prevent, just introduced by the tool meant
18+
// to speed that same pipeline up. This refuses to write ANY output if the invariant doesn't hold,
19+
// rather than risk it: a hard failure here (a broken CI step everyone sees) is a wildly better outcome
20+
// than a silent one (missing coverage nobody notices until something ships broken).
21+
22+
import { readdirSync, readFileSync, writeFileSync, existsSync } from "node:fs";
23+
import { join } from "node:path";
24+
25+
const TEST_ROOT = "test";
26+
const EXCLUDED_DIR = join(TEST_ROOT, "workers"); // mirrors vitest.config.ts's exclude: ["test/workers/**/*.test.ts"]
27+
28+
const shardsArg = Number(process.argv.find((a) => a.startsWith("--shards="))?.split("=")[1] ?? 6);
29+
const timingArg = process.argv.find((a) => a.startsWith("--timing="))?.split("=")[1];
30+
const outputArg = process.argv.find((a) => a.startsWith("--output="))?.split("=")[1];
31+
if (!outputArg) throw new Error("--output=<path> is required");
32+
33+
function discoverTestFiles(dir) {
34+
const results = [];
35+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
36+
const full = join(dir, entry.name);
37+
if (entry.isDirectory()) {
38+
if (full === EXCLUDED_DIR) continue;
39+
results.push(...discoverTestFiles(full));
40+
} else if (entry.name.endsWith(".test.ts")) {
41+
results.push(full);
42+
}
43+
}
44+
return results;
45+
}
46+
47+
function loadTimingData(path) {
48+
if (!path || !existsSync(path)) return {};
49+
const parsed = JSON.parse(readFileSync(path, "utf8"));
50+
return parsed.averageSecondsByFile ?? {};
51+
}
52+
53+
function median(values) {
54+
if (values.length === 0) return 0;
55+
const sorted = [...values].sort((a, b) => a - b);
56+
const mid = Math.floor(sorted.length / 2);
57+
return sorted.length % 2 === 0 ? (sorted[mid - 1] + sorted[mid]) / 2 : sorted[mid];
58+
}
59+
60+
function packShards(files, durationByFile, shardCount) {
61+
// New/untracked files (no historical row -- a file added since the last timing refresh, or the
62+
// refresh workflow hasn't run yet at all) get the median of known files' durations rather than 0:
63+
// treating an unknown file as free would let a burst of newly-added heavy test files land in the
64+
// same shard unbalanced, silently reintroducing the exact imbalance this script exists to remove.
65+
// If NO file has any known duration at all (the real state before the first timing refresh has ever
66+
// run), median() of an empty array is 0 -- every file would tie at the same weight, which matters
67+
// below, not just as an edge case: see the tie-break comment.
68+
const knownDurations = Object.values(durationByFile);
69+
const fallback = median(knownDurations);
70+
71+
const weighted = files
72+
.map((file) => ({ file, duration: durationByFile[file] ?? fallback }))
73+
.sort((a, b) => b.duration - a.duration);
74+
75+
const shards = Array.from({ length: shardCount }, (_unused, index) => ({ index, files: [], total: 0 }));
76+
// Picking the lightest shard via a plain reduce always resolves ties in favor of the FIRST shard
77+
// (shard.total < min.total is never true between two equal totals, so the running minimum never
78+
// moves off its starting candidate) -- harmless when durations vary, but catastrophic whenever many
79+
// files tie at the same weight: every one of them would collapse onto shard 1 while the rest stay
80+
// empty. This is the real, common case, not a theoretical one -- it's exactly what happens on this
81+
// script's very first run, before any timing data has ever been fetched (every file falls back to
82+
// the same value, verified by running this script with no --timing argument against this repo's
83+
// real ~1000 test files: without the rotation below, 100% of them landed in shard 1). Rotating the
84+
// tie-break starting point after every assignment makes N equal-weight files distribute round-robin
85+
// across all shards instead, regardless of whether the tied weight happens to be zero or not.
86+
let tiebreakStart = 0;
87+
for (const { file, duration } of weighted) {
88+
let lightest = shards[tiebreakStart];
89+
for (let offset = 1; offset < shardCount; offset += 1) {
90+
const candidate = shards[(tiebreakStart + offset) % shardCount];
91+
if (candidate.total < lightest.total) lightest = candidate;
92+
}
93+
lightest.files.push(file);
94+
lightest.total += duration;
95+
tiebreakStart = (lightest.index + 1) % shardCount;
96+
}
97+
return shards;
98+
}
99+
100+
function assertInvariant(files, shards) {
101+
const original = new Set(files);
102+
const seen = new Set();
103+
const duplicates = [];
104+
for (const shard of shards) {
105+
for (const file of shard.files) {
106+
if (seen.has(file)) duplicates.push(file);
107+
seen.add(file);
108+
}
109+
}
110+
const missing = files.filter((file) => !seen.has(file));
111+
const extra = [...seen].filter((file) => !original.has(file));
112+
113+
if (missing.length > 0 || extra.length > 0 || duplicates.length > 0) {
114+
const details = [
115+
missing.length > 0 ? `missing from every shard: ${JSON.stringify(missing)}` : null,
116+
duplicates.length > 0 ? `assigned to more than one shard: ${JSON.stringify(duplicates)}` : null,
117+
extra.length > 0 ? `assigned but not in the discovered file set: ${JSON.stringify(extra)}` : null,
118+
]
119+
.filter(Boolean)
120+
.join("; ");
121+
throw new Error(`compute-test-shards: shard-assignment invariant violated -- ${details}`);
122+
}
123+
}
124+
125+
if (!existsSync(TEST_ROOT)) {
126+
throw new Error(`compute-test-shards: ${TEST_ROOT}/ does not exist -- run this from the repo root`);
127+
}
128+
const files = discoverTestFiles(TEST_ROOT).sort(); // sorted for deterministic ordering before weighting
129+
if (files.length === 0) throw new Error(`compute-test-shards: discovered zero test files under ${TEST_ROOT}/ -- refusing to write an empty assignment`);
130+
131+
const durationByFile = loadTimingData(timingArg);
132+
const shards = packShards(files, durationByFile, shardsArg);
133+
assertInvariant(files, shards);
134+
135+
const assignment = {};
136+
shards.forEach((shard, index) => {
137+
assignment[String(index + 1)] = shard.files;
138+
});
139+
140+
writeFileSync(outputArg, JSON.stringify(assignment));
141+
142+
const knownCount = files.filter((f) => f in durationByFile).length;
143+
console.log(`Assigned ${files.length} files to ${shardsArg} shards (${knownCount} with known timing, ${files.length - knownCount} using the fallback estimate).`);
144+
shards.forEach((shard, index) => {
145+
console.log(` shard ${index + 1}: ${shard.files.length} files, ~${shard.total.toFixed(1)}s estimated`);
146+
});

scripts/fetch-test-timing.mjs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
#!/usr/bin/env node
2+
// Fetches per-test-file historical duration data from Codecov's Test Analytics API and aggregates it
3+
// into a per-file average, for the test-shard bin-packer (scripts/compute-test-shards.mjs) to consume.
4+
// Codecov already ingests a JUnit report per shard on every push to main (see ci.yml's coverage-upload
5+
// steps, report_type: test_results) and pools it across runs -- this reads that pooled history back
6+
// out instead of this repo tracking its own duration history from scratch.
7+
//
8+
// Filtered to branch=main deliberately: a PR's own JUnit upload is override_branch'd to that PR's own
9+
// branch name (see ci.yml's upload steps), not "main" -- so branch=main naturally selects only
10+
// push-triggered, full-unscoped-suite runs, which is exactly the population the shard bin-packer needs
11+
// (duration-aware sharding only applies to the full-suite case; see compute-test-shards.mjs).
12+
//
13+
// Requires a Codecov personal API access token (Codecov Settings -> Access -> Generate Token), NOT the
14+
// existing CODECOV_TOKEN secret -- that one is an upload-only token and doesn't authenticate this read
15+
// API. Codecov's docs don't publish a numeric rate limit for this endpoint, so this is deliberately run
16+
// on a schedule (test-timing-refresh.yml), not per-PR.
17+
18+
import { writeFileSync } from "node:fs";
19+
20+
const MAX_PAGES = Number(process.argv.find((a) => a.startsWith("--max-pages="))?.split("=")[1] ?? 20);
21+
const outputArg = process.argv.find((a) => a.startsWith("--output="));
22+
const OUTPUT_PATH = outputArg ? outputArg.split("=")[1] : null;
23+
24+
const repo = process.env.GITHUB_REPOSITORY;
25+
if (!repo) throw new Error("GITHUB_REPOSITORY is required (e.g. JSONbored/loopover)");
26+
const [owner, repoName] = repo.split("/");
27+
const token = process.env.CODECOV_API_TOKEN;
28+
if (!token) throw new Error("CODECOV_API_TOKEN is required");
29+
30+
const RETRYABLE_STATUS = new Set([429, 500, 502, 503, 504]);
31+
32+
async function fetchWithRetry(url, maxAttempts = 4) {
33+
for (let attempt = 1; attempt <= maxAttempts; attempt += 1) {
34+
const response = await fetch(url, {
35+
headers: { Authorization: `bearer ${token}`, Accept: "application/json" },
36+
});
37+
if (response.ok) return response.json();
38+
if (!RETRYABLE_STATUS.has(response.status) || attempt === maxAttempts) {
39+
throw new Error(`Codecov API error ${response.status} on ${url}: ${await response.text()}`);
40+
}
41+
const delayMs = 2 ** attempt * 1000;
42+
console.warn(`Codecov API returned ${response.status} (attempt ${attempt}/${maxAttempts}), retrying in ${delayMs}ms`);
43+
await new Promise((resolve) => setTimeout(resolve, delayMs));
44+
}
45+
throw new Error("unreachable");
46+
}
47+
48+
async function fetchAllTestRuns() {
49+
const rows = [];
50+
let url = `https://api.codecov.io/api/v2/gh/${owner}/repos/${repoName}/test-results/?branch=main&page_size=100`;
51+
let pages = 0;
52+
while (url && pages < MAX_PAGES) {
53+
const body = await fetchWithRetry(url);
54+
rows.push(...body.results);
55+
url = body.next;
56+
pages += 1;
57+
}
58+
return { rows, truncated: url !== null };
59+
}
60+
61+
function aggregateByFile(rows) {
62+
// Per-file duration must be averaged ACROSS RUNS, not just summed across every row: a file with many
63+
// test cases would otherwise dwarf a file with few, and a file that appears in many historical rows
64+
// (many runs) would inflate further with each additional run pooled in -- neither reflects "how long
65+
// does this file actually take in a single run." So first sum each file's rows *within* a single
66+
// commit (that commit's real per-run file duration), then average those per-commit totals across all
67+
// commits the file appears in.
68+
const perCommitTotals = new Map(); // filename -> Map(commit_sha -> totalSeconds)
69+
for (const row of rows) {
70+
if (!row.filename || row.duration_seconds == null) continue;
71+
if (!perCommitTotals.has(row.filename)) perCommitTotals.set(row.filename, new Map());
72+
const commits = perCommitTotals.get(row.filename);
73+
commits.set(row.commit_sha, (commits.get(row.commit_sha) ?? 0) + row.duration_seconds);
74+
}
75+
76+
const averages = {};
77+
for (const [filename, commits] of perCommitTotals) {
78+
const totals = [...commits.values()];
79+
averages[filename] = totals.reduce((sum, value) => sum + value, 0) / totals.length;
80+
}
81+
return averages;
82+
}
83+
84+
const { rows, truncated } = await fetchAllTestRuns();
85+
const averageSecondsByFile = aggregateByFile(rows);
86+
87+
const report = {
88+
fetchedAt: new Date().toISOString(),
89+
sourceRowCount: rows.length,
90+
fileCount: Object.keys(averageSecondsByFile).length,
91+
truncated, // true if MAX_PAGES was hit before the API ran out of pages -- more history existed than was pulled
92+
averageSecondsByFile,
93+
};
94+
95+
const json = JSON.stringify(report, null, 2);
96+
if (OUTPUT_PATH) {
97+
writeFileSync(OUTPUT_PATH, json);
98+
console.log(`Wrote ${report.fileCount} files' timing data (from ${report.sourceRowCount} rows) to ${OUTPUT_PATH}`);
99+
} else {
100+
process.stdout.write(`${json}\n`);
101+
}

0 commit comments

Comments
 (0)