mirror of https://github.com/garrytan/gstack.git
fix(test): un-leak GSTACK_HOME across files, file-scoped process.exit guard + idle-timer stop, closeTab zero-tab invariant, align config tests to DEFAULTS contract
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
7c9df1c568
commit
4a9c4c54d2
|
|
@ -803,15 +803,14 @@ export class BrowserManager {
|
|||
this.tabSessions.delete(tabId);
|
||||
this.tabOwnership.delete(tabId);
|
||||
|
||||
// Switch to another tab if we closed the active one
|
||||
if (tabId === this.activeTabId) {
|
||||
// Never leave zero tabs, regardless of which tab was closed — activeTabId
|
||||
// can point at an already-removed id, and the old active-only guard then
|
||||
// skipped auto-create when the last tab closed.
|
||||
if (this.pages.size === 0) {
|
||||
await this.newTab();
|
||||
} else if (tabId === this.activeTabId) {
|
||||
const remaining = [...this.pages.keys()];
|
||||
if (remaining.length > 0) {
|
||||
this.activeTabId = remaining[remaining.length - 1];
|
||||
} else {
|
||||
// No tabs left — create a new blank one
|
||||
await this.newTab();
|
||||
}
|
||||
this.activeTabId = remaining[remaining.length - 1];
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -664,6 +664,11 @@ export const __testInternals__ = {
|
|||
// need shutdown to fire. Without this, the second test's shutdown
|
||||
// returns early at the `if (isShuttingDown) return;` guard.
|
||||
resetShutdownState: () => { isShuttingDown = false; },
|
||||
// Stop the module-level 60s idle interval. In-process test runs import this
|
||||
// module once for the whole suite; the interval otherwise outlives the test
|
||||
// file and can fire a real process.exit(0) mid-suite, silently truncating
|
||||
// the run with a green exit code.
|
||||
stopIdleTimer: () => { clearInterval(idleCheckInterval); },
|
||||
};
|
||||
|
||||
// ─── Parent-Process Watchdog ────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import { startTestServer } from './test-server';
|
|||
import { BrowserManager } from '../src/browser-manager';
|
||||
|
||||
const TMP_HOME = path.join(os.tmpdir(), `gstack-cdp-e2e-${process.pid}-${Date.now()}`);
|
||||
const ORIG_GSTACK_HOME = process.env.GSTACK_HOME;
|
||||
process.env.GSTACK_HOME = TMP_HOME;
|
||||
process.env.GSTACK_TELEMETRY_OFF = '1'; // don't pollute analytics during tests
|
||||
|
||||
|
|
@ -36,6 +37,11 @@ beforeAll(async () => {
|
|||
});
|
||||
|
||||
afterAll(async () => {
|
||||
// Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config,
|
||||
// so leaving it set breaks later test files in-process.
|
||||
if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME;
|
||||
else process.env.GSTACK_HOME = ORIG_GSTACK_HOME;
|
||||
|
||||
try { await bm.cleanup?.(); } catch {}
|
||||
try { testServer.server.stop(); } catch {}
|
||||
await fs.rm(TMP_HOME, { recursive: true, force: true });
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ import { startTestServer } from './test-server';
|
|||
import { BrowserManager } from '../src/browser-manager';
|
||||
|
||||
const TMP_HOME = path.join(os.tmpdir(), `gstack-domain-e2e-${process.pid}-${Date.now()}`);
|
||||
const ORIG_GSTACK_HOME = process.env.GSTACK_HOME;
|
||||
process.env.GSTACK_HOME = TMP_HOME;
|
||||
process.env.GSTACK_PROJECT_SLUG = 'e2e-test-slug';
|
||||
|
||||
|
|
@ -41,6 +42,11 @@ beforeAll(async () => {
|
|||
});
|
||||
|
||||
afterAll(async () => {
|
||||
// Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config,
|
||||
// so leaving it set breaks later test files in-process.
|
||||
if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME;
|
||||
else process.env.GSTACK_HOME = ORIG_GSTACK_HOME;
|
||||
|
||||
try { await bm.cleanup?.(); } catch {}
|
||||
try { testServer.server.stop(); } catch {}
|
||||
await fs.rm(TMP_HOME, { recursive: true, force: true });
|
||||
|
|
|
|||
|
|
@ -1,11 +1,19 @@
|
|||
import { describe, it, expect, beforeEach } from 'bun:test';
|
||||
import { describe, it, expect, beforeEach, afterAll } from 'bun:test';
|
||||
import { promises as fs } from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
|
||||
const TMP_HOME = path.join(os.tmpdir(), `gstack-test-${process.pid}-${Date.now()}`);
|
||||
const ORIG_GSTACK_HOME = process.env.GSTACK_HOME;
|
||||
process.env.GSTACK_HOME = TMP_HOME;
|
||||
|
||||
afterAll(() => {
|
||||
// Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config,
|
||||
// so leaving it set breaks later test files in-process.
|
||||
if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME;
|
||||
else process.env.GSTACK_HOME = ORIG_GSTACK_HOME;
|
||||
});
|
||||
|
||||
// Re-import after env var set so module reads updated GSTACK_HOME
|
||||
async function freshImport() {
|
||||
// Bun caches modules; force reload by appending a query-string-like hack via dynamic import URL
|
||||
|
|
|
|||
|
|
@ -18,7 +18,12 @@ function run(args: string[] = [], extraEnv: Record<string, string> = {}) {
|
|||
const result = Bun.spawnSync(['bash', SCRIPT, ...args], {
|
||||
env: {
|
||||
...process.env,
|
||||
// Set all three state-dir vars: other test files leak GSTACK_HOME /
|
||||
// GSTACK_STATE_ROOT into process.env at module scope, and those
|
||||
// outrank GSTACK_STATE_DIR in the script's resolution order.
|
||||
GSTACK_STATE_DIR: stateDir,
|
||||
GSTACK_STATE_ROOT: stateDir,
|
||||
GSTACK_HOME: stateDir,
|
||||
...extraEnv,
|
||||
},
|
||||
stdout: 'pipe',
|
||||
|
|
|
|||
|
|
@ -411,6 +411,27 @@ function makeMockBrowserManager(mode: 'launched' | 'headed') {
|
|||
}
|
||||
|
||||
describe('idle timer + onDisconnect dual-instance fix', () => {
|
||||
// File-scoped process.exit guard. The shutdown path is fire-and-forget
|
||||
// async: a test's per-call exit stub can be restored while a second
|
||||
// shutdown invocation is still in flight, and that straggler then hits the
|
||||
// REAL process.exit(0) — truncating the whole bun test run mid-suite with
|
||||
// a green exit code and no failure tally. Per-test stubs installed below
|
||||
// capture-and-restore against this file stub, never the real exit.
|
||||
const fileExitStub = mock((_code?: number) => {});
|
||||
let realProcessExit: typeof process.exit;
|
||||
beforeAll(() => {
|
||||
realProcessExit = process.exit;
|
||||
(process as any).exit = fileExitStub;
|
||||
});
|
||||
afterAll(async () => {
|
||||
// Kill the leaked module-level 60s interval so it can't fire shutdown
|
||||
// (with the real process.exit) during a later test file.
|
||||
__testInternals__.stopIdleTimer();
|
||||
// Grace period: let any straggler async shutdown land on the stub.
|
||||
await new Promise<void>(r => setTimeout(r, 300));
|
||||
(process as any).exit = realProcessExit;
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
__resetRegistry();
|
||||
// Reset module state every test. Bun memoizes the server.ts module
|
||||
|
|
@ -453,14 +474,19 @@ describe('idle timer + onDisconnect dual-instance fix', () => {
|
|||
buildFetchHandler(makeMinimalConfig({ browserManager: mockBM as any }));
|
||||
__testInternals__.setLastActivity(Date.now() - (31 * 60 * 1000));
|
||||
__testInternals__.idleCheckTick();
|
||||
// Drain microtasks: shutdown awaits flushBuffers + cfgBrowserManager.close
|
||||
// before reaching process.exit.
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
await new Promise<void>(r => setImmediate(r));
|
||||
await new Promise<void>(r => setImmediate(r));
|
||||
// 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;
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ const TELEMETRY_FILE = path.join(TMP_HOME, 'analytics', 'browse-telemetry.jsonl'
|
|||
|
||||
// Use GSTACK_HOME env to redirect telemetry writes (read each call,
|
||||
// not cached at module-load).
|
||||
const ORIG_GSTACK_HOME = process.env.GSTACK_HOME;
|
||||
process.env.GSTACK_HOME = TMP_HOME;
|
||||
process.env.GSTACK_TELEMETRY_OFF = '0';
|
||||
|
||||
|
|
@ -17,6 +18,10 @@ beforeEach(async () => {
|
|||
|
||||
afterAll(async () => {
|
||||
await fs.rm(TMP_HOME, { recursive: true, force: true });
|
||||
// Un-leak GSTACK_HOME: it outranks GSTACK_STATE_DIR in bin/gstack-config's
|
||||
// resolution order, so leaving it set breaks later test files in-process.
|
||||
if (ORIG_GSTACK_HOME === undefined) delete process.env.GSTACK_HOME;
|
||||
else process.env.GSTACK_HOME = ORIG_GSTACK_HOME;
|
||||
});
|
||||
|
||||
async function readEvents(): Promise<any[]> {
|
||||
|
|
|
|||
Loading…
Reference in New Issue