Merge PR #8458: fix(ci): distinguish a watchdog kill from an OOM kill in the sequential test path (@Mustaqeem66)

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<number> {
+	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<string> {
+	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;
+		}
+	});
+});

scripts/ci-test-ts.ts

@@ -417,19 +417,25 @@ async function runTestCommand(testCommand: TestCommand): Promise<void> {
 			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<number, true> = {
 // 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.