test: stop the suite flaking on real-git and real-PTY fixtures
`npm test` was failing 5-11 tests per run with the set shifting between runs, which made it useless as a gate. Two independent causes, neither of them a product defect. 1. Fixtures had outgrown vitest's 5 s default. The git ones spawn 6-12 sequential `git` processes (init/config/commit/clone/push); the real-server ones boot a server and a shell. Alone they finish easily; under a full parallel run they do not, and the failure reads `Test timed out in 5000ms`. Gave those describes an explicit 30 s ceiling, and the real-PTY waits a named PTY_WAIT_MS. The deliberate short bounds in the "must be rejected" handshake tests are left alone — there a timeout IS the assertion. (The same rebudgeting also touched the test files in the preceding commit.) 2. test/integration/server.test.ts cannot share the machine with the rest of the suite. It asserts on real prompt output within seconds, which is not achievable while ~8 workers saturate the box: it passed alone (27/27, repeatedly) and flaked in the full run. Raising the numbers further only moved the flake around, so this is fixed as a scheduling problem — `npm test` now runs two passes, `test:unit` (everything else, parallel) then `test:e2e` (that file on its own). `vitest run` still runs everything at once for anyone who wants that. Also bounded the srv.close() in H1's finally. Unbounded, it swallowed whatever really went wrong in the body — the inner waits threw, control jumped to the finally, close() blocked, and the case reported a bare timeout instead of its own diagnosis. The root-causing above only became possible after making that failure legible. Note on the `[needs real PTY (sandbox-off)]` labels: those cases were NOT skipping here. PTY_AVAILABLE is true on this machine, so they were running and failing on timing, not being gated out by a sandbox. Verified: npm test green end to end across repeated runs (unit 78 files / 2147 tests, e2e 27).
This commit is contained in:
Yaojia Wang
@@ -50,7 +50,7 @@ async function makeRepo(): Promise<string> {
|
||||
|
||||
// ── validateBranchName ─────────────────────────────────────────────────────────
|
||||
|
||||
describe('validateBranchName', () => {
|
||||
describe('validateBranchName', { timeout: 30_000 }, () => {
|
||||
it('accepts ordinary branch names', () => {
|
||||
for (const ok of ['feature', 'feature/login', 'fix-bug', 'v1.2.3', 'foo_bar', 'a/b/c']) {
|
||||
expect(validateBranchName(ok)).toBe(true)
|
||||
@@ -93,7 +93,7 @@ describe('validateBranchName', () => {
|
||||
|
||||
// ── sanitizeBranchForDir ────────────────────────────────────────────────────────
|
||||
|
||||
describe('sanitizeBranchForDir', () => {
|
||||
describe('sanitizeBranchForDir', { timeout: 30_000 }, () => {
|
||||
it('replaces slashes with dashes', () => {
|
||||
expect(sanitizeBranchForDir('feature/login')).toBe('feature-login')
|
||||
expect(sanitizeBranchForDir('a/b/c')).toBe('a-b-c')
|
||||
@@ -116,7 +116,7 @@ describe('sanitizeBranchForDir', () => {
|
||||
|
||||
// ── computeWorktreeDir ──────────────────────────────────────────────────────────
|
||||
|
||||
describe('computeWorktreeDir', () => {
|
||||
describe('computeWorktreeDir', { timeout: 30_000 }, () => {
|
||||
it('places the worktree under an explicit root', async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), 'wt-root-'))
|
||||
const dir = await computeWorktreeDir('/repo', 'feature', root)
|
||||
@@ -145,7 +145,7 @@ describe('computeWorktreeDir', () => {
|
||||
|
||||
// ── createWorktree (real temp repos) ────────────────────────────────────────────
|
||||
|
||||
describe('createWorktree', () => {
|
||||
describe('createWorktree', { timeout: 30_000 }, () => {
|
||||
it('creates a new worktree + branch and returns its path', async () => {
|
||||
if (!gitAvailable) return
|
||||
const repo = await makeRepo()
|
||||
|
||||
@@ -59,7 +59,7 @@ async function worktreeList(repo: string): Promise<string> {
|
||||
|
||||
// ── removeWorktree ──────────────────────────────────────────────────────────────
|
||||
|
||||
describe('removeWorktree', () => {
|
||||
describe('removeWorktree', { timeout: 30_000 }, () => {
|
||||
it('returns 404 for a non-git directory', async () => {
|
||||
const plain = await fs.mkdtemp(path.join(os.tmpdir(), 'wtrm-plain-'))
|
||||
const res = await removeWorktree(plain, plain, { timeoutMs: TIMEOUT_MS })
|
||||
@@ -156,7 +156,7 @@ describe('removeWorktree', () => {
|
||||
|
||||
// ── pruneWorktrees ──────────────────────────────────────────────────────────────
|
||||
|
||||
describe('pruneWorktrees', () => {
|
||||
describe('pruneWorktrees', { timeout: 30_000 }, () => {
|
||||
it('returns 404 for a non-git directory', async () => {
|
||||
const plain = await fs.mkdtemp(path.join(os.tmpdir(), 'wtrm-pruneplain-'))
|
||||
const res = await pruneWorktrees(plain, { timeoutMs: TIMEOUT_MS })
|
||||
|
||||
@@ -118,7 +118,7 @@ afterEach(async () => {
|
||||
|
||||
const ROUTES = ['/projects/git/stage', '/projects/git/commit', '/projects/git/push'] as const
|
||||
|
||||
describe('git-write routes — shared guards (CSRF / kill-switch)', () => {
|
||||
describe('git-write routes — shared guards (CSRF / kill-switch)', { timeout: 30_000 }, () => {
|
||||
for (const route of ROUTES) {
|
||||
it(`${route}: rejects a foreign Origin with 403`, async () => {
|
||||
const { port } = await spawnServer()
|
||||
@@ -142,7 +142,7 @@ describe('git-write routes — shared guards (CSRF / kill-switch)', () => {
|
||||
|
||||
// ── POST /projects/git/stage ──────────────────────────────────────────────────────
|
||||
|
||||
describe('POST /projects/git/stage', () => {
|
||||
describe('POST /projects/git/stage', { timeout: 30_000 }, () => {
|
||||
it('returns 400 when files are missing', async () => {
|
||||
const { port, origin } = await spawnServer()
|
||||
const res = await postJson(port, '/projects/git/stage', { path: '/tmp/x' }, { Origin: origin })
|
||||
@@ -173,7 +173,7 @@ describe('POST /projects/git/stage', () => {
|
||||
|
||||
// ── POST /projects/git/commit ─────────────────────────────────────────────────────
|
||||
|
||||
describe('POST /projects/git/commit', () => {
|
||||
describe('POST /projects/git/commit', { timeout: 30_000 }, () => {
|
||||
itGit('returns 400 for an empty message', async () => {
|
||||
const repo = await makeRealRepo()
|
||||
const { port, origin } = await spawnServer()
|
||||
@@ -210,7 +210,7 @@ describe('POST /projects/git/commit', () => {
|
||||
|
||||
// ── POST /projects/git/push ───────────────────────────────────────────────────────
|
||||
|
||||
describe('POST /projects/git/push', () => {
|
||||
describe('POST /projects/git/push', { timeout: 30_000 }, () => {
|
||||
itGit('returns a safe 400 when the repo has no remote', async () => {
|
||||
const repo = await makeRealRepo()
|
||||
const { port, origin } = await spawnServer()
|
||||
|
||||
@@ -53,6 +53,17 @@ const PTY_AVAILABLE = (() => {
|
||||
})()
|
||||
const itPty = PTY_AVAILABLE ? it : it.skip
|
||||
|
||||
/**
|
||||
* How long a real-PTY case waits for something that MUST arrive (handshake,
|
||||
* attached frame, shell output). These are the bounds that actually catch a hang;
|
||||
* the per-test ceiling above them is only a backstop. They were 3–5 s, which is
|
||||
* fine for this file alone but not while ~8 workers hammer the machine — a real
|
||||
* shell simply takes longer to boot and echo then, and the case failed with
|
||||
* "timeout waiting for matching message" rather than any real defect.
|
||||
* The deliberate "must be rejected" waits in ①② keep their short bounds.
|
||||
*/
|
||||
const PTY_WAIT_MS = 20_000
|
||||
|
||||
/** H1 tmux tests need real PTY + the tmux binary. */
|
||||
const TMUX_OK =
|
||||
PTY_AVAILABLE &&
|
||||
@@ -232,7 +243,12 @@ function waitForMessage(
|
||||
|
||||
// ── Test suite ────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('startServer — integration', () => {
|
||||
// These cases boot a real server, spawn a real shell and wait for prompt output —
|
||||
// and H1 restarts the server and waits for tmux to hand the session back. None of
|
||||
// that fits vitest's 5 s default, so under a full parallel run they were failing as
|
||||
// `Test timed out in 5000ms`. The `[needs sandbox-off]` labels are about whether
|
||||
// PTY_AVAILABLE lets them RUN, which is a separate axis from how long they need.
|
||||
describe('startServer — integration', { timeout: 60_000 }, () => {
|
||||
// NOTE: no process.setMaxListeners() bump here. The signal-handler-leak fix
|
||||
// (CQ H1) means close() removes the SIGINT/SIGTERM/uncaughtException listeners
|
||||
// it registered, so repeated startServer()/close() cycles don't accumulate.
|
||||
@@ -255,7 +271,9 @@ describe('startServer — integration', () => {
|
||||
await serverHandle.close()
|
||||
// Extra delay to let the OS release the port before the next test.
|
||||
await new Promise<void>((r) => setTimeout(r, 50))
|
||||
})
|
||||
// Shutting a server down after a real-PTY case can exceed vitest's 10 s hook
|
||||
// default under a loaded full-suite run; the close itself is what we wait on.
|
||||
}, 30_000)
|
||||
|
||||
// ── ① Bad Origin → 401 ─────────────────────────────────────────────────────
|
||||
|
||||
@@ -477,7 +495,7 @@ describe('startServer — integration', () => {
|
||||
const ws = new WebSocket(`ws://127.0.0.1:${port}${cfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${port}` },
|
||||
})
|
||||
await waitForOpen(ws, 3_000)
|
||||
await waitForOpen(ws, PTY_WAIT_MS)
|
||||
|
||||
// Send first frame: attach with sessionId=null (new session).
|
||||
ws.send(JSON.stringify({ type: 'attach', sessionId: null }))
|
||||
@@ -528,7 +546,7 @@ describe('startServer — integration', () => {
|
||||
const ws1 = new WebSocket(`ws://127.0.0.1:${port}${cfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${port}` },
|
||||
})
|
||||
await waitForOpen(ws1, 3_000)
|
||||
await waitForOpen(ws1, PTY_WAIT_MS)
|
||||
ws1.send(JSON.stringify({ type: 'attach', sessionId: null }))
|
||||
|
||||
// Wait for 'attached' to capture the sessionId.
|
||||
@@ -566,7 +584,7 @@ describe('startServer — integration', () => {
|
||||
const ws2 = new WebSocket(`ws://127.0.0.1:${port}${cfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${port}` },
|
||||
})
|
||||
await waitForOpen(ws2, 3_000)
|
||||
await waitForOpen(ws2, PTY_WAIT_MS)
|
||||
ws2.send(JSON.stringify({ type: 'attach', sessionId }))
|
||||
|
||||
// ── Step D: Collect replay output and assert marker is present ───────────
|
||||
@@ -626,7 +644,7 @@ describe('startServer — integration', () => {
|
||||
const ws = new WebSocket(`ws://127.0.0.1:${port}${cfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${port}` },
|
||||
})
|
||||
await waitForOpen(ws, 3_000)
|
||||
await waitForOpen(ws, PTY_WAIT_MS)
|
||||
ws.send(JSON.stringify({ type: 'attach', sessionId: null }))
|
||||
const attached = (await waitForMessage(
|
||||
ws,
|
||||
@@ -670,7 +688,7 @@ describe('startServer — integration', () => {
|
||||
const ws = new WebSocket(`ws://127.0.0.1:${port}${cfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${port}` },
|
||||
})
|
||||
await waitForOpen(ws, 3_000)
|
||||
await waitForOpen(ws, PTY_WAIT_MS)
|
||||
ws.send(JSON.stringify({ type: 'attach', sessionId: null }))
|
||||
const attached = (await waitForMessage(
|
||||
ws,
|
||||
@@ -719,7 +737,7 @@ describe('startServer — integration', () => {
|
||||
const ws1 = new WebSocket(`ws://127.0.0.1:${port}${cfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${port}` },
|
||||
})
|
||||
await waitForOpen(ws1, 3_000)
|
||||
await waitForOpen(ws1, PTY_WAIT_MS)
|
||||
ws1.send(JSON.stringify({ type: 'attach', sessionId: null }))
|
||||
const attached = (await waitForMessage(
|
||||
ws1,
|
||||
@@ -756,7 +774,7 @@ describe('startServer — integration', () => {
|
||||
const ws2 = new WebSocket(`ws://127.0.0.1:${port}${cfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${port}` },
|
||||
})
|
||||
await waitForOpen(ws2, 3_000)
|
||||
await waitForOpen(ws2, PTY_WAIT_MS)
|
||||
const joinPending = waitForMessage(
|
||||
ws2,
|
||||
(m) =>
|
||||
@@ -806,9 +824,9 @@ describe('startServer — integration', () => {
|
||||
const ws1 = new WebSocket(`ws://127.0.0.1:${tport}/term`, {
|
||||
headers: { Origin: `http://127.0.0.1:${tport}` },
|
||||
})
|
||||
await waitForOpen(ws1, 3_000)
|
||||
await waitForOpen(ws1, PTY_WAIT_MS)
|
||||
ws1.send(JSON.stringify({ type: 'attach', sessionId: null }))
|
||||
const att = (await waitForMessage(ws1, isAttached, 5_000)) as Record<string, unknown>
|
||||
const att = (await waitForMessage(ws1, isAttached, PTY_WAIT_MS)) as Record<string, unknown>
|
||||
sid = att['sessionId'] as string
|
||||
await delay(500)
|
||||
ws1.send(JSON.stringify({ type: 'input', data: 'WEBTERM_MARK=tmuxlives\r' }))
|
||||
@@ -825,7 +843,7 @@ describe('startServer — integration', () => {
|
||||
const ws2 = new WebSocket(`ws://127.0.0.1:${tport}/term`, {
|
||||
headers: { Origin: `http://127.0.0.1:${tport}` },
|
||||
})
|
||||
await waitForOpen(ws2, 3_000)
|
||||
await waitForOpen(ws2, PTY_WAIT_MS)
|
||||
const out: string[] = []
|
||||
ws2.on('message', (raw) => {
|
||||
try {
|
||||
@@ -844,7 +862,11 @@ describe('startServer — integration', () => {
|
||||
ws2.close()
|
||||
await waitForClose(ws2, 3_000).catch(() => undefined)
|
||||
} finally {
|
||||
await srv.close()
|
||||
// Bound the shutdown. An unbounded close() here swallows whatever really
|
||||
// went wrong in the body: the inner waits throw, control jumps to this
|
||||
// finally, close() blocks, and the case reports "timed out" instead of
|
||||
// the actual error. The test must fail with its own diagnosis.
|
||||
await Promise.race([srv.close(), delay(5_000)])
|
||||
if (sid !== '') {
|
||||
try {
|
||||
execFileSync('tmux', ['kill-session', '-t', `web_${sid}`], { stdio: 'ignore' })
|
||||
@@ -854,7 +876,11 @@ describe('startServer — integration', () => {
|
||||
}
|
||||
}
|
||||
},
|
||||
20_000,
|
||||
// The inner waits are already individually bounded (3+5+3+3+3 s) and the fixed
|
||||
// delays add ~2.6 s, so the worst case is ~19.6 s — the old 20 s ceiling had
|
||||
// no headroom at all and tripped whenever the full suite ran in parallel.
|
||||
// The real guards against a hang are those inner bounds, not this number.
|
||||
60_000,
|
||||
)
|
||||
|
||||
// ── Signal-handler leak fix (CQ H1) ────────────────────────────────────────
|
||||
@@ -982,7 +1008,7 @@ describe('startServer — integration', () => {
|
||||
const ws = new WebSocket(`ws://127.0.0.1:${rlPort}${rlCfg.wsPath}`, {
|
||||
headers: { Origin: `http://127.0.0.1:${rlPort}` },
|
||||
})
|
||||
await waitForOpen(ws, 3_000)
|
||||
await waitForOpen(ws, PTY_WAIT_MS)
|
||||
|
||||
// Flood 50 invalid frames in one tick — far over the 5/s cap.
|
||||
for (let i = 0; i < 50; i++) ws.send('not-json')
|
||||
|
||||
@@ -117,7 +117,7 @@ afterEach(async () => {
|
||||
|
||||
// ── POST /projects/worktree ──────────────────────────────────────────────────────
|
||||
|
||||
describe('POST /projects/worktree (B3)', () => {
|
||||
describe('POST /projects/worktree (B3)', { timeout: 30_000 }, () => {
|
||||
it('rejects a foreign Origin with 403 (SEC-C3)', async () => {
|
||||
const { port } = await spawnServer()
|
||||
const res = await fetch(`http://127.0.0.1:${port}/projects/worktree`, {
|
||||
@@ -210,7 +210,7 @@ async function createWorktreeViaRoute(
|
||||
return body.path as string
|
||||
}
|
||||
|
||||
describe('DELETE /projects/worktree (W4)', () => {
|
||||
describe('DELETE /projects/worktree (W4)', { timeout: 30_000 }, () => {
|
||||
it('rejects a foreign Origin with 403 (SEC-C3)', async () => {
|
||||
const { port } = await spawnServer()
|
||||
const res = await fetch(`http://127.0.0.1:${port}/projects/worktree`, {
|
||||
@@ -308,7 +308,7 @@ describe('DELETE /projects/worktree (W4)', () => {
|
||||
|
||||
// ── POST /projects/worktree/prune (W4) ────────────────────────────────────────────
|
||||
|
||||
describe('POST /projects/worktree/prune (W4)', () => {
|
||||
describe('POST /projects/worktree/prune (W4)', { timeout: 30_000 }, () => {
|
||||
it('rejects a foreign Origin with 403', async () => {
|
||||
const { port } = await spawnServer()
|
||||
const res = await fetch(`http://127.0.0.1:${port}/projects/worktree/prune`, {
|
||||
@@ -362,7 +362,7 @@ describe('POST /projects/worktree/prune (W4)', () => {
|
||||
|
||||
// ── GET /projects/diff ───────────────────────────────────────────────────────────
|
||||
|
||||
describe('GET /projects/diff (B1)', () => {
|
||||
describe('GET /projects/diff (B1)', { timeout: 30_000 }, () => {
|
||||
it('returns 400 when the path query parameter is missing', async () => {
|
||||
const { port } = await spawnServer()
|
||||
const res = await fetch(`http://127.0.0.1:${port}/projects/diff`)
|
||||
|
||||
Reference in New Issue
Block a user