mirror of https://github.com/garrytan/gstack.git
fix(test): remove delayed process.exit(0) time bombs from 8 browser test files + exitFn seam in ServerConfig
Each afterAll scheduled setTimeout(() => process.exit(0), 500) which fired ~500ms into the NEXT test file during full-suite runs, killing bun with a green exit code and no tally — silently truncating the suite and hiding every failure in later files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
4a9c4c54d2
commit
94b7895ab3
|
|
@ -243,6 +243,14 @@ export interface ServerConfig {
|
|||
* reference is the PID record + the file paths.
|
||||
*/
|
||||
ownsTerminalAgent?: boolean;
|
||||
/**
|
||||
* Process-exit seam. shutdown() calls this at the very end of its async
|
||||
* teardown; defaults to process.exit. In-process test runs inject a
|
||||
* recording stub here — the fire-and-forget shutdown path otherwise races
|
||||
* per-test process.exit stubs, and a straggler real exit(0) truncates the
|
||||
* whole bun test run with a green exit code and no failure tally.
|
||||
*/
|
||||
exitFn?: (code?: number) => void;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1494,6 +1502,9 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
|||
// to gstack-owns. Matches the "default-true preserves CLI bit-for-bit"
|
||||
// premise even under malformed cfg.
|
||||
const ownsTerminalAgent = cfg.ownsTerminalAgent === false ? false : true;
|
||||
// Exit seam: production defaults to process.exit; tests inject a stub so
|
||||
// in-process shutdowns can never terminate the test runner itself.
|
||||
const exitFn = cfg.exitFn ?? ((code?: number) => process.exit(code));
|
||||
|
||||
// ─── Terminal-Agent Watchdog (v1.44+) ─────────────────────────────
|
||||
//
|
||||
|
|
@ -1619,7 +1630,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
|
|||
|
||||
cleanSingletonLocks(resolveChromiumProfile());
|
||||
safeUnlinkQuiet(config.stateFile);
|
||||
process.exit(exitCode);
|
||||
exitFn(exitCode);
|
||||
}
|
||||
|
||||
// Named lifecycle helper (matches closeTunnel style). Logs failures so
|
||||
|
|
|
|||
|
|
@ -42,9 +42,16 @@ beforeAll(async () => {
|
|||
// The test needs to start a server. Let's use the existing server infrastructure.
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
try { testServer.server.stop(); } catch {}
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the real browser (capped at 5s — close can hang) instead of a
|
||||
// delayed process.exit(0): that timer fired ~500ms into the NEXT test
|
||||
// file in a full-suite run and killed the whole bun process with a green
|
||||
// exit code and no tally, silently truncating the run.
|
||||
await Promise.race([
|
||||
bm.close().catch(() => {}),
|
||||
new Promise<void>(r => setTimeout(r, 5000)),
|
||||
]);
|
||||
});
|
||||
|
||||
// We need a running browse server for HTTP tests.
|
||||
|
|
|
|||
|
|
@ -94,11 +94,18 @@ beforeAll(async () => {
|
|||
await bm.launch();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
// Force kill browser instead of graceful close (avoids hang)
|
||||
try { testServer.server.stop(); } catch {}
|
||||
// bm.close() can hang — just let process exit handle it
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the real browser (capped at 5s — close can hang) instead of a
|
||||
// delayed process.exit(0): that timer fired ~500ms into the NEXT test
|
||||
// file in a full-suite run and killed the whole bun process with a green
|
||||
// exit code and no tally, silently truncating the run.
|
||||
await Promise.race([
|
||||
bm.close().catch(() => {}),
|
||||
new Promise<void>(r => setTimeout(r, 5000)),
|
||||
]);
|
||||
});
|
||||
|
||||
// ─── Navigation ─────────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -69,10 +69,17 @@ beforeAll(async () => {
|
|||
await handleWriteCommand('goto', [boardUrl], bm);
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
try { server.stop(); } catch {}
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the real browser (capped at 5s — close can hang) instead of a
|
||||
// delayed process.exit(0): that timer fired ~500ms into the NEXT test
|
||||
// file in a full-suite run and killed the whole bun process with a green
|
||||
// exit code and no tally, silently truncating the run.
|
||||
await Promise.race([
|
||||
bm.close().catch(() => {}),
|
||||
new Promise<void>(r => setTimeout(r, 5000)),
|
||||
]);
|
||||
});
|
||||
|
||||
// ─── DOM Structure ──────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -422,9 +422,16 @@ describe('Hidden element stripping', () => {
|
|||
await bm.launch();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
try { testServer.server.stop(); } catch {}
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the real browser (capped at 5s — close can hang) instead of a
|
||||
// delayed process.exit(0): that timer fired ~500ms into the NEXT test
|
||||
// file in a full-suite run and killed the whole bun process with a green
|
||||
// exit code and no tally, silently truncating the run.
|
||||
await Promise.race([
|
||||
bm.close().catch(() => {}),
|
||||
new Promise<void>(r => setTimeout(r, 5000)),
|
||||
]);
|
||||
});
|
||||
|
||||
test('detects CSS-hidden elements on injection-hidden page', async () => {
|
||||
|
|
|
|||
|
|
@ -26,9 +26,16 @@ beforeAll(async () => {
|
|||
await bm.launch();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
try { testServer.server.stop(); } catch {}
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the real browser (capped at 5s — close can hang) instead of a
|
||||
// delayed process.exit(0): that timer fired ~500ms into the NEXT test
|
||||
// file in a full-suite run and killed the whole bun process with a green
|
||||
// exit code and no tally, silently truncating the run.
|
||||
await Promise.race([
|
||||
bm.close().catch(() => {}),
|
||||
new Promise<void>(r => setTimeout(r, 5000)),
|
||||
]);
|
||||
});
|
||||
|
||||
// ─── Unit Tests: Failure Tracking (no browser needed) ────────────
|
||||
|
|
|
|||
|
|
@ -56,9 +56,16 @@ describe('defense-in-depth — live Playwright fixture', () => {
|
|||
await bm.launch();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
try { testServer.server.stop(); } catch {}
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the real browser (capped at 5s — close can hang) instead of a
|
||||
// delayed process.exit(0): that timer fired ~500ms into the NEXT test
|
||||
// file in a full-suite run and killed the whole bun process with a green
|
||||
// exit code and no tally, silently truncating the run.
|
||||
await Promise.race([
|
||||
bm.close().catch(() => {}),
|
||||
new Promise<void>(r => setTimeout(r, 5000)),
|
||||
]);
|
||||
});
|
||||
|
||||
test('L2 — content-security.ts hidden-element stripper detects the .sneaky div', async () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { describe, test, expect, beforeEach, mock } from 'bun:test';
|
||||
import { describe, test, expect, beforeAll, beforeEach, afterAll, mock } from 'bun:test';
|
||||
import {
|
||||
resolveConfigFromEnv,
|
||||
buildFetchHandler,
|
||||
|
|
@ -445,92 +445,68 @@ describe('idle timer + onDisconnect dual-instance fix', () => {
|
|||
});
|
||||
|
||||
test('CRITICAL — REGRESSION: headed embedder does not auto-shutdown at idle', () => {
|
||||
const exitMock = mock((_code?: number) => { throw new Error('process.exit called'); });
|
||||
const originalExit = process.exit;
|
||||
(process as any).exit = exitMock;
|
||||
try {
|
||||
const mockBM = makeMockBrowserManager('headed');
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any }));
|
||||
// Drive lastActivity past the idle threshold via the test seam instead
|
||||
// of mutating Date.now — the leaked module-level setInterval would
|
||||
// see fake-time and could fire shutdown if the timing aligned.
|
||||
__testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000));
|
||||
__testInternals__.idleCheckTick();
|
||||
expect(exitMock).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
(process as any).exit = originalExit;
|
||||
}
|
||||
const exitSpy = mock((_code?: number) => {});
|
||||
const mockBM = makeMockBrowserManager('headed');
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy }));
|
||||
// Drive lastActivity past the idle threshold via the test seam instead
|
||||
// of mutating Date.now — the leaked module-level setInterval would
|
||||
// see fake-time and could fire shutdown if the timing aligned.
|
||||
__testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000));
|
||||
__testInternals__.idleCheckTick();
|
||||
expect(exitSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('headless still auto-shuts down at idle (paired defensive)', async () => {
|
||||
// Non-throwing mock: idleCheckTick fires shutdown as a fire-and-forget
|
||||
// async call. Throwing from process.exit becomes an unhandled rejection
|
||||
// that the test runner catches. Recording the call is enough.
|
||||
const exitMock = mock((_code?: number) => {});
|
||||
const originalExit = process.exit;
|
||||
(process as any).exit = exitMock;
|
||||
try {
|
||||
const mockBM = makeMockBrowserManager('launched');
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any }));
|
||||
__testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000));
|
||||
__testInternals__.idleCheckTick();
|
||||
// Poll until the fire-and-forget shutdown reaches process.exit. A fixed
|
||||
// microtask drain can lose the race on slower hosts — and then the REAL
|
||||
// process.exit(0) fires after the finally restores it, killing the whole
|
||||
// bun test run mid-suite with a green exit code and no failure tally.
|
||||
for (let i = 0; i < 500 && exitMock.mock.calls.length === 0; i++) {
|
||||
await new Promise<void>(r => setTimeout(r, 10));
|
||||
}
|
||||
expect(exitMock).toHaveBeenCalled();
|
||||
} finally {
|
||||
// Reset activity so the module-level 60s idle interval (which outlives
|
||||
// this file) cannot re-fire shutdown with the real process.exit later
|
||||
// in the suite.
|
||||
__testInternals__.setLastActivity(Date.now());
|
||||
(process as any).exit = originalExit;
|
||||
// Shutdown is fire-and-forget async and hard-calling process.exit from it
|
||||
// is what this seam exists to avoid: a per-test process.exit stub can be
|
||||
// restored while a straggler shutdown is still in flight, and the real
|
||||
// exit(0) then kills the whole bun run mid-suite with a green exit code.
|
||||
// The injected exitFn spy makes the assertion race-free and harmless.
|
||||
const exitSpy = mock((_code?: number) => {});
|
||||
const mockBM = makeMockBrowserManager('launched');
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy }));
|
||||
__testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000));
|
||||
__testInternals__.idleCheckTick();
|
||||
// Poll until the async shutdown reaches the exit seam.
|
||||
for (let i = 0; i < 500 && exitSpy.mock.calls.length === 0; i++) {
|
||||
await new Promise<void>(r => setTimeout(r, 10));
|
||||
}
|
||||
expect(exitSpy).toHaveBeenCalled();
|
||||
// Reset activity so the module-level 60s idle interval cannot re-fire
|
||||
// shutdown later in the suite.
|
||||
__testInternals__.setLastActivity(Date.now());
|
||||
});
|
||||
|
||||
test('buildFetchHandler chains cfgBrowserManager.onDisconnect, preserving caller-set handler', async () => {
|
||||
const mockBM = makeMockBrowserManager('headed');
|
||||
const callerCb = mock(async (_code?: number) => {});
|
||||
mockBM.onDisconnect = callerCb;
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any }));
|
||||
const exitSpy = mock((_code?: number) => {});
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy }));
|
||||
// gstack should have wrapped the caller-installed handler instead of
|
||||
// clobbering it (Codex finding: BrowserManager.onDisconnect is a public
|
||||
// field; gbrowser may set it before calling buildFetchHandler).
|
||||
expect(typeof mockBM.onDisconnect).toBe('function');
|
||||
expect(mockBM.onDisconnect).not.toBe(callerCb);
|
||||
// Verify the chain: invoking the wrapped handler runs the caller
|
||||
// callback AND reaches activeShutdown (which calls process.exit at the
|
||||
// very end of its async path). Stubbing process.exit to throw aborts
|
||||
// the chain before isShuttingDown can leak into later tests.
|
||||
const exitMock = mock((_code?: number) => { throw new Error('process.exit called'); });
|
||||
const originalExit = process.exit;
|
||||
(process as any).exit = exitMock;
|
||||
try {
|
||||
await expect((mockBM.onDisconnect as any)(0)).rejects.toThrow('process.exit called');
|
||||
expect(callerCb).toHaveBeenCalledWith(0);
|
||||
expect(exitMock).toHaveBeenCalledWith(0);
|
||||
} finally {
|
||||
(process as any).exit = originalExit;
|
||||
// callback AND reaches activeShutdown, whose teardown ends at the
|
||||
// injected exit seam instead of the real process.exit.
|
||||
await (mockBM.onDisconnect as any)(0);
|
||||
for (let i = 0; i < 500 && exitSpy.mock.calls.length === 0; i++) {
|
||||
await new Promise<void>(r => setTimeout(r, 10));
|
||||
}
|
||||
expect(callerCb).toHaveBeenCalledWith(0);
|
||||
expect(exitSpy).toHaveBeenCalledWith(0);
|
||||
});
|
||||
|
||||
test('tunnelActive blocks idle-shutdown even in headless mode', () => {
|
||||
const exitMock = mock((_code?: number) => { throw new Error('process.exit called'); });
|
||||
const originalExit = process.exit;
|
||||
(process as any).exit = exitMock;
|
||||
try {
|
||||
const mockBM = makeMockBrowserManager('launched');
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any }));
|
||||
__testInternals__.setTunnelActive(true);
|
||||
__testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000));
|
||||
__testInternals__.idleCheckTick();
|
||||
expect(exitMock).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
(process as any).exit = originalExit;
|
||||
}
|
||||
const exitSpy = mock((_code?: number) => {});
|
||||
const mockBM = makeMockBrowserManager('launched');
|
||||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any, exitFn: exitSpy }));
|
||||
__testInternals__.setTunnelActive(true);
|
||||
__testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000));
|
||||
__testInternals__.idleCheckTick();
|
||||
expect(exitSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('lifecycle handlers (idleCheckTick + parent watchdog + SIGTERM) read activeBrowserManager, not module-level browserManager', () => {
|
||||
|
|
|
|||
|
|
@ -31,9 +31,13 @@ beforeAll(async () => {
|
|||
await bm.launch();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
try { testServer.server.stop(); } catch {}
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the real browser instead of scheduling a delayed process.exit(0):
|
||||
// in a full-suite run that timer fired ~500ms into the NEXT test file and
|
||||
// killed the entire bun process with a green exit code and no tally —
|
||||
// silently truncating the run and hiding every failure after this file.
|
||||
await bm.close().catch(() => {});
|
||||
});
|
||||
|
||||
// ─── Snapshot Output ────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -121,10 +121,15 @@ beforeAll(async () => {
|
|||
await bm.launch();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
afterAll(async () => {
|
||||
try { server.stop(); } catch {}
|
||||
fs.rmSync(tmpDir, { recursive: true, force: true });
|
||||
setTimeout(() => process.exit(0), 500);
|
||||
// Close the browser (capped) instead of a delayed process.exit(0): that
|
||||
// timer killed the whole bun process mid-suite with a green exit code.
|
||||
await Promise.race([
|
||||
bm.close().catch(() => {}),
|
||||
new Promise(r => setTimeout(r, 5000)),
|
||||
]);
|
||||
});
|
||||
|
||||
// ─── The critical test: browser click → file on disk ─────────────
|
||||
|
|
|
|||
Loading…
Reference in New Issue