Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 92 additions & 57 deletions .github/workflows/ci.yml

Large diffs are not rendered by default.

48 changes: 29 additions & 19 deletions .github/workflows/shard-timings-refresh.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
#
# scripts/test-shard-timings.json is the per-package duration dataset the Test
# Core split is binned from. It is GENERATED — scripts/measure-test-shard-
# timings.mjs turns a green run's six turbo summaries into it — but until now it
# was generated only when a person remembered to. It went 13 days without a
# timings.mjs turns a green run's per-shard turbo summaries into it — but until
# now it was generated only when a person remembered to. It went 13 days without a
# refresh while ~700 test files were added, and the shard it mis-weighted was
# killed by the job wall twelve times in one day and ejected from the merge queue
# twice (#16173).
Expand Down Expand Up @@ -46,8 +46,11 @@
#
# HOW IT GETS ITS INPUTS, AND WHY IT RUNS HERE RATHER THAN IN AN AGENT WORKTREE
# ----------------------------------------------------------------------------
# The documented refresh path downloads the six `test-core-run-summary-N-of-6`
# artifacts of a green run. `GET /actions/artifacts/{id}/zip` redirects to
# The documented refresh path downloads the `test-core-run-summary-N-of-M`
# artifacts of a green run, one per Test Core shard. M is the RUN's own width,
# read from its `Test Core (N/M)` job names, so runs from before and after a
# matrix resize both read (#25376).
# `GET /actions/artifacts/{id}/zip` redirects to
# `productionresultssa*.blob.core.windows.net`, and every agent container's
# egress policy answers 403 to CONNECT for that host — reproduced four ways, on
# two artifacts across two runs, on the pre-signed URL as well as the API one, on
Expand All @@ -71,8 +74,8 @@
# Until #16467 a push to `main` re-ran the FULL Test Core battery, so "a green
# push run on main" and "a measurement of the workspace" were the same thing.
# They are not any more: a push run computes its package set with `--affected`
# against `github.event.before`. Its six shards still conclude `success` and
# still upload six run summaries, so NOTHING in the eligibility test would have
# against `github.event.before`. Its shards still conclude `success` and
# still upload every run summary, so NOTHING in the eligibility test would have
# noticed — this lane would have kept regenerating the balancing dataset from
# measurements of whatever the last merge happened to touch, and the coverage
# check would not have caught it either (a package the affected set skipped is a
Expand All @@ -96,8 +99,8 @@
# follows from the cache design rather than from luck — turbo's key is
# namespaced per shard and only main pushes write it, so a package whose inputs
# have not changed is a HIT, and the generator refuses hits rather than
# recording a replayed ~0.1s window as a suite's cost. "Download six artifacts
# from any green run" therefore measures a SLICE of the workspace. That number
# recording a replayed ~0.1s window as a suite's cost. "Download the shard
# artifacts from any green run" therefore measures a SLICE of the workspace. That number
# should IMPROVE under the hourly run, because a push no longer seeds the whole
# workspace's entries — but it is quoted as it was measured, and the next live
# run is what re-measures it.
Expand Down Expand Up @@ -178,7 +181,7 @@
# - DOWNS, in one line.
# - FILES OVER BUDGET: every test file over 10 s of wall in the window, with
# its package and its slowest run (#22998). The dataset holds no per-file
# reading; the window's runs do, in the per-shard `test-core-timings-N-of-6`
# reading; the window's runs do, in the per-shard `test-core-timings-N-of-M`
# captures, which a step before the body downloads: the same runs, retained
# exactly as long as their run summaries.
#
Expand Down Expand Up @@ -432,14 +435,14 @@ jobs:
# not bad luck, it is the cache design: turbo's key is namespaced per shard
# and only main pushes write it, so a package whose inputs have not changed
# is a HIT — and the generator refuses hits rather than recording a
# replayed ~0.1s window as a suite's cost. "Download six artifacts from any
# green run" therefore measures a SLICE of the workspace, never all of it.
# replayed ~0.1s window as a suite's cost. "Download the shard artifacts
# from any green run" therefore measures a SLICE of the workspace, never all of it.
# Since #16467 the runs read here are the HOURLY `schedule` runs, which are
# the full battery; the accumulation stays because the cache argument above
# is unchanged by which event ran the suite.
#
# So runs are accumulated. Each contributes its six summaries under its own
# `--run <id>` group, which is exactly the grouping #16473 added: slices are
# So runs are accumulated. Each contributes its shards' summaries under its
# own `--run <id>` group, which is exactly the grouping #16473 added: slices are
# summed within a run and the per-run sums are medianed across runs, so a
# package measured by three of these runs gets a median of three
# observations rather than whichever run happened to be read last. Feeding
Expand Down Expand Up @@ -482,7 +485,13 @@ jobs:
rm -rf "$SUMDIR"; mkdir -p "$SUMDIR"

