diff --git a/scripts/ci-test-ts.test.ts b/scripts/ci-test-ts.test.ts new file mode 100644 index 000000000..2b73cb41a --- /dev/null +++ b/scripts/ci-test-ts.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, test } from "bun:test"; +import { describeChunkFailure } from "./ci-test-ts.ts"; + +// The two ways a chunk reaches SIGKILL are indistinguishable by exit code, so +// these drive real subprocesses to produce a genuine 137 rather than asserting +// against a hand-written constant. +async function spawnExitCode(script: string): Promise { + const proc = Bun.spawn(["sh", "-c", script], { stdout: "ignore", stderr: "ignore" }); + return await proc.exited; +} + +// Re-hosts the sequential runner's failure tail: spawn, watchdog, attribute. +// `runTestCommand` itself is not injectable (it builds argv from the repo +// layout), so the decision under test is driven directly. +async function runWithWatchdog(script: string, timeoutMs: number): Promise { + const proc = Bun.spawn(["sh", "-c", script], { stdout: "ignore", stderr: "ignore" }); + let timedOut = false; + const killTimer = setTimeout(() => { + timedOut = true; + proc.kill("SIGKILL"); + }, timeoutMs); + const exitCode = await proc.exited; + clearTimeout(killTimer); + return describeChunkFailure(exitCode, timedOut); +} + +describe("describeChunkFailure", () => { + test("a real SIGKILL that the watchdog did not cause is attributed to the OOM killer", async () => { + const exitCode = await spawnExitCode("kill -9 $$"); + expect(exitCode).toBe(137); + + const message = describeChunkFailure(exitCode, false); + expect(message).toContain("OOM killer"); + expect(message).toContain("chunkSize"); + // The old wording carried no cause at all; it must not come back. + expect(message).not.toBe("failed with exit code 137"); + }); + + test("a watchdog kill is attributed to the watchdog, not to memory", async () => { + const message = await runWithWatchdog("sleep 30", 150); + expect(message).toContain("chunk watchdog"); + expect(message).toContain("OMP_TEST_CHUNK_TIMEOUT"); + expect(message).not.toContain("OOM killer"); + }); + + test("the two SIGKILL causes produce different messages from the same exit code", async () => { + const oomKilled = describeChunkFailure(137, false); + const watchdogKilled = describeChunkFailure(137, true); + expect(oomKilled).not.toBe(watchdogKilled); + }); + + test("an ordinary test failure keeps the plain wording", async () => { + const exitCode = await spawnExitCode("exit 1"); + expect(exitCode).toBe(1); + expect(describeChunkFailure(exitCode, false)).toBe("failed with exit code 1"); + }); + + test("a bun crash exit keeps the plain wording so the retry log still reads naturally", () => { + expect(describeChunkFailure(134, false)).toBe("failed with exit code 134"); + expect(describeChunkFailure(139, false)).toBe("failed with exit code 139"); + }); + + test("the watchdog message reports the configured timeout", () => { + const previous = Bun.env.OMP_TEST_CHUNK_TIMEOUT; + Bun.env.OMP_TEST_CHUNK_TIMEOUT = "42"; + try { + expect(describeChunkFailure(137, true)).toContain("42s"); + } finally { + if (previous === undefined) delete Bun.env.OMP_TEST_CHUNK_TIMEOUT; + else Bun.env.OMP_TEST_CHUNK_TIMEOUT = previous; + } + }); +}); diff --git a/scripts/ci-test-ts.ts b/scripts/ci-test-ts.ts index 61dfb9d79..51e492c96 100755 --- a/scripts/ci-test-ts.ts +++ b/scripts/ci-test-ts.ts @@ -417,19 +417,25 @@ async function runTestCommand(testCommand: TestCommand): Promise { stdout: "inherit", stderr: "inherit", }); - const killTimer = setTimeout(() => proc.kill("SIGKILL"), chunkTimeoutMs()); + // Watchdog, mirroring the parallel path: record that *we* killed the child, + // otherwise the resulting 137 is indistinguishable from an OOM kill. + let timedOut = false; + const killTimer = setTimeout(() => { + timedOut = true; + proc.kill("SIGKILL"); + }, chunkTimeoutMs()); const exitCode = await proc.exited; clearTimeout(killTimer); if (exitCode === 0) { return; } - if (BUN_CRASH_EXITS[exitCode] && attempt < MAX_CHUNK_ATTEMPTS) { + if (!timedOut && BUN_CRASH_EXITS[exitCode] && attempt < MAX_CHUNK_ATTEMPTS) { console.log( `==> ${testCommand.label}: bun crashed (exit ${exitCode}); retrying (attempt ${attempt + 1}/${MAX_CHUNK_ATTEMPTS})`, ); continue; } - throw new Error(`${testCommand.label} failed with exit code ${exitCode}: ${renderedCommand}`); + throw new Error(`${testCommand.label} ${describeChunkFailure(exitCode, timedOut)}: ${renderedCommand}`); } } @@ -505,6 +511,21 @@ const BUN_CRASH_EXITS: Record = { // deterministic crash still fails every attempt and is reported normally. const MAX_CHUNK_ATTEMPTS = 3; +// Why a chunk failed, in words. Exit 137 is SIGKILL, which this runner reaches +// two very different ways -- the per-chunk watchdog firing, or the kernel OOM +// killer reaping a chunk that outgrew the runner -- and the bare exit code +// cannot tell them apart. Which one it was is the difference between "raise +// OMP_TEST_CHUNK_TIMEOUT" and "lower this bucket's chunkSize", so say it. +export function describeChunkFailure(exitCode: number, timedOut: boolean): string { + if (timedOut) { + return `exceeded the ${Math.round(chunkTimeoutMs() / 1000)}s chunk watchdog and was killed (exit ${exitCode}; OMP_TEST_CHUNK_TIMEOUT to change)`; + } + if (exitCode === 137) { + return "was SIGKILLed (exit 137) without reaching the chunk watchdog, which on a CI runner means the OOM killer; lower this bucket's chunkSize"; + } + return `failed with exit code ${exitCode}`; +} + // The standard `CI` signal is authoritative. In CI each bucket is its own // memory-capped runner job (a single fat invocation gets OOM-killed at 137), so // chunks run sequentially within a job and parallelism happens across jobs.