golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

test(#4513): migrate git plumbing batch to named timeout constants

Browse Source 
Batch 2 of the ad hoc timeout literal migration (epic #4445). Replaces
every bare numeric timeout/timeoutMs object-literal property in
tests/git-base-branch.test.cjs, tests/commit-files-pathspec.test.cjs,
and tests/git-fixture.test.cjs with a named constant, per
eslint-rules/no-adhoc-timeout-literal.cjs. Removes the 3 files from
the rule's allowlist.

This batch introduces a violation shape not seen in Batch 1: several
sites are pinned-value test assertions verifying the EXACT timeout
production code hardcodes (not bounds on this suite's own subprocess
calls). Named as four separate constants even where values coincide,
so the tests keep catching independent production drift instead of
silently tolerating it. No src/bin file touched, no numeric value
changed anywhere -- verified site-by-site by two independent isolated
review passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
sim
2026-09-08 19:56:08 -04:00
parent ff5c5782ee
commit 95e6a58fd4
4 changed files with 79 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
eslint-rules/no-adhoc-timeout-literal.allowlist.json
View File

@@ -26,7 +26,6 @@
"tests/code-review-pipeline-regression.test.cjs",
"tests/code-review.test.cjs",
"tests/commands.test.cjs",
"tests/commit-files-pathspec.test.cjs",
"tests/config-get-default.test.cjs",
"tests/cursor-hook-workspace-roots.test.cjs",
"tests/cursor-hooks.test.cjs",
@@ -46,8 +45,6 @@
"tests/gen-health-docs.test.cjs",
"tests/gen-section-manifest.test.cjs",
"tests/gen-state-md-docs.test.cjs",
"tests/git-base-branch.test.cjs",
"tests/git-fixture.test.cjs",
"tests/graphify.test.cjs",
"tests/gsd-check-update-worker-platform-gate.test.cjs",
"tests/gsd-mcp-server-bin.test.cjs",

35
tests/commit-files-pathspec.test.cjs
View File

@@ -22,7 +22,7 @@ const { createTempGitProject, cleanup, runGsdTools } = require('./helpers.cjs');
const { execFileSync } = require('node:child_process');
const { gitOrThrow } = require('./helpers/git-fixture.cjs');
// #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs.
const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const { GIT_TIMEOUT_MS, PROBE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
const {
bareCommandName, tokenize, shellDashCPayloads, commentPortion,
ISSUE_REF_RE, declarationReason, isDeclared, isUntrackedDeclaration,
@@ -433,6 +433,16 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
// shared DEFAULT_GIT_TIMEOUT_MS norm.
const STAGING_GIT_TIMEOUT_MS = 5000;
/**
* `commitWithFailingAdd`/`subrepoCommitWithFailingAdd` below spawn
* `process.execPath -e <script>` where the script mocks `execGit` in-process
* (no real git subprocess runs) and calls `cmdCommit`/`cmdCommitToSubrepo`.
* Not the same class as `PROBE_TIMEOUT_MS` (15000ms) below — kept at its
* pre-existing, more generous value rather than lowered without a bench
* citation proving 15000ms is safe for this harness.
*/
const MOCKED_GIT_SPAWN_TIMEOUT_MS = 30000;
const LIB = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib');
/**
@@ -488,7 +498,7 @@ cmdCommit(${JSON.stringify(cwd)}, 'docs: map existing codebase', ${JSON.stringif
const run = spawnSync(process.execPath, ['-e', script], {
encoding: 'utf8',
timeout: 30000,
timeout: MOCKED_GIT_SPAWN_TIMEOUT_MS,
killSignal: 'SIGKILL',
env: { ...process.env, GSD_TEST_MODE: '1' },
});
@@ -548,7 +558,7 @@ cmdCommitToSubrepo(${JSON.stringify(cwd)}, 'feat: subrepo change', ${JSON.string
`;
const run = spawnSync(process.execPath, ['-e', script], {
encoding: 'utf8',
timeout: 30000,
timeout: MOCKED_GIT_SPAWN_TIMEOUT_MS,
killSignal: 'SIGKILL',
env: { ...process.env, GSD_TEST_MODE: '1' },
});
@@ -3195,7 +3205,7 @@ projection.execGit = (args, opts) => {
};
cmdCommit(${JSON.stringify(cwd)}, 'docs: probe', ${JSON.stringify(files)}, false, false, false);
`;
const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: 15_000 });
const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: PROBE_TIMEOUT_MS });
if (run.status !== 0 && !run.stdout) {
throw new Error(`probe crashed: ${run.stderr}`);
}
@@ -3207,9 +3217,16 @@ cmdCommit(${JSON.stringify(cwd)}, 'docs: probe', ${JSON.stringify(files)}, false
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3886-'));
fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true });
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n');
execFileSync('git', ['init', '-b', 'main'], { cwd: tmpDir, timeout: 15_000 });
execFileSync('git', ['config', 'user.email', 't@example.com'], { cwd: tmpDir, timeout: 15_000 });
execFileSync('git', ['config', 'user.name', 'T'], { cwd: tmpDir, timeout: 15_000 });
// `GIT_TIMEOUT_MS` (15000ms, "plumbing READS") rather than the heavier
// `GIT_FIXTURE_TIMEOUT_MS` (60000ms, "fixture CONSTRUCTION calls —
// init/config/add/commit") that helpers/git-fixture.cjs's own docstring
// says this exact call shape belongs to. Not reclassified: doing so would
// RAISE this site's bound 15000ms -> 60000ms with no bench citation
// proving the smaller value is unsafe here — out of scope for a
// rename-only migration. Flagged, not silently resolved either way.
execFileSync('git', ['init', '-b', 'main'], { cwd: tmpDir, timeout: GIT_TIMEOUT_MS });
execFileSync('git', ['config', 'user.email', 't@example.com'], { cwd: tmpDir, timeout: GIT_TIMEOUT_MS });
execFileSync('git', ['config', 'user.name', 'T'], { cwd: tmpDir, timeout: GIT_TIMEOUT_MS });
});
afterEach(() => cleanup(tmpDir));
@@ -3247,7 +3264,7 @@ projection.execGit = (args, opts) => {
};
cmdCommit(${JSON.stringify(tmpDir)}, 'docs: probe', ['.planning/STATE.md'], false, false, false);
`;
const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: 15_000 });
const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: PROBE_TIMEOUT_MS });
const result = JSON.parse(run.stdout);
assert.equal(result.reason, 'commit_timeout', 'the timeout gate must win over the nothing-to-commit text in partial output');
assert.equal(result.timed_out, true);
@@ -3268,7 +3285,7 @@ projection.execGit = (args, opts) => {
};
cmdCommit(${JSON.stringify(tmpDir)}, 'docs: probe', ['.planning/STATE.md'], false, false, false);
`;
const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: 15_000 });
const run = spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8', timeout: PROBE_TIMEOUT_MS });
const result = JSON.parse(run.stdout);
assert.equal(result.committed, false);
assert.equal(result.reason, 'commit_failed');

55
tests/git-base-branch.test.cjs
View File

@@ -38,6 +38,33 @@ const { runHook } = require('./helpers/process-seam.cjs');
// #3145: class-norm timeout, not a per-suite value — see helpers/timeouts.cjs.
const { GIT_TIMEOUT_MS, HOOK_FANOUT_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
/**
* The #3819 executor pre-commit protected-branch guard test harness below
* invokes a bash script with fake `git`/`gsd_run` shell functions defined
* INLINE — no real subprocess fan-out happens, so this is lighter than
* `HOOK_FANOUT_TIMEOUT_MS` (60000ms, sized for a hook that fans out to
* nested shell spawns). Pre-existing value, unchanged by this migration.
*/
const EXECUTOR_GUARD_TIMEOUT_MS = 10000;
/**
* These four constants are NOT bounds on this suite's own subprocess calls —
* they are the EXACT timeout values production code (gitBaseBranch's
* trySymbolicRef / tryRemoteShow / tryLocalBranch / gitWorktreeInfoInternal,
* in src/, out of this batch's scope) hardcodes when calling its own git
* execution callback. Each test below asserts the captured call args to
* prove production didn't drift from its documented bound. Kept as FOUR
* separate constants rather than consolidated, even where values coincide
* today (TIER2 and TIER4 are both 5000ms) — collapsing them would let two
* independently-implemented production tiers drift from each other while
* the test kept passing, which defeats the point of a pinned-value
* assertion. All four are pre-existing values, unchanged by this migration.
*/
const TIER2_SYMBOLIC_REF_TIMEOUT_MS = 5000;
const TIER3_REMOTE_SHOW_TIMEOUT_MS = 15000;
const TIER4_LOCAL_BRANCH_TIMEOUT_MS = 5000;
const WORKTREE_INFO_TIMEOUT_MS = 5000;
// ─── helpers ──────────────────────────────────────────────────────────────────
/**
@@ -1267,7 +1294,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 1, result.stderr);
assert.match(result.stderr, /protected\/default branch/);
@@ -1283,7 +1310,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 0, result.stderr);
assert.match(result.stdout, /GUARD_PASSED/);
@@ -1298,7 +1325,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 1, result.stderr);
assert.match(result.stderr, /protected\/default branch/);
@@ -1313,7 +1340,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 1, result.stderr);
assert.match(result.stderr, /not in the agent-\* \/ worktree-agent-\* \/ worktree-wf_\* namespace/);
@@ -1328,7 +1355,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 0, result.stderr);
assert.match(result.stdout, /GUARD_PASSED/);
@@ -1343,7 +1370,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 1, result.stderr);
assert.match(result.stderr, /detached/);
@@ -1374,7 +1401,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 1, result.stderr);
assert.match(result.stderr, /protected\/default branch/);
@@ -1391,7 +1418,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 1, result.stderr);
assert.match(result.stderr, /protected\/default branch/);
@@ -1412,7 +1439,7 @@ describe('#3819: gsd-executor.md pre-commit protected-branch guard', () => {
});
t.after(() => cleanup(scriptDir));
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: 10000 });
const result = runHook(scriptPath, [], { interpreter: 'bash', cwd: scriptDir, timeoutMs: EXECUTOR_GUARD_TIMEOUT_MS });
assert.strictEqual(result.exitCode, 0, result.stderr);
assert.match(result.stdout, /GUARD_PASSED/);
@@ -1896,7 +1923,7 @@ describe('#3057 W3: trySymbolicRef — tier-2 output that resolves to nothing',
});
assert.deepStrictEqual(seen, [{
args: ['symbolic-ref', '--quiet', '--short', 'refs/remotes/origin/HEAD'],
opts: { cwd: '/some/cwd', timeout: 5_000 },
opts: { cwd: '/some/cwd', timeout: TIER2_SYMBOLIC_REF_TIMEOUT_MS },
}]);
});
});
@@ -1953,7 +1980,7 @@ describe('#3057 W3: tryRemoteShow — tier-3 output that is present but not auth
});
assert.deepStrictEqual(seen, [{
args: ['remote', 'show', 'origin'],
opts: { cwd: '/some/cwd', timeout: 15_000 },
opts: { cwd: '/some/cwd', timeout: TIER3_REMOTE_SHOW_TIMEOUT_MS },
}]);
});
});
@@ -2006,7 +2033,7 @@ describe('#3057 W3: tryLocalBranch — non-empty stdout that names neither main
});
assert.deepStrictEqual(seen, [{
args: ['branch', '--list', 'main', 'master'],
opts: { cwd: '/some/cwd', timeout: 5_000 },
opts: { cwd: '/some/cwd', timeout: TIER4_LOCAL_BRANCH_TIMEOUT_MS },
}]);
});
});
@@ -2184,8 +2211,8 @@ describe('#3057 W3: gitWorktreeInfoInternal — no work tree, and git failing mi
});
gitBaseBranch.gitWorktreeInfoInternal('/some/cwd', { execGit: git });
assert.deepStrictEqual(git.calls, [
{ args: ['rev-parse', '--is-inside-work-tree'], opts: { cwd: '/some/cwd', timeout: 5000 } },
{ args: ['rev-parse', '--show-toplevel'], opts: { cwd: '/some/cwd', timeout: 5000 } },
{ args: ['rev-parse', '--is-inside-work-tree'], opts: { cwd: '/some/cwd', timeout: WORKTREE_INFO_TIMEOUT_MS } },
{ args: ['rev-parse', '--show-toplevel'], opts: { cwd: '/some/cwd', timeout: WORKTREE_INFO_TIMEOUT_MS } },
]);
});
});

15
tests/git-fixture.test.cjs
View File

@@ -31,6 +31,15 @@ const runGitSpy = mock.method(processSeam, 'runGit');
after(() => mock.restoreAll());
const { gitOrThrow, throwIfFailed, toLegacyResult, DEFAULT_GIT_TIMEOUT_MS } = require('./helpers/git-fixture.cjs');
/**
* Deliberately NOT DEFAULT_GIT_TIMEOUT_MS or any real class norm — chosen
* specifically to differ from it, to prove gitOrThrow's timeoutMs override
* plumbing actually threads a caller-supplied value through rather than
* silently keeping the default. See the E11 test's own
* assert.notEqual(..., DEFAULT_GIT_TIMEOUT_MS) a few lines below.
*/
const ARBITRARY_OVERRIDE_TIMEOUT_MS = 12345;
const { createTempDir, cleanup } = require('./helpers.cjs');
/**
@@ -161,10 +170,10 @@ describe('git-fixture: E — gitOrThrow', () => {
test('E11: explicit timeoutMs overrides the default', () => {
runGitSpy.mock.resetCalls();
gitOrThrow(['--version'], { cwd: dir, timeoutMs: 12345 });
gitOrThrow(['--version'], { cwd: dir, timeoutMs: ARBITRARY_OVERRIDE_TIMEOUT_MS });
assert.equal(runGitSpy.mock.calls.length, 1);
assert.equal(runGitSpy.mock.calls[0].arguments[1].timeoutMs, 12345);
assert.notEqual(12345, DEFAULT_GIT_TIMEOUT_MS);
assert.equal(runGitSpy.mock.calls[0].arguments[1].timeoutMs, ARBITRARY_OVERRIDE_TIMEOUT_MS);
assert.notEqual(ARBITRARY_OVERRIDE_TIMEOUT_MS, DEFAULT_GIT_TIMEOUT_MS);
});
test('E12: shell-string args are rejected', () => {