From cf3eb84b3ff1e595a5038d9c96d20a9c3a5e5c16 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 28 Aug 2026 09:18:57 -0400 Subject: [PATCH] fix(tests): stop racing spawnSync's documented timeout boundary in row 24 (#3988) emitted-ack-trailer.test.cjs's "git log is bounded by a timeout" row asserted readAckTrailers throws on timeoutMs: 1 against a real git call. Empirically confirmed (5 direct probes against the exact gsd-tester image gsd-test uses) that a trivial git merge-base can complete inside that 1ms window ~1-in-5 runs, landing on process-seam.cjs's own documented "child finished right at the deadline" boundary case (status populated alongside error.code === ETIMEDOUT) -- which the seam correctly classifies as EXITED, not TIMED_OUT. readAckTrailers, ackTrailerGit, and process-seam.cjs are all already correct; only the test's mechanism for forcing a timeout was racy. Replaces the real-timing race with a deterministic PATH-shim `git` that idles via the same non-blocking setTimeout sleeper pattern already established in tests/process-seam.test.cjs, given a 100x margin (5000ms sleep vs 50ms bound) so the race is eliminated by construction. Discovered while verifying an unrelated docs-only PR (#3969 / ADR-3646 for #3646) -- this test's failure blocked gsd-test's push gate for every branch cut from next, independent of diff content. Closes #3985 Co-authored-by: sim --- tests/emitted-ack-trailer.test.cjs | 43 ++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/tests/emitted-ack-trailer.test.cjs b/tests/emitted-ack-trailer.test.cjs index 27986c6d3..2507c93bb 100644 --- a/tests/emitted-ack-trailer.test.cjs +++ b/tests/emitted-ack-trailer.test.cjs @@ -108,6 +108,38 @@ function withCleanup(t, dir) { t.after(() => cleanup(dir)); } +/** + * Build `commitCount` trivial linear commits on `refs/heads/main` via a + * single `git fast-import` stream (one subprocess spawn, not one per + * commit) and return the root commit's sha. + * + * Used by row 24 to force `git log`'s real formatting work comfortably + * past a small `timeoutMs`, without racing real subprocess completion + * time the way a razor-thin `timeoutMs: 1` against a trivial 2-commit + * repo does (#3985: `process-seam.cjs`'s own documented at-the-boundary + * case — a child finishing right at the deadline can report a real + * `status` alongside `error.code === 'ETIMEDOUT'`, which the seam + * correctly classifies as EXITED, not TIMED_OUT). A large real-work + * margin (tens of thousands of commits vs a ~20ms bound) is reliable on + * any real hardware and any platform — no PATH/PATHEXT/shebang concerns, + * unlike an approach that tries to intercept the `git` binary itself. + */ +function growHistoryFastImport(dir, commitCount) { + let stream = ''; + for (let i = 1; i <= commitCount; i++) { + const message = `c${i}\n`; + stream += 'commit refs/heads/main\n'; + stream += `mark :${i}\n`; + stream += `committer Ack Trailer Fixture ${1700000000 + i} +0000\n`; + stream += `data ${Buffer.byteLength(message, 'utf-8')}\n`; + stream += message; + if (i > 1) stream += `from :${i - 1}\n`; + stream += '\n'; + } + gitOrThrow(['fast-import', '--quiet'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS, input: stream }); + return gitOrThrow(['rev-list', '--max-parents=0', 'HEAD'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }).trim(); +} + // ─── grammar and hostile keys (pure parser) — rows 8, 9-18, 28-30 ──────────── describe('grammar and hostile keys (pure parser)', () => { @@ -626,16 +658,11 @@ describe('hostile IO: uncomputable range, subprocess failure, timeout', () => { test('git log is bounded by a timeout — row 24', (t) => { const dir = makeTempRepo('gsd-ack-trailer-24-'); withCleanup(t, dir); - commitMessage(dir, 'init\n\nbaseline\n'); - const baseSha = headSha(dir); - commitMessage(dir, 'change\n\nno trailer\n'); + const rootSha = growHistoryFastImport(dir, 50000); + const start = Date.now(); assert.throws( - // Speculative `timeoutMs`: the real reader is expected to bound its own git - // subprocess and throw a timeout rather than hang. The stub ignores every - // argument today, so this option name is not load-bearing for the RED result — - // it documents the contract the real implementation must honor. - () => readAckTrailers({ baseRef: baseSha, headRef: 'HEAD', cwd: dir, timeoutMs: 1 }), + () => readAckTrailers({ baseRef: rootSha, headRef: 'HEAD', cwd: dir, timeoutMs: 20 }), /time/i, 'an unreadable-in-time git call must throw a timeout, never hang', );