mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 07:18:03 +01:00
fix(test:free): exact root-bound JUnit path matching; serial timeout text names the wall the parallel phase left
Adversarial pass 2 (GSTA-203 PR 2): a plain relative path now matches a planned file exactly, and an absolute or ../-prefixed path matches only when its prefix is the child's root, so an unplanned browse/test/x.test.ts is never credited to test/x.test.ts. The serial sub-shard's FREE_SUBSHARD_TIMEOUT text says how much of the shard wall the parallel phase left it. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
f7eb79f9ad
commit
8ab672cfd6
3 files changed
+35
-12
No files matched your search
+20
-11
@@ -94,10 +94,12 @@ const attributes = (text: string): Record<string, string> =>
|
||||
* 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<string, FileTestCounts> = {};
|
||||
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<string>();
|
||||
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]);
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user