diff --git a/scripts/lib/shard-jobs.ts b/scripts/lib/shard-jobs.ts index b98db15a8..68789f387 100644 --- a/scripts/lib/shard-jobs.ts +++ b/scripts/lib/shard-jobs.ts @@ -26,6 +26,7 @@ * (the console stream is byte-identical with or without it). Counts are * evidence for k > 1 comparisons; they never decide a verdict. */ +import * as fs from 'node:fs'; import * as os from 'node:os'; import type { FreeCiPlan, FreeCiResult, FreeShardOutcome } from '../test-free-shards'; import { guardFreeHome, sharedFreeHome, type FreeHomeGuard, type FreeHomeGuardFactory } from './free-home-guard'; @@ -107,7 +108,8 @@ export function parseJunitCounts(xml: string | null, planned: readonly string[], const canonical = (printed: string) => { const stripped = normalizeRelativePath(printed).replace(/^(?:\.{1,2}\/)+/, ''); if (plannedSet.has(stripped)) return stripped; - return planned.find(file => stripped.endsWith(`/${file}`) || file.endsWith(`/${stripped}`)) ?? stripped; + const matches = planned.filter(file => stripped.endsWith(`/${file}`) || file.endsWith(`/${stripped}`)); + return matches.length === 1 ? matches[0] : stripped; }; let current: { file: string; state: 'pass' | 'fail' | 'skip' | 'todo'; expects: number } | null = null; const finish = () => { @@ -139,6 +141,18 @@ export function parseJunitCounts(xml: string | null, planned: readonly string[], return result(true); } +const JUNIT_MAX_BYTES = 64 * 1024 * 1024; + +/** The child-written report, only when it is a regular file of bounded size (never a link, FIFO or device). */ +export function readJunitReport(file: string): string | null { + let fd: number; + try { fd = fs.openSync(file, fs.constants.O_RDONLY | (fs.constants.O_NOFOLLOW ?? 0) | (fs.constants.O_NONBLOCK ?? 0)); } catch { return null; } + try { + const stat = fs.fstatSync(fd); + return stat.isFile() && stat.size <= JUNIT_MAX_BYTES ? fs.readFileSync(fd, 'utf8') : null; + } catch { return null; } finally { fs.closeSync(fd); } +} + /** One shard's counts from its sub-shards' counts; complete only when every part is and no file repeats. */ export function mergeTestCounts(parts: Array): ShardTestCounts | undefined { if (parts.every(part => !part)) return undefined; @@ -199,7 +213,9 @@ export function subShardProblems(settled: SubShardSettlement): string[] { const problems: string[] = []; if (JSON.stringify(outcome.files) !== JSON.stringify(assignment.files)) problems.push(`${assignment.id} reported another file set`); const summary = outcome.summary; - if (outcome.status === 'passed' && (!summary || summary.filesRan !== assignment.files.length || summary.sawTerminalSummary !== true + if (!summary || (summary.sawTerminalSummary && summary.filesRan !== assignment.files.length)) { + problems.push(`${assignment.id} reported ${summary?.filesRan ?? 'no'} executed file(s) for ${assignment.files.length} assigned`); + } else if (outcome.status === 'passed' && (summary.filesRan !== assignment.files.length || summary.sawTerminalSummary !== true || outcome.exitCode !== 0 || outcome.failingFiles.length || outcome.unattributedFailures)) { problems.push(`${assignment.id} passed without a complete execution summary`); } @@ -275,8 +291,15 @@ export function subShardRecordProblems(result: FreeCiResult): string[] { const problems: string[] = []; if (new Set(files).size !== files.length || files.length !== result.outcome.files.length || !result.outcome.files.every(file => files.includes(file))) problems.push('sub-shard records do not cover the shard exactly once'); - if (result.outcome.status === 'passed' && records.some(record => record.status !== 'passed' || record.summary?.filesRan !== record.files.length - || (record.files.length > 0 && record.summary?.sawTerminalSummary !== true))) problems.push('a passing shard holds a failed or incomplete sub-shard'); + const prefix = `${result.planId.slice(0, 12)}:${result.outcome.shard}.`; + if (records.some((record, index) => record.id !== `${prefix}${index + 1}`) + || records.filter(record => record.phase === 'parallel').length > result.jobs! + || records.some((record, index) => record.phase === 'serial' ? index !== records.length - 1 : record.phase !== 'parallel')) { + problems.push('sub-shard ids or phases do not match the plan and --jobs'); + } + if (records.some(record => record.status === 'missing' || record.summary?.filesRan !== record.files.length + || (record.files.length > 0 && record.summary?.sawTerminalSummary !== true))) problems.push('a sub-shard did not run every assigned file'); + if (result.outcome.status === 'passed' && records.some(record => record.status !== 'passed')) problems.push('a passing shard holds a failed sub-shard'); return problems; } @@ -297,12 +320,15 @@ export function subShardTimeoutText( ): string[] { const suspects = inFlight.filter(file => record.files.includes(file)); const files = suspects.length ? suspects : record.files; + const fix = record.phase === 'serial' + ? 'this file already runs alone after the parallel sub-shards; fix or split it, never raise the timeout' + : `if it passes serially, add it to SLOW_UNDER_LOAD in scripts/lib/shard-jobs.ts with run id ${context.runId}; never raise the timeout`; return [ `[test:free] ${FREE_SUBSHARD_TIMEOUT}: CI shard ${shard} sub-shard ${record.id} (${record.files.length} files) hit its ${Math.round(wallTimeoutMs / 1000)}s wall at --jobs ${jobs} on ${context.runner}, run ${context.runId}.`, `[test:free] in flight: ${suspects.length ? suspects.join(', ') : 'unknown (no open file chunk; see the sub-shard log)'}`, `[test:free] reproduce serially: printf '%s\\n' ${files.map(quote).join(' ')} > files.txt && GSTACK_FREE_JOBS=1 bun run scripts/test-free-shards.ts --files-from files.txt`, `[test:free] or the whole CI shard at k=1: bun run scripts/test-free-shards.ts --ci-run ${quote(context.planPath)} --shard ${shard} --result shard-${shard}.json --jobs 1`, - `[test:free] if it passes serially, add it to SLOW_UNDER_LOAD in scripts/lib/shard-jobs.ts with run id ${context.runId}; never raise the timeout. See ${DOCS}`, + `[test:free] ${fix}. See ${DOCS}`, ]; } @@ -344,9 +370,12 @@ export async function runCiShard(input: CiShardInput, deps: CiShardDeps): Promis const total = assignments.length; log(`[test:free] CI shard ${shard.shard}/${plan.shards.length} at --jobs ${jobs}: ${assignments .map(entry => `sub-shard ${entry.id} = runner shard ${entry.index}/${total} (${entry.phase}, ${entry.files.length} files)`).join('; ')}`); - const run = (entry: SubShardAssignment, homeGuard?: FreeHomeGuardFactory, wall = wallTimeoutMs): Promise => - deps.runShard(entry.files, entry.index, total, { wallTimeoutMs: wall, verbose, testCounts: true, ...(homeGuard ? { homeGuard } : {}) }) + const walls = new Map(); + const run = (entry: SubShardAssignment, homeGuard?: FreeHomeGuardFactory, wall = wallTimeoutMs): Promise => { + walls.set(entry.id, wall); + return deps.runShard(entry.files, entry.index, total, { wallTimeoutMs: wall, verbose, testCounts: true, ...(homeGuard ? { homeGuard } : {}) }) .then(outcome => ({ assignment: entry, outcome }), (error: unknown) => ({ assignment: entry, error: error instanceof Error ? error.message : String(error) })); + }; const parallel = assignments.filter(entry => entry.phase === 'parallel'); const serial = assignments.filter(entry => entry.phase === 'serial'); const phaseProblems: string[] = []; @@ -359,13 +388,14 @@ export async function runCiShard(input: CiShardInput, deps: CiShardDeps): Promis if (homeChange) phaseProblems.push(homeChange); } for (const entry of serial) { + const left = startedAt + wallTimeoutMs - Date.now(); settled.push(deps.terminationRequested() ? { assignment: entry, error: 'cancelled before the serial phase' } - : await run(entry, undefined, Math.max(1, startedAt + wallTimeoutMs - Date.now()))); + : left <= 0 ? { assignment: entry, error: 'no wall left after the parallel phase' } : await run(entry, undefined, left)); } const merged = mergeSubShardOutcomes(shard, settled, Date.now() - startedAt, phaseProblems); for (const problem of merged.problems) console.error(`[test:free] ${FREE_SUBSHARD_REJECTED}: CI shard ${shard.shard}: ${problem}`); for (const record of merged.records.filter(entry => entry.status === 'timed-out')) { - for (const line of subShardTimeoutText(shard.shard, jobs, record, merged.outcome.inFlight ?? [], wallTimeoutMs, input.context)) console.error(line); + for (const line of subShardTimeoutText(shard.shard, jobs, record, merged.outcome.inFlight ?? [], walls.get(record.id) ?? wallTimeoutMs, input.context)) console.error(line); } log(`[test:free] CI shard ${shard.shard}: ${merged.outcome.status} across ${total} sub-shards in ${Math.round(merged.outcome.elapsedMs / 1000)}s`); const retried = await deps.retry([merged.outcome]); @@ -376,8 +406,8 @@ export async function runCiShard(input: CiShardInput, deps: CiShardDeps): Promis } /** The duration seed takes only one-file children's walls, never a sub-shard's or a shard's. */ -export function assertSoloSeedSample(file: string, outcome: Pick): void { - if (outcome.files.length !== 1 || outcome.files[0] !== file) { +export function assertSoloSeedSample(file: string, outcome: Pick): void { + if (outcome.files.length !== 1 || outcome.files[0] !== file || (outcome.summary?.filesRan ?? 0) > 1) { throw new Error(`Duration seed sample for ${file} came from a ${outcome.files.length}-file child; seeds are recorded only from one-file serial children.`); } } diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 5f94189ef..0c34ec93d 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -97,7 +97,7 @@ import { attributeFreeHomeWriters, guardFreeHome, sharedFreeHome, type FreeHomeG import { appendStepSummary, ciHealthSummary, SEED_REFRESH_COMMAND, unseededWarning, windowsCurationLine } from './lib/free-ci-health'; import { curateWindowsSafe, type CurationResult } from './lib/windows-curation'; import { - assertSoloSeedSample, ciRunContext, parseJobsFlag, parseJunitCounts, rejectJobsWithoutCiRun, runCiShard, SLOW_UNDER_LOAD, subShardRecordProblems, + assertSoloSeedSample, ciRunContext, parseJobsFlag, parseJunitCounts, readJunitReport, rejectJobsWithoutCiRun, runCiShard, SLOW_UNDER_LOAD, subShardRecordProblems, validateJobsForShard, type ShardTestCounts, type SubShardRecord, } from './lib/shard-jobs'; export { curateWindowsSafe, detectWindowsFragility, KNOWN_WINDOWS_INCOMPATIBLE, type CurationResult } from './lib/windows-curation'; @@ -1730,9 +1730,7 @@ export async function runFreeShard( shardLog.write(diagnostic + '\n'); } await new Promise((resolve) => shardLog.stream.end(() => resolve())); - if (junitPath) { - try { junitXml = fs.readFileSync(junitPath, 'utf8'); } catch { /* counts record the missing report */ } - } + if (junitPath) junitXml = readJunitReport(junitPath); try { if (!cleanupError) fs.rmSync(stateDir, { recursive: true, force: true }); } catch { diff --git a/test/shard-jobs.test.ts b/test/shard-jobs.test.ts index 96988d3c0..34121e316 100644 --- a/test/shard-jobs.test.ts +++ b/test/shard-jobs.test.ts @@ -15,7 +15,7 @@ import { } from '../scripts/test-free-shards'; import { assertSoloSeedSample, assignSubShards, ciRunContext, FREE_JOBS_INVALID, mergeSubShardOutcomes, parseJobsFlag, parseJunitCounts, - runCiShard, SLOW_UNDER_LOAD, subShardTimeoutText, validateJobsForShard, type CiShardDeps, type RunShardOptions, + readJunitReport, runCiShard, SLOW_UNDER_LOAD, subShardRecordProblems, subShardTimeoutText, validateJobsForShard, type CiShardDeps, type RunShardOptions, } from '../scripts/lib/shard-jobs'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -199,6 +199,36 @@ describe('k > 1: merge, rejection and one retry pass per CI shard', () => { } finally { errors.mockRestore(); } }); + test('a failed sub-shard that ran too few or too many files is rejected, so counts cannot cancel and no retry repairs it', async () => { + const errors = spyOn(console, 'error').mockImplementation(() => {}); + try { + const h = harness(files.slice(0, 4), runFiles => ({ ...failed(runFiles, [runFiles[0]]), + summary: { testsRan: 2, filesRan: runFiles.includes(files[0]) ? 1 : 3, sawTerminalSummary: true } })); + const { result } = await h.run(2); + expect(result.outcome.summary!.filesRan).toBe(4); + expect(result.outcome.unattributedFailures).toBeGreaterThanOrEqual(2); + expect(eligibleFreeRetryFiles([result.outcome])).toBeNull(); + const forged: FreeCiResult = { ...result, outcome: { ...result.outcome, unattributedFailures: 0 }, + retry: { ...passed([files[0], files[1]]), shard: 2 } }; + expect(() => verifyFreeCiResults({ ...h.plan, revision: 'revision-a' } as never, [forged])).toThrow(/did not run every assigned file|identity/); + expect(subShardRecordProblems(forged)).toContain('a sub-shard did not run every assigned file'); + } finally { errors.mockRestore(); } + }); + + test('the serial sub-shard gets only what is left of the wall, and none left means it is not launched', async () => { + const errors = spyOn(console, 'error').mockImplementation(() => {}); + try { + const h = harness([...files, EXCLUSIVE], async runFiles => { + if (!runFiles.includes(EXCLUSIVE)) await new Promise(resolve => setTimeout(resolve, 60)); + return passed(runFiles); + }); + const { result } = await runCiShard({ plan: h.plan, shard: h.shard, revision: 'revision-a', jobs: 2, wallTimeoutMs: 50, verbose: false, context }, h.deps); + expect(h.calls.some(call => call.files.includes(EXCLUSIVE))).toBe(false); + expect(result.subShards!.at(-1)).toMatchObject({ status: 'missing', phase: 'serial' }); + expect(result.outcome.status).toBe('failed'); + } finally { errors.mockRestore(); } + }); + test('a real-home change in the concurrent phase and a cancelled serial phase both fail the shard', async () => { const errors = spyOn(console, 'error').mockImplementation(() => {}); try { @@ -239,12 +269,19 @@ describe('k > 1: merge, rejection and one retry pass per CI shard', () => { const merged = mergeSubShardOutcomes(shard, assignments.map(assignment => ({ assignment, outcome: passed(assignment.files) })), 10); const result: FreeCiResult = { planId: plan.id, revision: 'revision-a', outcome: merged.outcome, retry: null, jobs: 2, subShards: merged.records }; expect(() => verifyFreeCiResults(plan, [result])).not.toThrow(); + const renamed = structuredClone(result); + renamed.subShards![0].id = 'other:1.1'; + expect(() => verifyFreeCiResults(plan, [renamed])).toThrow(/ids or phases/); + const tooMany = structuredClone(result); + tooMany.jobs = 1; + expect(subShardRecordProblems({ ...tooMany, jobs: 2, subShards: [...tooMany.subShards!, { ...tooMany.subShards![0], id: `${plan.id.slice(0, 12)}:1.3`, files: [] }] })) + .toContain('sub-shard ids or phases do not match the plan and --jobs'); const overlap = structuredClone(result); overlap.subShards![1].files = [...overlap.subShards![1].files, overlap.subShards![0].files[0]]; expect(() => verifyFreeCiResults(plan, [overlap])).toThrow(/sub-shards rejected/); const hidden = structuredClone(result); hidden.subShards![0].status = 'failed'; - expect(() => verifyFreeCiResults(plan, [hidden])).toThrow(/failed or incomplete sub-shard/); + expect(() => verifyFreeCiResults(plan, [hidden])).toThrow(/a passing shard holds a failed sub-shard/); expect(() => verifyFreeCiResults(plan, [{ ...result, jobs: 1 }])).toThrow(/valid --jobs/); }); }); @@ -268,6 +305,27 @@ describe('per-file test and expect() counts from bun JUnit', () => { totals: { tests: 4, pass: 1, fail: 1, skip: 1, todo: 1, expects: 9 } }); }); + test('an ambiguous short path is never credited to one of two planned files that share it', () => { + const counts = parseJunitCounts(xml('a.test.ts'), ['test/a.test.ts', 'browse/test/a.test.ts'], 4); + expect(counts.complete).toBe(false); + expect(counts.problem).toContain('unplanned'); + }); + + test('the report is read only as a bounded regular file, never through a link or a FIFO', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-junit-')); + try { + fs.writeFileSync(path.join(dir, 'real.xml'), ''); + expect(readJunitReport(path.join(dir, 'real.xml'))).toBe(''); + expect(readJunitReport(path.join(dir, 'absent.xml'))).toBeNull(); + if (process.platform !== 'win32') { + fs.symlinkSync(path.join(dir, 'real.xml'), path.join(dir, 'link.xml')); + expect(readJunitReport(path.join(dir, 'link.xml'))).toBeNull(); + const fifo = Bun.spawnSync(['mkfifo', path.join(dir, 'fifo.xml')], { timeout: 5_000 }); + if (fifo.exitCode === 0) expect(readJunitReport(path.join(dir, 'fifo.xml'))).toBeNull(); + } + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); + test('a missing report, an unplanned file or a count that disagrees with bun summary is incomplete, never silently complete', () => { expect(parseJunitCounts(null, ['test/a.test.ts'], 4)).toMatchObject({ complete: false, problem: 'bun wrote no JUnit report' }); expect(parseJunitCounts(xml('test/z.test.ts'), ['test/a.test.ts'], 4)).toMatchObject({ complete: false, problem: expect.stringContaining('unplanned') }); @@ -376,6 +434,7 @@ describe('serial lists and the duration seed', () => { test('seeds take only one-file children; sub-shard runs have no path to the seed writer', () => { expect(() => assertSoloSeedSample('test/a.test.ts', { files: ['test/a.test.ts'] })).not.toThrow(); expect(() => assertSoloSeedSample('test/a.test.ts', { files: ['test/a.test.ts', 'test/b.test.ts'] })).toThrow(/one-file serial children/); + expect(() => assertSoloSeedSample('test/a.test.ts', { files: ['test/a.test.ts'], summary: { testsRan: 9, filesRan: 2, sawTerminalSummary: true } })).toThrow(/one-file serial children/); expect(fs.readFileSync(path.join(ROOT, 'scripts/lib/shard-jobs.ts'), 'utf8')).not.toContain('writeDurationSeed'); const runner = fs.readFileSync(path.join(ROOT, 'scripts/test-free-shards.ts'), 'utf8'); const recorder = runner.slice(runner.indexOf('async function recordFreeTestDurations('), runner.indexOf('async function retryFailedFreeFiles('));