Skip to content

Commit 1a6fd7f

Browse files
gbasinGary Basin
andauthored
test: isolate terminalProxyFactory mock leak; disable skipMatchingPatterns in test runner (#140)
## Summary Two latent test fragilities, both invisible on CI and on most contributors' machines but reproducible from a working tree under `/tmp` or `/var/folders` (worktrees, scratch clones, some CI configs). - `logPoller.test.ts` builds project paths from `process.cwd()`. The default `config.skipMatchingPatterns` (`src/server/config.ts:65-71`) matches `/private/tmp/*`, `/tmp/*`, `/private/var/folders/*`, `/var/folders/*` — exactly where tmp-rooted checkouts live. The matcher then skips those projects, so the orphan-rematch tests silently fail (e.g. `rematches orphaned sessions on startup without new activity`, `clears resume error when orphaned session rematches during poll`). The patterns are a production safety net; they shouldn't gate test fixtures. - `terminalProxyFactory.test.ts:42-44` installs a top-level `mock.module('../config', ...)` whose replacement omits many real-config fields. Bun's `mock.restore()` in `afterAll` does not fully unwind module-level mocks, so later files that import `../config` read through the stripped mock. On many readdir orderings this happened to mask issue (1) above — the stripped config has no skip patterns, so the orphan-rematch tests "passed" via mock leakage instead of legitimately. ## Fix - Set `AGENTBOARD_SKIP_MATCHING_PATTERNS=''` in the runner's subprocess env so the matcher doesn't suppress test working directories. - Add `terminalProxyFactory.test.ts` to `ISOLATED_FILES` so it runs in its own bun process and can't bleed module mocks into later files. ## Test plan - [x] `bun run lint` — clean - [x] `bun run typecheck` — clean - [x] `bun run test` from `/Users/garybasin/Code/agentboard` — all green - [x] `bun run test` from `/tmp/test-fixes` worktree (the previously-failing case) — all green - [x] No production source code changed; only `scripts/test-runner.ts`. ## Notes - Tests that intentionally exercise skip-pattern logic (`logMatchGate.test.ts`) pass patterns directly to the matcher API, so they are unaffected. - Direct `bun test <file>` invocations from a `/tmp`-rooted checkout still skip — only `bun run test` (the documented entry point and what CI/hooks use) is wired up. Adding the env wiring at the `bun test` layer would require either Bun's `preload` mechanism or every test file setting its own env, both of which are heavier than warranted here. --------- Co-authored-by: Gary Basin <gary@pineapple.mortgage>
1 parent 7cbda40 commit 1a6fd7f

1 file changed

Lines changed: 30 additions & 14 deletions

File tree

scripts/test-runner.ts

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ async function main() {
4141
CODEX_HOME: codexDir,
4242
LOG_FILE: tempLogFile,
4343
AGENTBOARD_DB_PATH: tempDbPath,
44+
// Default skipMatchingPatterns excludes /tmp/* and /var/folders/* — both
45+
// common locations for test working directories (worktrees, CI runners on
46+
// some platforms). Tests that exercise matching logic from those paths
47+
// would otherwise be silently skipped. Tests that need specific skip
48+
// behavior pass patterns explicitly via the matcher API.
49+
AGENTBOARD_SKIP_MATCHING_PATTERNS: '',
4450
}
4551

4652
try {
@@ -55,6 +61,13 @@ async function main() {
5561
'sessionRefreshWorker.test.ts',
5662
'pipePaneTerminalProxy.test.ts',
5763
'hydrateSessionsEmptyGuard.test.ts',
64+
// terminalProxyFactory.test.ts installs a top-level
65+
// mock.module('../config', ...) whose replacement omits many real
66+
// config fields. Bun's mock.restore() in afterAll does not fully
67+
// unwind module-level mocks, so the stripped config can leak into
68+
// any later test file that imports `../config` (notably
69+
// logPoller.test.ts, which depends on skipMatchingPatterns).
70+
'terminalProxyFactory.test.ts',
5871
])
5972

6073
// Client tests that install top-level mock.module(...) hooks must run in a
@@ -90,21 +103,24 @@ async function main() {
90103
)
91104

92105
// Always run global-mutating tests in a separate process to prevent races.
93-
const isolatedFiles = Array.from(ISOLATED_FILES).map(
94-
(f) => `src/server/__tests__/${f}`
95-
)
96-
await runCommand(
97-
['bun', 'test', ...passthroughArgs, ...isolatedFiles],
98-
env
99-
)
106+
// Each file runs in its own bun process — isolation is from every other
107+
// file, not just from the main suite. terminalProxyFactory.test.ts
108+
// installs mock.module('../terminal/PipePaneTerminalProxy', ...) that
109+
// would otherwise leak into pipePaneTerminalProxy.test.ts on readdir
110+
// orderings where it loads first (Linux ext4).
111+
for (const file of ISOLATED_FILES) {
112+
await runCommand(
113+
['bun', 'test', ...passthroughArgs, `src/server/__tests__/${file}`],
114+
env
115+
)
116+
}
100117

101-
const isolatedClientFiles = Array.from(ISOLATED_CLIENT_FILES).map(
102-
(f) => `src/client/__tests__/${f}`
103-
)
104-
await runCommand(
105-
['bun', 'test', ...passthroughArgs, ...isolatedClientFiles],
106-
env
107-
)
118+
for (const file of ISOLATED_CLIENT_FILES) {
119+
await runCommand(
120+
['bun', 'test', ...passthroughArgs, `src/client/__tests__/${file}`],
121+
env
122+
)
123+
}
108124

109125
if (!skipIsolated) {
110126
await runCommand(

0 commit comments

Comments
 (0)