OK=1
for N in 1 2 3 4 5 6; do
# One summary per shard the selector found: it admits a run only when
# every shard OF THAT RUN concluded success with its artifact
# retained, at the run's own width (select-shard-timings-run.mjs,
# runShardWidth), so these keys ARE that run's roster. A count
# spelled here would break the first refresh after a resize (#25376).
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")
for N in $SHARD_NS; do
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")
# curl drops the Authorization header across the redirect to the
# blob host, which is correct: that URL is pre-signed and the
Expand Down Expand Up @@ -636,7 +645,7 @@ jobs:
# The per-file half of the regression readings (#22949 ruling E, item 3).
# The dataset holds no per-file durations; the window's runs do, in the
# per-shard captures `report-test-timings.mjs --capture` uploads as
# `test-core-timings-N-of-6`, retained 1 day like the run summaries the
# `test-core-timings-N-of-M`, retained 1 day like the run summaries the
# generator read. The selector listed their ids beside the summaries'
# (`timing_artifact_ids`), so this is a read of the SAME runs, never a new
# measurement, and it feeds the PR body alone.
Expand Down Expand Up @@ -822,7 +831,8 @@ jobs:
const rs = JSON.parse(require("fs").readFileSync(process.env.RUNNER_TEMP + "/candidates.json", "utf8"));
const r = rs.find((x) => String(x.run_id) === process.env.RUN_ID);
const s = r.shard_seconds ?? {};
console.log([1,2,3,4,5,6].map((n) => (s[n] == null ? "?" : `${n}: ${s[n]}s`)).join(" | "));
const ns = Object.keys(r.artifact_ids ?? {}).map(Number).sort((a, b) => a - b);
console.log(ns.map((n) => (s[n] == null ? "?" : `${n}: ${s[n]}s`)).join(" | "));
')

