mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(scripts): capture check/shard rc before watchdog teardown in no-timeout fallback (#2864)
On machines with neither gtimeout nor timeout on PATH, run-verify-parallel.sh and run-unit-parallel.sh fall back to a bg-pid + sleep-watchdog cap. Both read $? only after tearing the watchdog down (kill + wait on cap_pid), so the sentinel .exit files recorded the killed watchdog's status — 143 — instead of the check/shard's own exit code. Every run reported total failure (verify: pass=0 fail=31; unit: rc=143 per shard) while every per-check/shard log showed success. Capture rc immediately after `wait $pid` in both scripts, and reap the watchdog's sleep child (pkill -P, children-first — the same orphan quirk the heartbeat cleanup documents) so the fallback stops leaking one sleep per check/shard. Regression tests force the fallback branch hermetically on any host via a curated PATH with no timeout binaries: the verify dispatcher runs from a tempdir copy with a stubbed `bun`, pinning exit 0 + all-zero sentinels when checks pass and the check's own rc (not 143) when one fails; the unit wrapper runs real two-shard fixture passes, pinning rc=0 sentinels and a real failure's rc=1. Co-authored-by: YMYD <53603073+OJ-OnJourney@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Garry Tan
co-authored by
YMYD
Claude Fable 5
parent
ca47c054b8
commit
fbdc9fcd3b
@@ -133,6 +133,7 @@ for i in $(seq 1 "$N"); do
|
||||
env SHARD="$i/$N" \
|
||||
bash scripts/run-unit-shard.sh --max-concurrency="$INTRA_CONC" \
|
||||
> "$SHARD_LOG" 2>&1
|
||||
rc=$?
|
||||
else
|
||||
env SHARD="$i/$N" \
|
||||
bash scripts/run-unit-shard.sh --max-concurrency="$INTRA_CONC" \
|
||||
@@ -142,10 +143,20 @@ for i in $(seq 1 "$N"); do
|
||||
sleep 5 && kill -KILL "$pid" 2>/dev/null ) &
|
||||
cap_pid=$!
|
||||
wait "$pid" 2>/dev/null
|
||||
# Capture the shard's exit code from ITS `wait`, before any watchdog
|
||||
# teardown runs. The teardown commands below overwrite $? — the killed
|
||||
# watchdog reports 143 — which used to get stamped into every shard's
|
||||
# sentinel on machines with no gtimeout/timeout: every run "failed"
|
||||
# with rc=143 summaries even when all tests passed.
|
||||
rc=$?
|
||||
# Reap the watchdog's `sleep` child too (pkill -P), then the watchdog.
|
||||
# Killing only the subshell leaves the sleep orphaned until
|
||||
# $SHARD_TIMEOUT elapses — same quirk the heartbeat cleanup below works
|
||||
# around; CI's orphan-process sweep flags those.
|
||||
pkill -P "$cap_pid" 2>/dev/null
|
||||
kill "$cap_pid" 2>/dev/null
|
||||
wait "$cap_pid" 2>/dev/null
|
||||
fi
|
||||
rc=$?
|
||||
echo "$rc" > "$LOG_DIR/shard-$i.exit"
|
||||
[ "$rc" = "124" ] && echo "WEDGED" > "$LOG_DIR/shard-$i.wedged"
|
||||
) &
|
||||
|
||||
@@ -126,6 +126,7 @@ for c in "${CHECKS[@]}"; do
|
||||
(
|
||||
if [ -n "$TIMEOUT_BIN" ]; then
|
||||
"$TIMEOUT_BIN" "${TIMEOUT}s" bun run "$c" > "$LOG_FILE" 2>&1
|
||||
rc=$?
|
||||
else
|
||||
bun run "$c" > "$LOG_FILE" 2>&1 &
|
||||
pid=$!
|
||||
@@ -133,10 +134,20 @@ for c in "${CHECKS[@]}"; do
|
||||
sleep 5 && kill -KILL "$pid" 2>/dev/null ) &
|
||||
cap_pid=$!
|
||||
wait "$pid" 2>/dev/null
|
||||
# Capture the check's exit code from ITS `wait`, before any watchdog
|
||||
# teardown runs. The teardown commands below overwrite $? — the killed
|
||||
# watchdog reports 143 — which used to get stamped into every sentinel
|
||||
# on machines with no gtimeout/timeout: verify reported pass=0
|
||||
# fail=<all> while every per-check log said OK.
|
||||
rc=$?
|
||||
# Reap the watchdog's `sleep` child too (pkill -P), then the watchdog.
|
||||
# Killing only the subshell leaves the sleep orphaned until $TIMEOUT
|
||||
# elapses — same quirk the heartbeat cleanup in run-unit-parallel.sh
|
||||
# works around; CI's orphan-process sweep flags those.
|
||||
pkill -P "$cap_pid" 2>/dev/null
|
||||
kill "$cap_pid" 2>/dev/null
|
||||
wait "$cap_pid" 2>/dev/null
|
||||
fi
|
||||
rc=$?
|
||||
echo "$rc" > "$EXIT_FILE"
|
||||
) &
|
||||
PIDS+=($!)
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { execFileSync, spawnSync } from 'child_process';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync, copyFileSync, chmodSync } from 'fs';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync, copyFileSync, chmodSync, symlinkSync } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join, resolve } from 'path';
|
||||
|
||||
@@ -154,3 +154,93 @@ describe('failing-on-purpose', () => {
|
||||
expect(summary).toMatch(/shard 2\/2: pass=\d+ fail=\d+ skip=\d+ rc=\d+/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('run-unit-parallel.sh no-timeout-binary fallback (rc from shard wait, not watchdog teardown)', () => {
|
||||
// Forces the no-gtimeout/no-timeout branch by running the wrapper under a
|
||||
// curated PATH that has every tool the scripts call EXCEPT timeout
|
||||
// binaries (real `bun` symlinked in), so the fallback executes even on
|
||||
// hosts with coreutils installed.
|
||||
//
|
||||
// Regression pinned here: the shard's sentinel .exit file must record the
|
||||
// exit code read right after `wait $pid` (the shard's own rc). The
|
||||
// watchdog subshell is killed with SIGTERM and reports 143; reading `$?`
|
||||
// after that teardown stamped rc=143 into every shard's sentinel — the
|
||||
// wrapper exited non-zero with rc=143 summaries even when every test
|
||||
// passed.
|
||||
let FROOT: string;
|
||||
let FENV: Record<string, string>;
|
||||
|
||||
beforeAll(() => {
|
||||
FROOT = mkdtempSync(join(tmpdir(), 'gbrain-parallel-fallback-'));
|
||||
mkdirSync(join(FROOT, 'scripts'), { recursive: true });
|
||||
mkdirSync(join(FROOT, 'test'), { recursive: true });
|
||||
for (const s of ['run-unit-parallel.sh', 'run-unit-shard.sh', 'run-serial-tests.sh']) {
|
||||
copyFileSync(resolve(REPO_ROOT, 'scripts', s), join(FROOT, 'scripts', s));
|
||||
chmodSync(join(FROOT, 'scripts', s), 0o755);
|
||||
}
|
||||
const passing = `import { describe, it, expect } from 'bun:test';
|
||||
describe('passing', () => {
|
||||
it('arithmetic works', () => { expect(1 + 1).toBe(2); });
|
||||
});`;
|
||||
writeFileSync(join(FROOT, 'test', 'a-pass.test.ts'), passing);
|
||||
writeFileSync(join(FROOT, 'test', 'b-pass.test.ts'), passing);
|
||||
|
||||
const bin = join(FROOT, 'bin');
|
||||
mkdirSync(bin);
|
||||
for (const tool of ['bash', 'sh', 'env', 'dirname', 'basename', 'mktemp', 'date', 'sleep', 'cat', 'tail', 'head', 'rm', 'mkdir', 'pkill', 'grep', 'sed', 'awk', 'wc', 'tr', 'seq', 'find', 'sort', 'bun']) {
|
||||
const p = Bun.which(tool);
|
||||
if (p) symlinkSync(p, join(bin, tool));
|
||||
}
|
||||
FENV = {
|
||||
PATH: bin,
|
||||
HOME: process.env.HOME ?? FROOT,
|
||||
TMPDIR: process.env.TMPDIR ?? '/tmp',
|
||||
GBRAIN_TEST_SHARD_TIMEOUT: '300',
|
||||
};
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (FROOT) rmSync(FROOT, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function runFallbackWrapper(): { code: number; stdout: string; stderr: string } {
|
||||
const result = spawnSync(
|
||||
'bash',
|
||||
[join(FROOT, 'scripts', 'run-unit-parallel.sh'), '--shards', '2'],
|
||||
{ cwd: FROOT, encoding: 'utf-8', env: FENV },
|
||||
);
|
||||
return {
|
||||
code: result.status ?? -1,
|
||||
stdout: result.stdout || '',
|
||||
stderr: result.stderr || '',
|
||||
};
|
||||
}
|
||||
|
||||
it('exits zero with rc=0 shard sentinels when all shards pass', () => {
|
||||
const r = runFallbackWrapper();
|
||||
const summary = readFileSync(join(FROOT, '.context', 'test-summary.txt'), 'utf-8');
|
||||
expect(summary).toMatch(/shard 1\/2: pass=\d+ fail=0 skip=0 rc=0/);
|
||||
expect(summary).toMatch(/shard 2\/2: pass=\d+ fail=0 skip=0 rc=0/);
|
||||
expect(summary).not.toContain('rc=143');
|
||||
expect(r.code).toBe(0);
|
||||
});
|
||||
|
||||
it('propagates a failing shard rc as the test runner rc (1), not the watchdog 143', () => {
|
||||
const failing = `import { describe, it, expect } from 'bun:test';
|
||||
describe('failing-on-purpose', () => {
|
||||
it('expects 1 to equal 2', () => { expect(1).toBe(2); });
|
||||
});`;
|
||||
writeFileSync(join(FROOT, 'test', 'z-fail.test.ts'), failing);
|
||||
try {
|
||||
const r = runFallbackWrapper();
|
||||
expect(r.code).not.toBe(0);
|
||||
const summary = readFileSync(join(FROOT, '.context', 'test-summary.txt'), 'utf-8');
|
||||
expect(summary).toMatch(/shard \d\/2: pass=\d+ fail=1 skip=0 rc=1/);
|
||||
expect(summary).not.toContain('rc=143');
|
||||
const failureLog = readFileSync(join(FROOT, '.context', 'test-failures.log'), 'utf-8');
|
||||
expect(failureLog).toContain('failing-on-purpose');
|
||||
} finally {
|
||||
rmSync(join(FROOT, 'test', 'z-fail.test.ts'), { force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -15,7 +15,16 @@
|
||||
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { writeFileSync, mkdtempSync, rmSync } from "node:fs";
|
||||
import {
|
||||
copyFileSync,
|
||||
mkdirSync,
|
||||
mkdtempSync,
|
||||
readFileSync,
|
||||
readdirSync,
|
||||
rmSync,
|
||||
symlinkSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
@@ -173,3 +182,95 @@ exit 0
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("run-verify-parallel.sh — no-timeout-binary fallback rc capture (regression)", () => {
|
||||
// macOS ships no `timeout`; without brew coreutils (`gtimeout`) — stock
|
||||
// machines, minimal containers, restricted/sandboxed PATHs — the dispatcher
|
||||
// degrades to the bg-pid + sleep-watchdog branch.
|
||||
//
|
||||
// Regression pinned here: each check's sentinel .exit file must record the
|
||||
// exit code of the CHECK (read right after `wait $pid`), not of the
|
||||
// watchdog teardown. The watchdog subshell is killed with SIGTERM and so
|
||||
// reports 143; reading `$?` after the teardown stamped 143 into every
|
||||
// sentinel — verify reported pass=0 fail=<all> while every per-check log
|
||||
// said OK.
|
||||
//
|
||||
// Hermetic on any host: the script runs from a tempdir copy with `bun`
|
||||
// stubbed (checks complete instantly, no repo needed) and PATH set to a
|
||||
// curated symlink dir containing everything the script calls EXCEPT
|
||||
// gtimeout/timeout — forcing the fallback branch even where coreutils is
|
||||
// installed.
|
||||
|
||||
function makeFallbackHarness(): { root: string; env: Record<string, string> } {
|
||||
const root = mkdtempSync(join(tmpdir(), "verify-fallback-"));
|
||||
mkdirSync(join(root, "scripts"), { recursive: true });
|
||||
copyFileSync(SCRIPT, join(root, "scripts", "run-verify-parallel.sh"));
|
||||
|
||||
const bin = join(root, "bin");
|
||||
mkdirSync(bin);
|
||||
// Everything the dispatcher and its subshells invoke, minus timeout bins.
|
||||
for (const tool of ["bash", "sh", "env", "dirname", "mktemp", "date", "sleep", "cat", "tail", "head", "rm", "mkdir", "pkill", "grep", "sed", "awk"]) {
|
||||
const p = Bun.which(tool);
|
||||
if (p) symlinkSync(p, join(bin, tool));
|
||||
}
|
||||
// `bun run <name>` stand-in: instant, prints OK, exits 7 for the check
|
||||
// named in $STUB_FAIL_CHECK (if any).
|
||||
writeFileSync(
|
||||
join(bin, "bun"),
|
||||
`#!/usr/bin/env bash
|
||||
name="\${2:-}"
|
||||
echo "stub check OK: $name"
|
||||
if [ -n "\${STUB_FAIL_CHECK:-}" ] && [ "$name" = "\${STUB_FAIL_CHECK}" ]; then
|
||||
echo "stub check failing: $name" >&2
|
||||
exit 7
|
||||
fi
|
||||
exit 0
|
||||
`,
|
||||
{ mode: 0o755 },
|
||||
);
|
||||
|
||||
return {
|
||||
root,
|
||||
env: {
|
||||
PATH: bin,
|
||||
HOME: process.env.HOME ?? root,
|
||||
TMPDIR: process.env.TMPDIR ?? "/tmp",
|
||||
GBRAIN_VERIFY_TIMEOUT: "30",
|
||||
GBRAIN_VERIFY_LOG_DIR: join(root, "logs"),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("all checks passing → exit 0, every sentinel records 0 (not the watchdog's 143)", () => {
|
||||
const { root, env } = makeFallbackHarness();
|
||||
try {
|
||||
const r = spawnSync("bash", [join(root, "scripts", "run-verify-parallel.sh")], { encoding: "utf8", env });
|
||||
expect(r.stderr).toMatch(/pass=\d+ fail=0/);
|
||||
expect(r.status).toBe(0);
|
||||
const exits = readdirSync(join(root, "logs")).filter((f) => f.endsWith(".exit"));
|
||||
expect(exits.length).toBeGreaterThan(10);
|
||||
for (const f of exits) {
|
||||
expect(readFileSync(join(root, "logs", f), "utf8").trim()).toBe("0");
|
||||
}
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("one check failing → exit 1, sentinel records the check's own rc (7), not 143", () => {
|
||||
const { root, env } = makeFallbackHarness();
|
||||
try {
|
||||
const r = spawnSync("bash", [join(root, "scripts", "run-verify-parallel.sh")], {
|
||||
encoding: "utf8",
|
||||
env: { ...env, STUB_FAIL_CHECK: "check:jsonb" },
|
||||
});
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stderr).toContain("--- check:jsonb (rc=7)");
|
||||
expect(r.stderr).toContain("stub check failing: check:jsonb");
|
||||
expect(r.stderr).toMatch(/fail=1\b/);
|
||||
expect(readFileSync(join(root, "logs", "check_jsonb.exit"), "utf8").trim()).toBe("7");
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user