From 115e245ace86ccc7cf6eb3452ca9b4ab19fe2301 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 6 Nov 2026 08:31:18 -0800 Subject: [PATCH] free-ci-health: a split shard's overrun compares wall time with its prediction over jobs (GST-1653) (#4085) shardOverruns compared a shard's wall-clock elapsedMs with predictedMs, the shard's serial seed sum. free-tests.yml plans every shard at --shard-jobs 4, so the 1.5x warning only fired after about a 6x slowdown. The prediction is now divided by the plan shard's jobs, ciHealthSummary carries jobs through, and the warning names the parallel job count. k=1 output is unchanged. Rewritten on main from #4051, which fixes #3851. Co-authored-by: Indra Prasetya Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../capy-gst-1653-shard-overrun-jobs.md | 1 + docs/TESTING_INTERNALS.md | 4 ++- scripts/lib/free-ci-health.ts | 20 +++++++++----- test/free-seed-ratchet.test.ts | 26 +++++++++++++++++++ 4 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 changelog.d/capy-gst-1653-shard-overrun-jobs.md diff --git a/changelog.d/capy-gst-1653-shard-overrun-jobs.md b/changelog.d/capy-gst-1653-shard-overrun-jobs.md new file mode 100644 index 000000000..67267a5a2 --- /dev/null +++ b/changelog.d/capy-gst-1653-shard-overrun-jobs.md @@ -0,0 +1 @@ +- **The free-CI shard-overrun warning now works on split shards.** `--ci-verify` compared a shard's wall time with its serial seed prediction, so at the CI default of 4 sub-shards per shard it only warned after about a 6x slowdown. The prediction is now divided by the shard's job count, and the warning says how many parallel jobs it ran across. Thanks to @indraoverflow (#4051, #3851). diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index 76ff503ad..5c549acb6 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -1594,7 +1594,9 @@ refresh to a maintainer. - `--ci-plan` warns in the job summary, with the refresh command, when more than 5 files are unseeded; it never fails a PR. `--ci-verify` lists shards that ran - longer than 1.5× their prediction and at least 30 seconds over. + longer than 1.5× their prediction and at least 30 seconds over. A shard split + by `--shard-jobs k` runs its k sub-shards in parallel, so its prediction is the + serial seed sum divided by k. - The weekly test-health run fails its enforced `unseeded-free-files` check ("stale seed: N unseeded file(s) > 5") when more than 5 free test files are missing from the seed, so contributors without Ubicloud access learn within a diff --git a/scripts/lib/free-ci-health.ts b/scripts/lib/free-ci-health.ts index aebf6dfc4..00ba76f5a 100644 --- a/scripts/lib/free-ci-health.ts +++ b/scripts/lib/free-ci-health.ts @@ -30,17 +30,23 @@ export function unseededWarning(unseeded: string[]): string | null { + `Fix: run \`${SEED_REFRESH_COMMAND}\` and commit scripts/free-test-durations.json. See ${SEED_DOCS_ANCHOR}.`; } -export interface ShardTiming { shard: number; predictedMs: number; elapsedMs: number } +/** `predictedMs` is the shard's serial seed sum; `elapsedMs` is wall time across its `jobs` parallel sub-shards. */ +export interface ShardTiming { shard: number; predictedMs: number; elapsedMs: number; jobs?: number } + +const expectedWallMs = ({ predictedMs, jobs }: ShardTiming) => predictedMs / Math.max(1, jobs ?? 1); export function shardOverruns(timings: ShardTiming[]): ShardTiming[] { - return timings.filter(({ predictedMs, elapsedMs }) => - elapsedMs > predictedMs * SHARD_OVERRUN_FACTOR && elapsedMs - predictedMs >= SHARD_OVERRUN_FLOOR_MS); + return timings.filter((timing) => { + const expectedMs = expectedWallMs(timing); + return timing.elapsedMs > expectedMs * SHARD_OVERRUN_FACTOR && timing.elapsedMs - expectedMs >= SHARD_OVERRUN_FLOOR_MS; + }); } export function overrunWarning(overruns: ShardTiming[]): string | null { if (overruns.length === 0) return null; - const listed = overruns.map(({ shard, predictedMs, elapsedMs }) => - `shard ${shard} took ${seconds(elapsedMs)} vs ${seconds(predictedMs)} predicted`).join('; '); + const listed = overruns.map((timing) => + `shard ${timing.shard} took ${seconds(timing.elapsedMs)} vs ${seconds(expectedWallMs(timing))} predicted` + + ((timing.jobs ?? 1) > 1 ? ` across ${timing.jobs} parallel jobs` : '')).join('; '); return `WARNING: ${overruns.length} shard(s) exceeded ${SHARD_OVERRUN_FACTOR}x their predicted duration (${listed}). ` + `A file in them is slower than its seed entry or unseeded. Fix: run \`${SEED_REFRESH_COMMAND}\` ` + `and commit scripts/free-test-durations.json; split any file over ${seconds(SEED_CEILING_MS)}. See ${SEED_DOCS_ANCHOR}.`; @@ -50,8 +56,8 @@ export function overrunWarning(overruns: ShardTiming[]): string | null { export function ciHealthSummary(plan: FreeCiPlan, results: FreeCiResult[]): string[] { const ordered = [...results].sort((a, b) => a.outcome.shard - b.outcome.shard); const overruns = shardOverruns(ordered.flatMap(({ outcome }) => { - const predictedMs = plan.shards[outcome.shard - 1]?.predictedMs; - return predictedMs === undefined ? [] : [{ shard: outcome.shard, predictedMs, elapsedMs: outcome.elapsedMs }]; + const planned = plan.shards[outcome.shard - 1]; + return planned === undefined ? [] : [{ shard: outcome.shard, predictedMs: planned.predictedMs, elapsedMs: outcome.elapsedMs, jobs: planned.jobs }]; })); const warning = overrunWarning(overruns); return warning ? [warning] : []; diff --git a/test/free-seed-ratchet.test.ts b/test/free-seed-ratchet.test.ts index 31950be21..ab28f6ae3 100644 --- a/test/free-seed-ratchet.test.ts +++ b/test/free-seed-ratchet.test.ts @@ -134,6 +134,15 @@ describe('free lane CI warnings', () => { expect(overrunWarning([])).toBeNull(); }); + test('a shard split across parallel jobs is measured against its prediction divided by its job count', () => { + const timing = (shard: number, jobs: number, elapsedMs: number) => ({ shard, predictedMs: 400_000, elapsedMs, jobs }); + expect(shardOverruns([timing(1, 1, 610_000), timing(2, 1, 590_000)]).map(overrun => overrun.shard)).toEqual([1]); + const overruns = shardOverruns([timing(1, 4, 160_000), timing(2, 4, 140_000)]); + expect(overruns.map(overrun => overrun.shard)).toEqual([1]); + expect(overrunWarning(overruns)).toContain('shard 1 took 160s vs 100s predicted across 4 parallel jobs'); + expect(overrunWarning([timing(1, 1, 610_000)])).toContain('shard 1 took 610s vs 400s predicted)'); + }); + test('the --ci-verify summary lists shards that outran the seed, counting the first attempt only', () => { const outcome = (shard: number, files: string[], failingFiles: string[], elapsedMs: number): FreeShardOutcome => ({ shard, files, failingFiles, elapsedMs, status: failingFiles.length ? 'failed' : 'passed' }) as FreeShardOutcome; @@ -151,4 +160,21 @@ describe('free lane CI warnings', () => { expect(summary[0]).not.toContain('shard 1'); expect(ciHealthSummary(plan, [results[0]].map(r => ({ ...r, outcome: { ...r.outcome, elapsedMs: 100_000 } })))).toEqual([]); }); + + test('the --ci-verify summary divides each shard\'s prediction by the plan shard\'s jobs', () => { + const outcome = (shard: number, elapsedMs: number): FreeShardOutcome => + ({ shard, files: [], status: 'passed', exitCode: 0, elapsedMs, groupPid: null, failingFiles: [], unattributedFailures: 0 }); + const plan = { version: 1, revision: 'r', id: 'p', shards: [ + { shard: 1, files: ['test/a.test.ts'], predictedMs: 400_000, jobs: 4 }, + { shard: 2, files: ['test/b.test.ts'], predictedMs: 400_000, jobs: 4 }, + ] } satisfies FreeCiPlan; + const results: FreeCiResult[] = [ + { planId: 'p', revision: 'r', outcome: outcome(1, 160_000), retry: null }, + { planId: 'p', revision: 'r', outcome: outcome(2, 120_000), retry: null }, + ]; + const summary = ciHealthSummary(plan, results); + expect(summary).toHaveLength(1); + expect(summary[0]).toContain('shard 1 took 160s vs 100s predicted across 4 parallel jobs'); + expect(summary[0]).not.toContain('shard 2'); + }); });