{
Expand Down Expand Up @@ -851,8 +861,8 @@ jobs:
done
echo
echo "- Newest run in the set: \`$RUN_ID\`, commit \`$HEAD_SHA\` — the date this refresh carries."
echo "- Every run above had all six \`Test Core (N/6)\` jobs conclude \`success\` with its six"
echo " run-summary artifacts still retained; runs that were cancelled, failed or had lost"
echo "- Every run above had every \`Test Core (N/M)\` shard job conclude \`success\` with its"
echo " run-summary artifact still retained; runs that were cancelled, failed or had lost"
echo " their artifacts were rejected by name in the log before any of these were used."
echo
echo "$CARRY_LINE"
Expand Down Expand Up @@ -886,7 +896,7 @@ jobs:
echo "## The partitioner's own pins RED on this refresh — read this before merging"
echo
echo "This is the designed behaviour, not a defect in the refresh: the acceptance bound is"
echo "a ratio, and a package that has grown past what any six-way split can bin makes the"
echo "a ratio, and a package that has grown past what the Test Core split can bin makes the"
echo "pins fail with the arithmetic in the message. The remedy the partitioner names is to"
echo "raise the file-level slice count for that package — ⛔ never to raise the bound, and"
echo "⛔ never to hand-edit this dataset. This workflow deliberately does neither: it"
Expand Down
6 changes: 3 additions & 3 deletions scripts/check-required-contexts.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2026,13 +2026,13 @@ async function selfTest() {
);

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

Expand Down
23 changes: 15 additions & 8 deletions scripts/check-shard-attestation.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2085,6 +2085,13 @@ async function selfTest() {
// ── #22653: the per-shard cause read itself, offline ─────────────────────
battery('#22653 the cause read, offline: one jobs list, then one annotations read per unattested shard');
const CI_TEXT = readFileSync(join(scriptRepoRoot(), '.github', 'workflows', 'ci.yml'), 'utf8');
// The runs replayed below (38036705318, 34007386254) ran the SIX-shard matrix,
// which #25376 widened to eight, so they are read against the live workflow
// with the shard name as it stood then. The live names are pinned on their own
// at the end of this battery.
const LIVE_TEST_NAME = 'name: Test Core (${{ matrix.shard }}/8)';
const CI_TEXT_SIX_SHARDS = CI_TEXT.replace(LIVE_TEST_NAME, 'name: Test Core (${{ matrix.shard }}/6)');
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`);
const PARTIAL_JOBS = /\/repos\/o\/r\/actions\/runs\/38036705318\/attempts\/1\/jobs\?per_page=100&page=1$/;
const ANY_ANNOTATIONS = /\/check-runs\/\d+\/annotations\?per_page=100$/;
const shardCheckRun = (n) => 114168570850 + n;
Expand All @@ -2109,7 +2116,7 @@ async function selfTest() {
apiUrl: API,
token: 'not-a-real-token',
fetchJson,
workflowText: CI_TEXT,
workflowText: CI_TEXT_SIX_SHARDS,
...over,
});

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

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');
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');
assert([1, 2, 3].every((n) => workflowJobDisplayName(CI_TEXT, 'dogfood', n) === `Dogfood Regression Gate (${n}/3)`), '#22653: …and each dogfood shard');
assert(workflowJobDisplayName(CI_TEXT, 'dogfood-verify') === 'Dogfood Verify CLI', "#22653 control: the single leg's literal name reads as before");
assert(workflowJobDisplayName(CI_TEXT, 'test') === undefined, '#22653: a matrix template asked WITHOUT a shard is still unresolvable ⇒ undefined');
Expand Down Expand Up @@ -2480,8 +2487,8 @@ async function selfTest() {
// `String.replace` takes the first match. A short anchor would silently
// start mutating dogfood's matrix instead — still red, still for a
// plausible-looking reason, and no longer the assertion written here.
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');
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');
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');
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');
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');
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');
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');
Expand Down Expand Up @@ -2573,13 +2580,13 @@ async function selfTest() {
hasProblem(await fixture('drop the token (test-gate)', inJob('test-gate', TOKEN_LINE, '')), "gate 'test-gate'", 'GITHUB_TOKEN'),
"#22653: test-gate's verify step without GITHUB_TOKEN ⇒ red",
);
const TEST_NAME = ' name: Test Core (${{ matrix.shard }}/6)\n';
const TEST_NAME = ' name: Test Core (${{ matrix.shard }}/8)\n';
assert(
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"),
'#22653: a matrix name with no `${{ matrix.shard }}` ⇒ red — the platform would append the values itself, a name the reader cannot rebuild',
);
assert(
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"),
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"),
'#22653: any expression besides the shard ⇒ red — the text reading cannot evaluate it',
);
assert(
Expand All @@ -2591,11 +2598,11 @@ async function selfTest() {
"#22653: dogfood's matrix leg is held to the same name rule — it is asked on a partial cancel too",
);
assert(
(await fixture('the matrix name double-quoted', (s) => s.replace(TEST_NAME, ' name: "Test Core (${{ matrix.shard }}/6)"\n'))).problems.length === 0,
(await fixture('the matrix name double-quoted', (s) => s.replace(TEST_NAME, ' name: "Test Core (${{ matrix.shard }}/8)"\n'))).problems.length === 0,
'#22653 positive limb: a quoted template reads the same from text and from YAML ⇒ green',
);
assert(
(await fixture('the shard expression without inner spaces', (s) => s.replace(TEST_NAME, ' name: Test Core (${{matrix.shard}}/6)\n'))).problems.length === 0,
(await fixture('the shard expression without inner spaces', (s) => s.replace(TEST_NAME, ' name: Test Core (${{matrix.shard}}/8)\n'))).problems.length === 0,
'#22653 positive limb: `${{matrix.shard}}` is the same expression ⇒ green',
);
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');
Expand Down
Loading
Loading