Skip to content

Commit ebc7c8f

Browse files
committed
ci(test-shards): widen Test Core from 6 to 8 shards
Sized on merge_group run 40155484566 on current main (59 packages executed, the CLI as 3 slices). Replayed through planRun() with a wall model calibrated on that run, the slowest shard drops from ~1696s at six (measured 1794s) to ~1436s at eight, -15%; -14% on the dataset's full list and -10% on a 21-package merge_group run. Predicted bins go from 2489s to 1868s, ratio 1.00x, with spec's 1607s floor binding only at ten. - SHARD_COUNT 6 -> 8 in partition-test-shards.mjs and the timings selector; ci.yml matrix, name, --shard, --total, artifact names and the aggregator's --leg move with it. - shard-timings-refresh.yml reads the shard roster from the selector's candidates instead of spelling 1..6. - Test Core timeout-minutes 50 -> 60: the stall-guard headroom tool over the last day's 512 shard jobs found 50 already short at six (worst p + s 40m27s); scaled to eight, the cap-deferred verdict lands at 56m37s. - Self-test fixtures that read the live ci.yml follow the new names; the replays of measured six-shard runs read the six-shard name. - The standing "6 is enough" comments are rewritten with the new numbers. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M781cxsyRAqT49LvLkJKXb
1 parent 97277f1 commit ebc7c8f

8 files changed

Lines changed: 226 additions & 144 deletions

‎.github/workflows/ci.yml‎

Lines changed: 92 additions & 57 deletions
Large diffs are not rendered by default.

