* test(#3144): bound the git/worktree cluster onto the process seam Migrates 180 unbounded sync spawn sites across 19 files. Every previously unbounded call now carries an explicit timeout with a comment giving the number and why. The migration is not a callee swap. execSync and execFileSync throw on a non-zero exit and the seam never does, so each site was classified first: sites that rely on the throw route to gitOrThrow, and sites that already read .status to detect an EXPECTED non-zero -- an intended cherry-pick conflict, a rev-parse outside a repo driving a skip -- route to the never-throwing runGit instead, which would otherwise throw on exactly the exit being probed for. Two same-named git() helpers in worktree-cleanup.test.cjs have different return contracts, one trimmed and one raw; both are preserved rather than unified. Collapses five hand-rolled throw wrappers onto one throwIfFailed in git-fixture.cjs, which gitOrThrow now also uses so the shape cannot drift. Allowlist drops 139 to 120; BASELINE lowered to match. * test(#3144): fix pre-PR review findings Documents throwIfFailed in the CONTEXT.md glossary and CONTRIBUTING.md -- it became the shared throw mechanism without either doc naming it. Routes the sixth and seventh hand-rolled copies of the throw shape through throwIfFailed (worktree-baseref-install, worktree-safety-reap); the first consolidation missed both. Converts ci-rebase-check's 8 fixture-setup calls from unchecked runGit to gitOrThrow so a failed setup step aborts where it fails rather than surfacing later as a confusing failure against the wrong subject. Adds 12 direct unit tests for throwIfFailed, which until now was only exercised transitively. Splits verify.test.cjs's non-git grep/sed bound off GIT_TIMEOUT_MS. --------- Co-authored-by: sim <sim@local>
This commit is contained in:
Tom Boucher
@@ -15,9 +15,12 @@ const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { execSync } = require('child_process');
|
||||
|
||||
const { createTempGitProject, cleanup, runGsdTools } = require('./helpers.cjs');
|
||||
const { gitOrThrow } = require('./helpers/git-fixture.cjs');
|
||||
|
||||
// 15000ms: git plumbing (add/commit/diff/status/rev-list) on a small mkdtemp
|
||||
// fixture repo — far over any observed duration for that class of call.
|
||||
const GIT_TIMEOUT_MS = 15000;
|
||||
|
||||
describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
let tmpDir;
|
||||
@@ -33,7 +36,7 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
test('commit --files does not absorb unrelated staged files', () => {
|
||||
// Developer stages a WIP file via git add (not via --files).
|
||||
fs.writeFileSync(path.join(tmpDir, 'src-wip.txt'), 'work in progress\n');
|
||||
execSync('git add src-wip.txt', { cwd: tmpDir, stdio: 'pipe' });
|
||||
gitOrThrow(['add', 'src-wip.txt'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
|
||||
// GSD writes and commits a planning artifact, naming ONLY that file.
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'PLAN.md'), '# Plan\n');
|
||||
@@ -43,9 +46,9 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
);
|
||||
|
||||
// The commit must contain ONLY .planning/PLAN.md.
|
||||
const diffOutput = execSync('git diff HEAD~1 HEAD --name-only', {
|
||||
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], {
|
||||
cwd: tmpDir,
|
||||
encoding: 'utf-8',
|
||||
timeoutMs: GIT_TIMEOUT_MS,
|
||||
}).trim();
|
||||
assert.strictEqual(
|
||||
diffOutput,
|
||||
@@ -54,9 +57,9 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
);
|
||||
|
||||
// The WIP file must still be staged, not committed.
|
||||
const statusOutput = execSync('git status --porcelain', {
|
||||
const statusOutput = gitOrThrow(['status', '--porcelain'], {
|
||||
cwd: tmpDir,
|
||||
encoding: 'utf-8',
|
||||
timeoutMs: GIT_TIMEOUT_MS,
|
||||
}).trim();
|
||||
assert.ok(
|
||||
statusOutput.includes('A src-wip.txt') || statusOutput.includes('A\tsrc-wip.txt'),
|
||||
@@ -73,9 +76,9 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
tmpDir,
|
||||
);
|
||||
|
||||
const diffOutput = execSync('git diff HEAD~1 HEAD --name-only', {
|
||||
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], {
|
||||
cwd: tmpDir,
|
||||
encoding: 'utf-8',
|
||||
timeoutMs: GIT_TIMEOUT_MS,
|
||||
});
|
||||
const files = diffOutput.trim().split('\n').sort();
|
||||
assert.deepEqual(
|
||||
@@ -88,18 +91,18 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
test('commit without --files still commits the entire .planning/ index (default path)', () => {
|
||||
// Write a planning artifact and stage it.
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'PLAN.md'), '# Plan\n');
|
||||
execSync('git add .planning/PLAN.md', { cwd: tmpDir, stdio: 'pipe' });
|
||||
gitOrThrow(['add', '.planning/PLAN.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
|
||||
// Also stage an unrelated file.
|
||||
fs.writeFileSync(path.join(tmpDir, 'extra.txt'), 'extra\n');
|
||||
execSync('git add extra.txt', { cwd: tmpDir, stdio: 'pipe' });
|
||||
gitOrThrow(['add', 'extra.txt'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
|
||||
runGsdTools(['commit', 'docs: default commit'], tmpDir);
|
||||
|
||||
// Default path (no --files) commits everything staged.
|
||||
const diffOutput = execSync('git diff HEAD~1 HEAD --name-only', {
|
||||
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], {
|
||||
cwd: tmpDir,
|
||||
encoding: 'utf-8',
|
||||
timeoutMs: GIT_TIMEOUT_MS,
|
||||
});
|
||||
const files = diffOutput.trim().split('\n').sort();
|
||||
assert.ok(
|
||||
@@ -111,8 +114,8 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
test('missing tracked file in --files is still not committed as deletion (#2014 guard)', () => {
|
||||
// Create and commit STATE.md, then remove it from disk.
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n');
|
||||
execSync('git add .planning/STATE.md', { cwd: tmpDir, stdio: 'pipe' });
|
||||
execSync('git commit -m "add STATE.md"', { cwd: tmpDir, stdio: 'pipe' });
|
||||
gitOrThrow(['add', '.planning/STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
gitOrThrow(['commit', '-m', 'add STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
fs.unlinkSync(path.join(tmpDir, '.planning', 'STATE.md'));
|
||||
|
||||
// Also create a valid file to commit.
|
||||
@@ -123,9 +126,9 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
tmpDir,
|
||||
);
|
||||
|
||||
const diffOutput = execSync('git diff HEAD~1 HEAD --name-status', {
|
||||
const diffOutput = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-status'], {
|
||||
cwd: tmpDir,
|
||||
encoding: 'utf-8',
|
||||
timeoutMs: GIT_TIMEOUT_MS,
|
||||
});
|
||||
assert.ok(
|
||||
!diffOutput.includes('D\t.planning/STATE.md'),
|
||||
@@ -140,13 +143,13 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
test('commit --files with only missing files returns nothing_to_commit', () => {
|
||||
// Create and commit STATE.md, then remove it from disk.
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), '# State\n');
|
||||
execSync('git add .planning/STATE.md', { cwd: tmpDir, stdio: 'pipe' });
|
||||
execSync('git commit -m "add STATE.md"', { cwd: tmpDir, stdio: 'pipe' });
|
||||
gitOrThrow(['add', '.planning/STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
gitOrThrow(['commit', '-m', 'add STATE.md'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
fs.unlinkSync(path.join(tmpDir, '.planning', 'STATE.md'));
|
||||
|
||||
// Stage an unrelated file so the index is non-empty.
|
||||
fs.writeFileSync(path.join(tmpDir, 'extra.txt'), 'extra\n');
|
||||
execSync('git add extra.txt', { cwd: tmpDir, stdio: 'pipe' });
|
||||
gitOrThrow(['add', 'extra.txt'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS });
|
||||
|
||||
const result = runGsdTools(
|
||||
['commit', 'docs: try', '--files', '.planning/STATE.md'],
|
||||
@@ -164,9 +167,9 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
);
|
||||
|
||||
// The unrelated staged file must still be staged, not committed.
|
||||
const statusOutput = execSync('git status --porcelain', {
|
||||
const statusOutput = gitOrThrow(['status', '--porcelain'], {
|
||||
cwd: tmpDir,
|
||||
encoding: 'utf-8',
|
||||
timeoutMs: GIT_TIMEOUT_MS,
|
||||
}).trim();
|
||||
assert.ok(
|
||||
statusOutput.includes('extra.txt'),
|
||||
@@ -185,7 +188,7 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
assert.strictEqual(parsed.committed, true, `absolute path must commit, not nothing_to_commit: ${res.output}`);
|
||||
|
||||
// The absolute path must land in the commit, normalized to repo-relative.
|
||||
const diff = execSync('git diff HEAD~1 HEAD --name-only', { cwd: tmpDir, encoding: 'utf-8' }).trim();
|
||||
const diff = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS }).trim();
|
||||
assert.strictEqual(diff, '.planning/A.md', `absolute --files path must be committed (normalized to relative); got: ${diff}`);
|
||||
});
|
||||
|
||||
@@ -202,7 +205,7 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
const parsed = JSON.parse(res.output);
|
||||
assert.strictEqual(parsed.committed, true, `mixed list must commit: ${res.output}`);
|
||||
|
||||
const diff = execSync('git diff HEAD~1 HEAD --name-only', { cwd: tmpDir, encoding: 'utf-8' })
|
||||
const diff = gitOrThrow(['diff', 'HEAD~1', 'HEAD', '--name-only'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS })
|
||||
.trim().split('\n').sort();
|
||||
assert.deepStrictEqual(
|
||||
diff,
|
||||
@@ -243,10 +246,10 @@ describe('commit --files: pathspec honors declared scope (#2112)', () => {
|
||||
assert.match(parsed.error, /outside repository/, "git's own rejection message must be preserved (#2608)");
|
||||
|
||||
// No new commit created (still at the single initial commit).
|
||||
const logCount = execSync('git rev-list --count HEAD', { cwd: tmpDir, encoding: 'utf-8' }).trim();
|
||||
const logCount = gitOrThrow(['rev-list', '--count', 'HEAD'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS }).trim();
|
||||
assert.strictEqual(logCount, '1', 'no new commit must be created for an out-of-repo path');
|
||||
// Index stays clean (git add failed → nothing staged).
|
||||
const status = execSync('git status --porcelain', { cwd: tmpDir, encoding: 'utf-8' }).trim();
|
||||
const status = gitOrThrow(['status', '--porcelain'], { cwd: tmpDir, timeoutMs: GIT_TIMEOUT_MS }).trim();
|
||||
assert.strictEqual(status, '', `index must be clean (no pollution): ${status}`);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user