diff --git a/scripts/lib/shard-jobs.ts b/scripts/lib/shard-jobs.ts index 68789f387..c96100e01 100644 --- a/scripts/lib/shard-jobs.ts +++ b/scripts/lib/shard-jobs.ts @@ -94,10 +94,12 @@ const attributes = (text: string): Record => * Per-file counts from bun's JUnit XML. A testcase is a fail when it holds a * failure or error, a todo when skipped with message TODO, a skip when * skipped otherwise, else a pass; expects sum each testcase's assertions. - * Printed paths are matched back to planned files (bun prints them relative - * to the child's real cwd). Files that never ran a test are absent. + * Printed paths are matched back to planned files. Bun prints them relative + * to the child's real cwd, so a plain relative path must match exactly; an + * absolute or ../-prefixed path matches only when what precedes the planned + * file is the child's root. Files that never ran a test are absent. */ -export function parseJunitCounts(xml: string | null, planned: readonly string[], testsRan: number | null): ShardTestCounts { +export function parseJunitCounts(xml: string | null, planned: readonly string[], testsRan: number | null, root = process.cwd()): ShardTestCounts { const files: Record = {}; const totals = zeroCounts(); const result = (complete: boolean, problem?: string): ShardTestCounts => @@ -105,10 +107,14 @@ export function parseJunitCounts(xml: string | null, planned: readonly string[], if (xml === null) return result(false, 'bun wrote no JUnit report'); const plannedSet = new Set(planned); const unknown = new Set(); + const roots = new Set([root, (() => { try { return fs.realpathSync(root); } catch { return root; } })()].map(normalizeRelativePath)); + const isRoot = (prefix: string) => [...roots].some(dir => dir === prefix || dir.endsWith(`/${prefix}`)); const canonical = (printed: string) => { - const stripped = normalizeRelativePath(printed).replace(/^(?:\.{1,2}\/)+/, ''); + const normalized = normalizeRelativePath(printed); + const stripped = normalized.replace(/^(?:\.{1,2}\/)+/, ''); if (plannedSet.has(stripped)) return stripped; - const matches = planned.filter(file => stripped.endsWith(`/${file}`) || file.endsWith(`/${stripped}`)); + if (stripped === normalized && !/^(?:\/|[A-Za-z]:\/)/.test(stripped)) return stripped; + const matches = planned.filter(file => stripped.endsWith(`/${file}`) && isRoot(stripped.slice(0, -file.length - 1))); return matches.length === 1 ? matches[0] : stripped; }; let current: { file: string; state: 'pass' | 'fail' | 'skip' | 'todo'; expects: number } | null = null; @@ -316,15 +322,18 @@ const quote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; /** The FREE_SUBSHARD_TIMEOUT text: the file, k, runner, a serial reproduce command and the fix. */ export function subShardTimeoutText( - shard: number, jobs: number, record: SubShardRecord, inFlight: string[], wallTimeoutMs: number, context: CiRunContext, + shard: number, jobs: number, record: SubShardRecord, inFlight: string[], wallTimeoutMs: number, context: CiRunContext, shardWallMs = wallTimeoutMs, ): 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`; + const seconds = (ms: number) => Math.round(ms / 1000); + const fix = record.phase !== 'serial' + ? `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` + : wallTimeoutMs < shardWallMs + ? `the parallel sub-shards left this serial sub-shard ${seconds(wallTimeoutMs)}s of the ${seconds(shardWallMs)}s shard wall; if the k=1 command passes, the parallel phase used up the budget and this shard needs a lower --jobs; never raise the timeout` + : 'this file already runs alone after the parallel sub-shards; fix or split it, 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] ${FREE_SUBSHARD_TIMEOUT}: CI shard ${shard} sub-shard ${record.id} (${record.files.length} files) hit its ${seconds(wallTimeoutMs)}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`, @@ -395,7 +404,7 @@ export async function runCiShard(input: CiShardInput, deps: CiShardDeps): Promis 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 ?? [], walls.get(record.id) ?? 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, wallTimeoutMs)) 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]); diff --git a/scripts/test-free-shards.ts b/scripts/test-free-shards.ts index 0c34ec93d..deda761cf 100755 --- a/scripts/test-free-shards.ts +++ b/scripts/test-free-shards.ts @@ -1773,7 +1773,7 @@ export async function runFreeShard( shard: shardNumber, files, status, exitCode, elapsedMs: Date.now() - startedAt, groupPid, failingFiles, unattributedFailures, ...(status === 'passed' ? {} : { failures: report.failures }), summary: { testsRan: report.testsRan, filesRan: report.filesRan, sawTerminalSummary: report.sawTerminalSummary }, - ...(junitPath ? { counts: parseJunitCounts(junitXml, files, report.testsRan) } : {}), + ...(junitPath ? { counts: parseJunitCounts(junitXml, files, report.testsRan, rootDir) } : {}), ...(status === 'timed-out' && report.inFlight.length ? { inFlight: report.inFlight } : {}), }; log(shardEpilogue(outcome, totalShards)); diff --git a/test/shard-jobs.test.ts b/test/shard-jobs.test.ts index fe033a8af..83714013d 100644 --- a/test/shard-jobs.test.ts +++ b/test/shard-jobs.test.ts @@ -311,6 +311,15 @@ describe('per-file test and expect() counts from bun JUnit', () => { expect(counts.problem).toContain('unplanned'); }); + test('an absolute path is credited to the planned file under the root; an unplanned file never borrows a shorter planned path', () => { + const planned = ['test/telemetry.test.ts', 'browse/test/telemetry.test.ts']; + expect(Object.keys(parseJunitCounts(xml('/repo/browse/test/telemetry.test.ts'), planned, 4, '/repo').files)).toEqual(['browse/test/telemetry.test.ts']); + expect(Object.keys(parseJunitCounts(xml('../../repo/test/telemetry.test.ts'), planned, 4, '/repo').files)).toEqual(['test/telemetry.test.ts']); + const borrowed = parseJunitCounts(xml('browse/test/x.test.ts'), ['test/x.test.ts'], 4, '/repo'); + expect(borrowed).toMatchObject({ complete: false, problem: expect.stringContaining('browse/test/x.test.ts') }); + expect(parseJunitCounts(xml('/elsewhere/test/x.test.ts'), ['test/x.test.ts'], 4, '/repo').complete).toBe(false); + }); + 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 { @@ -447,5 +456,10 @@ describe('serial lists and the duration seed', () => { expect(ciRunContext('p.json', {}).runId).toBe('local'); expect(subShardTimeoutText(2, 4, { id: 'x:2.1', phase: 'parallel', files: ['test/a.test.ts'], status: 'timed-out', exitCode: null, elapsedMs: 1, failingFiles: [], unattributedFailures: 0 }, [], 10_000, context)[1]).toContain('in flight: unknown'); + const serial = { id: 'x:2.5', phase: 'serial' as const, files: ['test/s.test.ts'], status: 'timed-out' as const, exitCode: null, elapsedMs: 1, failingFiles: [], unattributedFailures: 0 }; + const squeezed = subShardTimeoutText(2, 4, serial, [], 4_000, context, 360_000); + expect(squeezed[0]).toContain('hit its 4s wall'); + expect(squeezed[4]).toContain('left this serial sub-shard 4s of the 360s shard wall'); + expect(subShardTimeoutText(2, 4, serial, [], 360_000, context, 360_000)[4]).toContain('already runs alone'); }); });