Keep Free Tests planning under its job cap when the duration seed goes stale (GST-2193) (#4245)

* Keep Free Tests planning under the job cap when the duration seed is stale (GST-2193)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Refresh the free-suite duration seed from run 42691699570 (GST-2193)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* Admit the eight test files the fresh seed records over 60 seconds as annotated exceptions (GST-2193)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-09 21:46:36 +00:00
1 parent dcebc44fa7
commit 72cbb49ddf
8 files changed
+1813 -1453

No files matched your search

+43 -3
View File
@@ -9,7 +9,9 @@ name: Free-suite duration seeds
# a seed PR. A file that fails while recording turns its job red but keeps its
# duration (a red file is still a real cost) and is named in the PR body; an
# interrupted recording, or more than 5 red files on either OS, proposes
# nothing (docs/TESTING_INTERNALS.md#free-suite-duration-seed).
# nothing (docs/TESTING_INTERNALS.md#free-suite-duration-seed). A seed PR
# closed without merging opens a blocker issue (the seed stays stale until the
# next recording lands), and a seed PR that merges closes it.
on:
schedule:
@@ -20,9 +22,15 @@ on:
description: Push the bot branch and open or refresh the seed PR (false records and reports only)
type: boolean
default: true
pull_request:
types: [closed]
branches: [main]
paths:
- scripts/free-test-durations.json
- scripts/free-test-durations-windows.json
concurrency:
group: free-durations-record
group: ${{ github.event_name == 'pull_request' && format('free-durations-seed-pr-{0}', github.event.pull_request.number) || 'free-durations-record' }}
cancel-in-progress: false
permissions:
@@ -30,6 +38,7 @@ permissions:
jobs:
linux:
if: github.event_name != 'pull_request'
runs-on: ubicloud-standard-8
timeout-minutes: 45
steps:
@@ -70,6 +79,7 @@ jobs:
if-no-files-found: error
windows:
if: github.event_name != 'pull_request'
uses: ./.github/workflows/windows-free-tests.yml
permissions:
contents: read
@@ -79,7 +89,7 @@ jobs:
propose:
needs: [linux, windows]
if: ${{ !cancelled() }}
if: ${{ !cancelled() && github.event_name != 'pull_request' }}
runs-on: ubuntu-24.04
timeout-minutes: 10
permissions:
@@ -174,3 +184,33 @@ jobs:
gh workflow run free-tests.yml --repo "$GITHUB_REPOSITORY" --ref "$branch"
gh workflow run windows-smoke.yml --repo "$GITHUB_REPOSITORY" --ref "$branch"
gh workflow run quality-gate.yml --repo "$GITHUB_REPOSITORY" --ref "$branch"
seed-pr-closed:
if: github.event_name == 'pull_request' && github.event.pull_request.head.ref == 'bot/free-duration-seeds' && github.event.pull_request.head.repo.full_name == github.repository
runs-on: ubuntu-24.04
timeout-minutes: 5
permissions:
issues: write
steps:
- name: Raise the stale-seed blocker when a seed PR closes unmerged, clear it when one merges
env:
GH_TOKEN: ${{ github.token }}
MERGED: ${{ github.event.pull_request.merged }}
PR_NUMBER: ${{ github.event.pull_request.number }}
run: |
set -euo pipefail
title="Blocker: free-suite duration seed PR closed unmerged"
existing=$(gh issue list --repo "$GITHUB_REPOSITORY" --state open --search "in:title \"$title\"" --json number --jq '.[0].number // empty')
if [ "$MERGED" = true ]; then
if [ -n "$existing" ]; then gh issue close "$existing" --repo "$GITHUB_REPOSITORY" --comment "Seed PR #$PR_NUMBER merged, so the seed is current again."; fi
exit 0
fi
cat > "$RUNNER_TEMP/blocker.md" <<EOF
Seed PR #$PR_NUMBER was closed without merging, so scripts/free-test-durations.json still lacks every free test file added since its last recording. Unseeded files are packed at the seed's 99th percentile, which unbalances shards and stretches every Free Tests job toward its 90-minute cap.
Fix: dispatch a fresh recording on main (\`gh workflow run free-durations-record.yml --ref main\`) and merge the seed PR it opens, or record with \`bun run test:ubicloud --record-durations\` and land the seed in a PR. This issue closes when a seed PR from the recorder merges; close it by hand if the seed lands another way. See docs/TESTING_INTERNALS.md#free-suite-duration-seed.
EOF
if [ -n "$existing" ]; then gh issue comment "$existing" --repo "$GITHUB_REPOSITORY" --body-file "$RUNNER_TEMP/blocker.md"
else gh issue create --repo "$GITHUB_REPOSITORY" --title "$title" --body-file "$RUNNER_TEMP/blocker.md"; fi
echo "::error::Seed PR #$PR_NUMBER closed unmerged; the free-suite duration seed stays stale until a recording lands."
exit 1
+4
View File
@@ -0,0 +1,4 @@
---
important: blocker
---
- **Free Tests no longer fails every PR's plan when the test-duration seed goes stale.** Test files missing from `scripts/free-test-durations.json` are still packed at the seed's 99th percentile, but when that estimate would push a CI shard's job past the 90-minute cap, `--ci-plan --job-setup-seconds` now cuts the shard's prediction back to the cap instead of throwing `FREE_JOB_TIMEOUT_OVER_CAP`. Recorded durations are never cut, so a shard whose seeded files alone need more than 90 minutes still fails the plan, and plans under the cap are unchanged. The seed is refreshed from a new recording (196 files had no recorded time), and a weekly seed PR that is closed without merging now opens a blocker issue, which closes when a seed PR from the recorder merges.
File diff suppressed because it is too large. Load diff
+9 -1
View File
@@ -4,6 +4,14 @@
{"file": "test/shared-libs-stage-actor.test.ts", "reason": "re-runs the paid shared-libs runner body against git fixture repos per test; owned by the paid-harness value-bar review"},
{"file": "test/setup-install-registry.test.ts", "reason": "82s in the 2026-10-15 recording (58s on 2026-10-04): 11 cases each run the real ./setup, gstack-relink, gstack-config and gstack-uninstall against throwaway checkouts", "todo": "split by describe block so the packer can balance it, then rerun bun run test:ubicloud --record-durations (TODOS.md, fix wave 2 follow-ups)"},
{"file": "test/brain-sync.test.ts", "reason": "89s in the 2026-10-19 weekly recording (58s on 2026-10-15): #3254 added the case-collision suite, and every case drains a real gstack-brain-sync against a local bare remote", "todo": "split by describe block so the packer can balance it, then rerun bun run test:ubicloud --record-durations (TODOS.md, fix wave 2 follow-ups)"},
{"file": "test/setup-codex-scope.test.ts", "reason": "64s in the 2026-10-19 weekly recording (59s on 2026-10-15): each case runs the real ./setup twice against a Codex install fixture", "todo": "split by describe block so the packer can balance it, then rerun bun run test:ubicloud --record-durations (TODOS.md, fix wave 2 follow-ups)"}
{"file": "test/setup-codex-scope.test.ts", "reason": "64s in the 2026-10-19 weekly recording (59s on 2026-10-15): each case runs the real ./setup twice against a Codex install fixture", "todo": "split by describe block so the packer can balance it, then rerun bun run test:ubicloud --record-durations (TODOS.md, fix wave 2 follow-ups)"},
{"file": "test/review-pass.test.ts", "reason": "221s in the 2026-11-09 recording (run 42691699570; 24s on 2026-10-31); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"},
{"file": "test/review-exclude.test.ts", "reason": "126s in the 2026-11-09 recording (run 42691699570; unseeded before); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"},
{"file": "test/ubicloud-runner-recovery.test.ts", "reason": "79s in the 2026-11-09 recording (run 42691699570; unseeded before); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"},
{"file": "test/ship-rereview-triggers.test.ts", "reason": "78s in the 2026-11-09 recording (run 42691699570; unseeded before); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"},
{"file": "crew/test/ship-queue-train-generic.test.ts", "reason": "77s in the 2026-11-09 recording (run 42691699570; unseeded before); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"},
{"file": "test/ship-finish-stack-parent.test.ts", "reason": "68s in the 2026-11-09 recording (run 42691699570; unseeded before); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"},
{"file": "test/ship-ci-carry.test.ts", "reason": "67s in the 2026-11-09 recording (run 42691699570; unseeded before); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"},
{"file": "test/ship-queue-land.test.ts", "reason": "60s in the 2026-11-09 recording (run 42691699570; 59s on 2026-10-31); admitted so the refreshed seed can land", "todo": "split or speed up; post-11-21 slow-test pass"}
]
}
+22 -4
View File
@@ -460,16 +460,34 @@ const runsSerialUnderJobs = (file: string) => file in TREE_MUTATING || file in S
const planDigest = (plan: Omit<FreeCiPlan, 'id'>): string =>
createHash('sha256').update(JSON.stringify(plan)).digest('hex');
/** One immutable plan is shared by isolated CI machines; never repack per job. */
export function createFreeCiPlan(files: string[], count: number, durations: Record<string, number>, revision: string, shardJobs = 1): FreeCiPlan {
/**
* The largest packed prediction whose CI job still fits MAX_CI_JOB_TIMEOUT_MINUTES
* (inverse of ciShardJobTimeoutMinutes's prediction term).
*/
export function maxPredictedMsUnderJobCap(setupSeconds: number): number {
return Math.floor(((MAX_CI_JOB_TIMEOUT_MINUTES * 60 - setupSeconds) * 1000 - wallTimeoutForShard(FLAKY_RETRY_CAP)) / 3);
}
/**
* One immutable plan is shared by isolated CI machines; never repack per job.
* With setupSeconds (--ci-plan --job-setup-seconds), a shard whose 99th-percentile
* estimates for unseeded files would push its job past the cap has its prediction
* cut back to the cap (GST-2193): a stale seed lengthens the job up to the cap and
* never fails the plan. Recorded durations are never cut, so a shard whose seeded
* files alone pass the cap still fails it.
*/
export function createFreeCiPlan(files: string[], count: number, durations: Record<string, number>, revision: string, shardJobs = 1, setupSeconds?: number): FreeCiPlan {
const readers = files.filter(file => !(file in TREE_MUTATING));
const exclusive = files.filter(file => file in TREE_MUTATING).sort();
const packed = packShardsByDuration(readers, count, durations);
const planned = packed.shards.map((files, index) => ({ files, predictedMs: packed.predictedMs[index] }));
if (exclusive.length) planned.push({ files: exclusive, predictedMs: exclusive.reduce((ms, file) => ms + (durations[file] ?? 0), 0) });
const limitMs = setupSeconds === undefined ? Infinity : maxPredictedMsUnderJobCap(setupSeconds);
const seededMs = (shardFiles: string[]) => shardFiles.reduce((ms, file) => ms + (durations[normalizeRelativePath(file)] ?? 0), 0);
const shards = planned.map(({ files, predictedMs }, index) => {
const jobs = plannedShardJobs(files, shardJobs, runsSerialUnderJobs);
return { shard: index + 1, files, predictedMs, ...(jobs ? { jobs } : {}) };
const capped = predictedMs > limitMs && seededMs(files) <= limitMs ? limitMs : predictedMs;
return { shard: index + 1, files, predictedMs: capped, ...(jobs ? { jobs } : {}) };
});
const body = { version: 1 as const, revision, shards };
return { ...body, id: planDigest(body) };
@@ -2036,7 +2054,7 @@ async function main(): Promise<number> {
appendStepSummary(line);
}
warnUnseededFreeFiles(ciFiles, durations);
const plan = createFreeCiPlan(ciFiles, options.shardCount, durations, revision, options.shardJobs);
const plan = createFreeCiPlan(ciFiles, options.shardCount, durations, revision, options.shardJobs, options.jobSetupSeconds);
validateFreeCiPlan(plan, ciFiles, revision);
const matrix = ciPlanMatrix(plan, options.jobSetupSeconds);
if ('include' in matrix) {
+41 -3
View File
@@ -20,7 +20,8 @@ afterEach(() => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: tru
test('the recorder runs weekly before test-health and on dispatch, read-only except the propose job', () => {
expect(recorder.on.schedule).toEqual([{ cron: '0 4 * * 1' }]);
expect(Object.keys(recorder.on).sort()).toEqual(['schedule', 'workflow_dispatch']);
expect(Object.keys(recorder.on).sort()).toEqual(['pull_request', 'schedule', 'workflow_dispatch']);
for (const job of ['linux', 'windows']) expect(recorder.jobs[job].if).toBe("github.event_name != 'pull_request'");
expect(Object.keys(recorder.on.workflow_dispatch.inputs)).toEqual(['open_pr']);
expect(load('test-health.yml').on.schedule[0].cron).toBe('0 10 * * 1');
expect(recorder.permissions).toEqual({ contents: 'read' });
@@ -46,9 +47,9 @@ test('Linux records on the free-suite runner image, turns red on any red file, a
});
test('Windows records through the windows-free-tests record_durations path, and propose needs both complete recordings', () => {
expect(recorder.jobs.windows).toEqual({ uses: './.github/workflows/windows-free-tests.yml', permissions: { contents: 'read', actions: 'read' }, with: { record_durations: true } });
expect(recorder.jobs.windows).toEqual({ if: "github.event_name != 'pull_request'", uses: './.github/workflows/windows-free-tests.yml', permissions: { contents: 'read', actions: 'read' }, with: { record_durations: true } });
expect(recorder.jobs.propose.needs).toEqual(['linux', 'windows']);
expect(recorder.jobs.propose.if).toBe('${{ !cancelled() }}');
expect(recorder.jobs.propose.if).toBe("${{ !cancelled() && github.event_name != 'pull_request' }}");
const downloads = recorder.jobs.propose.steps.filter((step: any) => step.uses?.startsWith('actions/download-artifact@'));
expect(downloads.map((step: any) => step.with.name)).toEqual(['free-test-durations', 'free-test-durations-windows']);
expect(downloads.every((step: any) => step['continue-on-error'] === undefined)).toBe(true);
@@ -180,3 +181,40 @@ test('the drift check counts added and removed files, so an unchanged file set p
expect(drifted.status, drifted.stderr).toBe(0);
expect(drifted.stdout.trim()).toBe('"2 1"');
});
test('a seed PR closed unmerged raises one blocker issue and a red run; a merged seed PR clears it (GST-2193)', () => {
expect(recorder.on.pull_request).toEqual({ types: ['closed'], branches: ['main'],
paths: ['scripts/free-test-durations.json', 'scripts/free-test-durations-windows.json'] });
expect(recorder.concurrency.group).toContain("format('free-durations-seed-pr-{0}', github.event.pull_request.number)");
const job = recorder.jobs['seed-pr-closed'];
expect(job.if).toBe("github.event_name == 'pull_request' && github.event.pull_request.head.ref == 'bot/free-duration-seeds' && github.event.pull_request.head.repo.full_name == github.repository");
expect(job.permissions).toEqual({ issues: 'write' });
expect(job.steps.some((step: any) => step.uses?.startsWith('actions/checkout@'))).toBe(false);
const step = job.steps[0];
const dir = mkdtempSync(path.join(os.tmpdir(), 'seed-blocker-'));
dirs.push(dir);
const run = (merged: boolean, existing: string) => {
const log = path.join(dir, `gh-${merged}-${existing || 'none'}.log`);
writeFileSync(path.join(dir, 'gh'), `#!/bin/sh\necho "$*" >> "${log}"\nif [ "$1 $2" = "issue list" ]; then echo "${existing}"; fi\n`, { mode: 0o755 });
const result = spawnSync('bash', ['-c', step.run], { encoding: 'utf8', timeout: 10_000,
env: { ...process.env, PATH: `${dir}:${process.env.PATH}`, RUNNER_TEMP: dir, GITHUB_REPOSITORY: 'garrytan/gstack', MERGED: String(merged), PR_NUMBER: '3835' } });
const calls = (() => { try { return readFileSync(log, 'utf8').trim().split('\n'); } catch { return []; } })();
return { status: result.status, stdout: result.stdout, calls };
};
const raised = run(false, '');
expect(raised.status).toBe(1);
expect(raised.stdout).toContain('::error::Seed PR #3835 closed unmerged');
expect(raised.calls[1]).toBe(`issue create --repo garrytan/gstack --title Blocker: free-suite duration seed PR closed unmerged --body-file ${dir}/blocker.md`);
const body = readFileSync(path.join(dir, 'blocker.md'), 'utf8');
expect(body).toContain('Seed PR #3835 was closed without merging');
expect(body).toContain('`gh workflow run free-durations-record.yml --ref main`');
const again = run(false, '41');
expect(again.status).toBe(1);
expect(again.calls[1]).toBe(`issue comment 41 --repo garrytan/gstack --body-file ${dir}/blocker.md`);
const cleared = run(true, '41');
expect(cleared.status).toBe(0);
expect(cleared.calls[1]).toBe('issue close 41 --repo garrytan/gstack --comment Seed PR #3835 merged, so the seed is current again.');
const quiet = run(true, '');
expect(quiet.status).toBe(0);
expect(quiet.calls).toHaveLength(1);
});
+1 -1
View File
@@ -331,7 +331,7 @@ describe('free-tests workflow wiring', () => {
const setupSeconds = Number(workflow.env.FREE_SUITE_SETUP_SECONDS);
expect(Number.isInteger(setupSeconds) && setupSeconds > 0).toBe(true);
const plan = createFreeCiPlan(collectFreeTestFiles(), Number(workflow.env.FREE_SUITE_SHARDS),
loadFreeTestDurations() ?? {}, 'wiring', Number(workflow.env.FREE_SUITE_JOBS));
loadFreeTestDurations() ?? {}, 'wiring', Number(workflow.env.FREE_SUITE_JOBS), setupSeconds);
const declared = workflow.jobs['free-suite']['timeout-minutes'];
const emitted = ciPlanMatrix(plan, setupSeconds);
if (!('include' in emitted)) throw new Error('the CI matrix carries no per-shard timeouts');
+56
View File
@@ -19,6 +19,7 @@ import {
DEFAULT_WALL_TIMEOUT_MS,
PER_FILE_WALL_MS,
wallTimeoutForShard,
FLAKY_RETRY_CAP,
recordingWallTimeoutMs,
parseCliOptions,
KNOWN_WINDOWS_INCOMPATIBLE,
@@ -1123,6 +1124,61 @@ describe('test-free-shards: duration-aware packing (full-suite LPT)', () => {
expect(ciPlanMatrix(plan, 300)).toEqual({ include: [{ shard: 1, timeout_minutes: 49 }, { shard: 2, timeout_minutes: 17 }] });
});
test('unseeded files never push a shard past the job cap; their packed share is cut to fit (GST-2193)', () => {
// A stale seed packs every unseeded file at the seed's 99th percentile. On
// 2026-11-09 196 unseeded files put each shard at 77 of the 90-minute cap,
// and about 60 more would have failed every PR's plan. Recorded durations
// keep their full weight; only the unseeded estimate gives way.
const durations: Record<string, number> = { 'test/slow.test.ts': 1_200_000 };
for (let i = 0; i < 99; i += 1) durations[`test/s${i}.test.ts`] = 1_000;
const files = [...Object.keys(durations), 'test/new-a.test.ts', 'test/new-b.test.ts'];
expect(() => ciPlanMatrix(createFreeCiPlan(files, 1, durations, 'rev'), 300)).toThrow(/FREE_JOB_TIMEOUT_OVER_CAP/);
const plan = createFreeCiPlan(files, 1, durations, 'rev', 1, 300);
validateFreeCiPlan(plan, files, 'rev');
expect(plan.shards[0].files).toEqual([...files].sort());
expect(ciPlanMatrix(plan, 300)).toEqual({ include: [{ shard: 1, timeout_minutes: MAX_CI_JOB_TIMEOUT_MINUTES }] });
const neededMs = wallTimeoutForPackedShard(plan.shards[0].predictedMs, WALL_BASE_MS, files.length) + wallTimeoutForShard(FLAKY_RETRY_CAP) + 300_000;
expect(neededMs).toBeLessThanOrEqual(MAX_CI_JOB_TIMEOUT_MINUTES * 60_000);
expect(plan.shards[0].predictedMs).toBeGreaterThanOrEqual(1_299_000);
});
test('a plan under the job cap is unchanged by the unseeded cap (GST-2193 hold)', () => {
const seeded = { 'test/a.test.ts': 750_000, 'test/b.test.ts': 1_000 };
const plain = createFreeCiPlan(Object.keys(seeded), 2, seeded, 'rev');
expect(createFreeCiPlan(Object.keys(seeded), 2, seeded, 'rev', 1, 300)).toEqual(plain);
expect(ciPlanMatrix(plain, 300)).toEqual({ include: [{ shard: 1, timeout_minutes: 49 }, { shard: 2, timeout_minutes: 17 }] });
const partly = { 'test/a.test.ts': 600_000 };
const files = ['test/a.test.ts', 'test/b.test.ts'];
const plainPartly = createFreeCiPlan(files, 1, partly, 'rev');
expect(plainPartly.shards[0].predictedMs).toBe(1_200_000);
expect(createFreeCiPlan(files, 1, partly, 'rev', 1, 300)).toEqual(plainPartly);
expect(ciPlanMatrix(plainPartly, 300)).toEqual({ include: [{ shard: 1, timeout_minutes: 71 }] });
});
test('recorded durations over the job cap still fail the plan; only the unseeded share is cut (GST-2193 hold)', () => {
const durations = { 'test/slow.test.ts': 1_700_000, 'test/b.test.ts': 99_000 };
const plan = createFreeCiPlan([...Object.keys(durations), 'test/new.test.ts'], 1, durations, 'rev', 1, 300);
expect(plan.shards[0].predictedMs).toBe(1_799_000 + 1_700_000);
expect(() => ciPlanMatrix(plan, 300)).toThrow(/FREE_JOB_TIMEOUT_OVER_CAP: CI shard 1/);
});
test('--ci-plan with a stale seed keeps every job under the cap instead of failing the plan (GST-2193)', () => {
const committed = loadFreeTestDurations() ?? {};
const kept = Object.keys(committed).sort().filter((_, index) => index % 4 === 0);
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'free-seed-stale-'));
try {
const seedPath = path.join(dir, 'seed.json');
fs.writeFileSync(seedPath, JSON.stringify({ version: 1, durations: Object.fromEntries(kept.map(file => [file, committed[file]])) }));
const planned = Bun.spawnSync([process.execPath, path.join(ROOT, 'scripts/test-free-shards.ts'), '--ci-plan', path.join(dir, 'plan.json'),
'--shards', '10', '--shard-jobs', '4', '--job-setup-seconds', '300'], { env: { ...process.env, GSTACK_FREE_TEST_DURATIONS: seedPath }, timeout: 10_000 });
expect(planned.exitCode, planned.stderr.toString()).toBe(0);
const matrix = JSON.parse(planned.stdout.toString()) as { include: Array<{ timeout_minutes: number }> };
expect(matrix.include.length).toBeGreaterThan(0);
expect(matrix.include.every(entry => entry.timeout_minutes <= MAX_CI_JOB_TIMEOUT_MINUTES)).toBe(true);
expect(planned.stderr.toString()).toMatch(/WARNING: \d+ free test files have no recorded duration/);
} finally { fs.rmSync(dir, { recursive: true, force: true }); }
});
test('--job-setup-seconds is a --ci-plan flag with whole seconds from 1 to 3600', () => {
expect(parseCliOptions(['--ci-plan', 'p.json', '--job-setup-seconds', '300']).jobSetupSeconds).toBe(300);
expect(() => parseCliOptions(['--shards', '2', '--job-setup-seconds', '300'])).toThrow('--job-setup-seconds applies only to --ci-plan');