‎.github/workflows/shard-timings-refresh.yml‎

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
#
88
# scripts/test-shard-timings.json is the per-package duration dataset the Test
99
# Core split is binned from. It is GENERATED — scripts/measure-test-shard-
10-
# timings.mjs turns a green run's six turbo summaries into it — but until now it
11-
# was generated only when a person remembered to. It went 13 days without a
10+
# timings.mjs turns a green run's per-shard turbo summaries into it — but until
11+
# now it was generated only when a person remembered to. It went 13 days without a
1212
# refresh while ~700 test files were added, and the shard it mis-weighted was
1313
# killed by the job wall twelve times in one day and ejected from the merge queue
1414
# twice (#16173).
@@ -46,8 +46,9 @@
4646
#
4747
# HOW IT GETS ITS INPUTS, AND WHY IT RUNS HERE RATHER THAN IN AN AGENT WORKTREE
4848
# ----------------------------------------------------------------------------
49-
# The documented refresh path downloads the six `test-core-run-summary-N-of-6`
50-
# artifacts of a green run. `GET /actions/artifacts/{id}/zip` redirects to
49+
# The documented refresh path downloads the `test-core-run-summary-N-of-M`
50+
# artifacts of a green run, one per Test Core shard (M = SHARD_COUNT).
51+
# `GET /actions/artifacts/{id}/zip` redirects to
5152
# `productionresultssa*.blob.core.windows.net`, and every agent container's
5253
# egress policy answers 403 to CONNECT for that host — reproduced four ways, on
5354
# two artifacts across two runs, on the pre-signed URL as well as the API one, on
@@ -71,8 +72,8 @@
7172
# Until #16467 a push to `main` re-ran the FULL Test Core battery, so "a green
7273
# push run on main" and "a measurement of the workspace" were the same thing.
7374
# They are not any more: a push run computes its package set with `--affected`
74-
# against `github.event.before`. Its six shards still conclude `success` and
75-
# still upload six run summaries, so NOTHING in the eligibility test would have
75+
# against `github.event.before`. Its shards still conclude `success` and
76+
# still upload every run summary, so NOTHING in the eligibility test would have
7677
# noticed — this lane would have kept regenerating the balancing dataset from
7778
# measurements of whatever the last merge happened to touch, and the coverage
7879
# check would not have caught it either (a package the affected set skipped is a
@@ -96,8 +97,8 @@
9697
# follows from the cache design rather than from luck — turbo's key is
9798
# namespaced per shard and only main pushes write it, so a package whose inputs
9899
# have not changed is a HIT, and the generator refuses hits rather than
99-
# recording a replayed ~0.1s window as a suite's cost. "Download six artifacts
100-
# from any green run" therefore measures a SLICE of the workspace. That number
100+
# recording a replayed ~0.1s window as a suite's cost. "Download the shard
101+
# artifacts from any green run" therefore measures a SLICE of the workspace. That number
101102
# should IMPROVE under the hourly run, because a push no longer seeds the whole
102103
# workspace's entries — but it is quoted as it was measured, and the next live
103104
# run is what re-measures it.
@@ -178,7 +179,7 @@
178179
# - DOWNS, in one line.
179180
# - FILES OVER BUDGET: every test file over 10 s of wall in the window, with
180181
# its package and its slowest run (#22998). The dataset holds no per-file
181-
# reading; the window's runs do, in the per-shard `test-core-timings-N-of-6`
182+
# reading; the window's runs do, in the per-shard `test-core-timings-N-of-M`
182183
# captures, which a step before the body downloads: the same runs, retained
183184
# exactly as long as their run summaries.
184185
#
@@ -432,14 +433,14 @@ jobs:
432433
# not bad luck, it is the cache design: turbo's key is namespaced per shard
433434
# and only main pushes write it, so a package whose inputs have not changed
434435
# is a HIT — and the generator refuses hits rather than recording a
435-
# replayed ~0.1s window as a suite's cost. "Download six artifacts from any
436-
# green run" therefore measures a SLICE of the workspace, never all of it.
436+
# replayed ~0.1s window as a suite's cost. "Download the shard artifacts
437+
# from any green run" therefore measures a SLICE of the workspace, never all of it.
437438
# Since #16467 the runs read here are the HOURLY `schedule` runs, which are
438439
# the full battery; the accumulation stays because the cache argument above
439440
# is unchanged by which event ran the suite.
440441
#
441-
# So runs are accumulated. Each contributes its six summaries under its own
442-
# `--run <id>` group, which is exactly the grouping #16473 added: slices are
442+
# So runs are accumulated. Each contributes its shards' summaries under its
443+
# own `--run <id>` group, which is exactly the grouping #16473 added: slices are
443444
# summed within a run and the per-run sums are medianed across runs, so a
444445
# package measured by three of these runs gets a median of three
445446
# observations rather than whichever run happened to be read last. Feeding
@@ -482,7 +483,12 @@ jobs:
482483
rm -rf "$SUMDIR"; mkdir -p "$SUMDIR"
483484
484485
OK=1
485-
for N in 1 2 3 4 5 6; do
486+
# One summary per shard the selector found: it admits a run only when
487+
# every one of SHARD_COUNT (select-shard-timings-run.mjs) shards
488+
# concluded success with its artifact retained, so these keys ARE the
489+
# roster. A count spelled here would be a third copy of it (#25376).
490+
SHARD_NS=$(node -e 'const r=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"))[Number(process.argv[1])];console.log(Object.keys(r.artifact_ids).map(Number).sort((a,b)=>a-b).join(" "))' "$i")
491+
for N in $SHARD_NS; do
486492
ART=$(node -e 'const r=JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP+"/candidates.json","utf8"))[Number(process.argv[1])];console.log(r.artifact_ids[process.argv[2]])' "$i" "$N")
487493
# curl drops the Authorization header across the redirect to the
488494
# blob host, which is correct: that URL is pre-signed and the
@@ -636,7 +642,7 @@ jobs:
636642
# The per-file half of the regression readings (#22949 ruling E, item 3).
637643
# The dataset holds no per-file durations; the window's runs do, in the
638644
# per-shard captures `report-test-timings.mjs --capture` uploads as
639-
# `test-core-timings-N-of-6`, retained 1 day like the run summaries the
645+
# `test-core-timings-N-of-M`, retained 1 day like the run summaries the
640646
# generator read. The selector listed their ids beside the summaries'
641647
# (`timing_artifact_ids`), so this is a read of the SAME runs, never a new
642648
# measurement, and it feeds the PR body alone.
@@ -822,7 +828,8 @@ jobs:
822828
const rs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8"));
823829
const r = rs.find((x) => String(x.run_id) === process.env.RUN_ID);
824830
const s = r.shard_seconds ?? {};
825-
console.log([1,2,3,4,5,6].map((n) => (s[n] == null ? "?" : `${n}: ${s[n]}s`)).join(" | "));
831+
const ns = Object.keys(r.artifact_ids ?? {}).map(Number).sort((a, b) => a - b);
832+
console.log(ns.map((n) => (s[n] == null ? "?" : `${n}: ${s[n]}s`)).join(" | "));
826833
')
827834
828835
{
@@ -851,8 +858,8 @@ jobs:
851858
done
852859
echo
853860
echo "- Newest run in the set: \`$RUN_ID\`, commit \`$HEAD_SHA\` — the date this refresh carries."
854-
echo "- Every run above had all six \`Test Core (N/6)\` jobs conclude \`success\` with its six"
855-
echo " run-summary artifacts still retained; runs that were cancelled, failed or had lost"
861+
echo "- Every run above had every \`Test Core (N/M)\` shard job conclude \`success\` with its"
862+
echo " run-summary artifact still retained; runs that were cancelled, failed or had lost"
856863
echo " their artifacts were rejected by name in the log before any of these were used."
857864
echo
858865
echo "$CARRY_LINE"
@@ -886,7 +893,7 @@ jobs:
886893
echo "## The partitioner's own pins RED on this refresh — read this before merging"
887894
echo
888895
echo "This is the designed behaviour, not a defect in the refresh: the acceptance bound is"
889-
echo "a ratio, and a package that has grown past what any six-way split can bin makes the"
896+
echo "a ratio, and a package that has grown past what the Test Core split can bin makes the"
890897
echo "pins fail with the arithmetic in the message. The remedy the partitioner names is to"
891898
echo "raise the file-level slice count for that package — ⛔ never to raise the bound, and"
892899
echo "⛔ never to hand-edit this dataset. This workflow deliberately does neither: it"

‎scripts/check-required-contexts.mjs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2026,13 +2026,13 @@ async function selfTest() {
20262026
);
20272027

20282028
// ── (9) the shadowing collision, on the live specimen ────────────────────
2029-
// ci.yml's sharded `test` job is named `Test Core (${{ matrix.shard }}/6)`
2029+
// ci.yml's sharded `test` job is named `Test Core (${{ matrix.shard }}/8)`
20302030
// and its aggregate gate is named `Test Core`. Dropping the suffix makes two
20312031
// jobs publish one context, and the surviving conclusion is whichever
20322032
// finished last — a shard could satisfy the aggregate's required gate.
20332033
battery('(9) the shadowing collision, on the live specimen');
20342034
const collided = fixture('collide the shard name with the gate name', 'ci.yml', (s) =>
2035-
s.replace('name: Test Core (${{ matrix.shard }}/6)', 'name: Test Core'),
2035+
s.replace('name: Test Core (${{ matrix.shard }}/8)', 'name: Test Core'),
20362036
);
20372037
assert(
20382038
collided.problems.some((p) => p.includes("job 'test'") && p.includes("published by ci.yml:test-gate")),
@@ -2041,7 +2041,7 @@ async function selfTest() {
20412041
// The suffixed spelling is NOT a collision — the guard must not read a
20422042
// prefix as a clash, or ci.yml is red on main today.
20432043
assert(
2044-
baseline.problems.length === 0 && sources['ci.yml'].includes('name: Test Core (${{ matrix.shard }}/6)'),
2044+
baseline.problems.length === 0 && sources['ci.yml'].includes('name: Test Core (${{ matrix.shard }}/8)'),
20452045
'the real suffixed shard name coexists with the bare gate name (assertion 9 compares whole names, not prefixes)',
20462046
);
20472047

‎scripts/check-shard-attestation.mjs‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2085,6 +2085,13 @@ async function selfTest() {
20852085
// ── #22653: the per-shard cause read itself, offline ─────────────────────
20862086
battery('#22653 the cause read, offline: one jobs list, then one annotations read per unattested shard');
20872087
const CI_TEXT = readFileSync(join(scriptRepoRoot(), '.github', 'workflows', 'ci.yml'), 'utf8');
2088+
// The runs replayed below (38036705318, 34007386254) ran the SIX-shard matrix,
2089+
// which #25376 widened to eight, so they are read against the live workflow
2090+
// with the shard name as it stood then. The live names are pinned on their own
2091+
// at the end of this battery.
2092+
const LIVE_TEST_NAME = 'name: Test Core (${{ matrix.shard }}/8)';
2093+
const CI_TEXT_SIX_SHARDS = CI_TEXT.replace(LIVE_TEST_NAME, 'name: Test Core (${{ matrix.shard }}/6)');
2094+
assert(CI_TEXT_SIX_SHARDS !== CI_TEXT, `#22653: the live ci.yml no longer carries \`${LIVE_TEST_NAME}\` — the six-shard replays below would read the live roster`);
20882095
const PARTIAL_JOBS = /\/repos\/o\/r\/actions\/runs\/38036705318\/attempts\/1\/jobs\?per_page=100&page=1$/;
20892096
const ANY_ANNOTATIONS = /\/check-runs\/\d+\/annotations\?per_page=100$/;
20902097
const shardCheckRun = (n) => 114168570850 + n;
@@ -2109,7 +2116,7 @@ async function selfTest() {
21092116
apiUrl: API,
21102117
token: 'not-a-real-token',
21112118
fetchJson,
2112-
workflowText: CI_TEXT,
2119+
workflowText: CI_TEXT_SIX_SHARDS,
21132120
...over,
21142121
});
21152122

@@ -2177,7 +2184,7 @@ async function selfTest() {
21772184
'#22653: a matrix name with no shard expression cannot locate a shard ⇒ unreadable, naming the shard',
21782185
);
21792186

2180-
assert([1, 2, 3, 4, 5, 6].every((n) => workflowJobDisplayName(CI_TEXT, 'test', n) === `Test Core (${n}/6)`), '#22653: the real ci.yml names each Test Core shard as the jobs API lists it');
2187+
assert([1, 2, 3, 4, 5, 6, 7, 8].every((n) => workflowJobDisplayName(CI_TEXT, 'test', n) === `Test Core (${n}/8)`), '#22653: the real ci.yml names each Test Core shard as the jobs API lists it');
21812188
assert([1, 2, 3].every((n) => workflowJobDisplayName(CI_TEXT, 'dogfood', n) === `Dogfood Regression Gate (${n}/3)`), '#22653: …and each dogfood shard');
21822189
assert(workflowJobDisplayName(CI_TEXT, 'dogfood-verify') === 'Dogfood Verify CLI', "#22653 control: the single leg's literal name reads as before");
21832190
assert(workflowJobDisplayName(CI_TEXT, 'test') === undefined, '#22653: a matrix template asked WITHOUT a shard is still unresolvable ⇒ undefined');
@@ -2480,8 +2487,8 @@ async function selfTest() {
24802487
// `String.replace` takes the first match. A short anchor would silently
24812488
// start mutating dogfood's matrix instead — still red, still for a
24822489
// plausible-looking reason, and no longer the assertion written here.
2483-
assert((await fixture('grow the matrix', (s) => s.replace('shard: [1, 2, 3, 4, 5, 6]', 'shard: [1, 2, 3, 4, 5, 6, 7]'))).problems.some((p) => p.includes('declares 7 shard')), 'growing the matrix without the gate ⇒ red');
2484-
assert((await fixture('shrink the roster', (s) => s.replace('--leg "test/6', '--leg "test/2'))).problems.some((p) => p.includes('expects 2 attestation')), 'shrinking the gate roster without the matrix ⇒ red');
2490+
assert((await fixture('grow the matrix', (s) => s.replace('shard: [1, 2, 3, 4, 5, 6, 7, 8]', 'shard: [1, 2, 3, 4, 5, 6, 7, 8, 9]'))).problems.some((p) => p.includes('declares 9 shard')), 'growing the matrix without the gate ⇒ red');
2491+
assert((await fixture('shrink the roster', (s) => s.replace('--leg "test/8', '--leg "test/2'))).problems.some((p) => p.includes('expects 2 attestation')), 'shrinking the gate roster without the matrix ⇒ red');
24852492
assert((await fixture('uncounted intruder', (s) => s.replace(/^jobs:$/m, 'jobs:\n intruder:\n runs-on: ubuntu-latest\n steps:\n - run: node scripts/check-shard-attestation.mjs --emit'))).problems.some((p) => p.includes('no gate counts')), 'an attestation no gate counts ⇒ red');
24862493
assert((await fixture('rename test-gate', (s) => s.replace(' test-gate:', ' test-gate-renamed:'))).problems.some((p) => p.includes("'test-gate'")), 'losing a required-context gate ⇒ red');
24872494
assert((await fixture('typo the artifact name', (s) => s.replace('name: shard-attest-test-', 'name: shard-attest-typo-'))).problems.some((p) => p.includes('cannot count')), 'an artifact name the gate cannot match ⇒ red');
@@ -2573,13 +2580,13 @@ async function selfTest() {
25732580
hasProblem(await fixture('drop the token (test-gate)', inJob('test-gate', TOKEN_LINE, '')), "gate 'test-gate'", 'GITHUB_TOKEN'),
25742581
"#22653: test-gate's verify step without GITHUB_TOKEN ⇒ red",
25752582
);
2576-
const TEST_NAME = ' name: Test Core (${{ matrix.shard }}/6)\n';
2583+
const TEST_NAME = ' name: Test Core (${{ matrix.shard }}/8)\n';
25772584
assert(
25782585
hasProblem(await fixture('a matrix name without the shard expression', (s) => s.replace(TEST_NAME, ' name: Test Core shards\n')), '#22653', "job 'test' is named"),
25792586
'#22653: a matrix name with no `${{ matrix.shard }}` ⇒ red — the platform would append the values itself, a name the reader cannot rebuild',
25802587
);
25812588
assert(
2582-
hasProblem(await fixture('a matrix name with a second expression', (s) => s.replace(TEST_NAME, ' name: Test Core (${{ matrix.shard }}/6) ${{ github.event_name }}\n')), '#22653', "job 'test' is named"),
2589+
hasProblem(await fixture('a matrix name with a second expression', (s) => s.replace(TEST_NAME, ' name: Test Core (${{ matrix.shard }}/8) ${{ github.event_name }}\n')), '#22653', "job 'test' is named"),
25832590
'#22653: any expression besides the shard ⇒ red — the text reading cannot evaluate it',
25842591
);
25852592
assert(
@@ -2591,11 +2598,11 @@ async function selfTest() {
25912598
"#22653: dogfood's matrix leg is held to the same name rule — it is asked on a partial cancel too",
25922599
);
25932600
assert(
2594-
(await fixture('the matrix name double-quoted', (s) => s.replace(TEST_NAME, ' name: "Test Core (${{ matrix.shard }}/6)"\n'))).problems.length === 0,
2601+
(await fixture('the matrix name double-quoted', (s) => s.replace(TEST_NAME, ' name: "Test Core (${{ matrix.shard }}/8)"\n'))).problems.length === 0,
25952602
'#22653 positive limb: a quoted template reads the same from text and from YAML ⇒ green',
25962603
);
25972604
assert(
2598-
(await fixture('the shard expression without inner spaces', (s) => s.replace(TEST_NAME, ' name: Test Core (${{matrix.shard}}/6)\n'))).problems.length === 0,
2605+
(await fixture('the shard expression without inner spaces', (s) => s.replace(TEST_NAME, ' name: Test Core (${{matrix.shard}}/8)\n'))).problems.length === 0,
25992606
'#22653 positive limb: `${{matrix.shard}}` is the same expression ⇒ green',
26002607
);
26012608
assert(baseline.problems.length === 0 && baseline.gateIds.includes('test-gate'), '#22653: the checked-in test-gate carries every prerequisite — the baseline is green with it counted as a gate');

0 commit comments

Comments
 (0)