cleanup() computed `isTmpPath` — the exact predicate for "is this path safe to delete" — and consulted it only inside the catch block, to classify a transient Windows error. The rmSync above it ran unconditionally against whatever path it was handed, with `recursive: true, force: true`. The guard existed and the destructive call never asked it. The chdir at the top of the function makes the failure mode worse rather than better: a wrong target first moves the process out of the tree, then deletes it. So the predicate moves above both, and an out-of-tmpdir path is refused before either can run. The refusal throws and names the path; returning quietly would reproduce the fail-open shape this wave exists to remove. The catch keeps its `&& isTmpPath` term. It is now always true, but it states the condition the swallow depends on rather than inheriting it from a check twenty lines up, and it stays correct if the guard is ever relaxed. All 300+ call sites resolve under os.tmpdir() today, including the two files that override TMPDIR — both create their override root through the real os.tmpdir() first — so nothing legitimate is refused. The regression test targets tests/ itself: a real directory that must never be deleted, so nothing is created and nothing needs tearing down. It asserts the throw names the path, that cwd is unchanged (the chdir hazard), and that a known file inside still exists — proving the directory was not emptied rather than merely still present. Whether tests/ is outside os.tmpdir() is environment-dependent: os.tmpdir() is /tmp on Linux, so a checkout under /tmp would put it inside, and there a correct guard would delete this directory rather than refuse it. The test asserts that precondition before calling cleanup, so that environment fails loudly instead of destructively. Refs #3057 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
162 lines
6.4 KiB
JavaScript
162 lines
6.4 KiB
JavaScript
/**
|
||
* GSD Tools Test Helpers – cleanup() behavioral tests
|
||
*
|
||
* Three deterministic, cross-platform tests that verify cleanup()'s
|
||
* observable contract at the seam rather than probing its internals.
|
||
*/
|
||
|
||
const { test } = require('node:test');
|
||
const assert = require('node:assert/strict');
|
||
const fs = require('fs');
|
||
const path = require('path');
|
||
|
||
const os = require('os');
|
||
|
||
const { cleanup, createTempDir } = require('./helpers.cjs');
|
||
|
||
// ─── Test 1: Real-FS happy path ──────────────────────────────────────────────
|
||
|
||
test('cleanup removes a real temp dir with nested subdirs and files', () => {
|
||
const dir = createTempDir('gsd-cleanup-test-');
|
||
const nested = path.join(dir, 'a', 'b', 'c');
|
||
fs.mkdirSync(nested, { recursive: true });
|
||
fs.writeFileSync(path.join(nested, 'file.txt'), 'hello');
|
||
fs.writeFileSync(path.join(dir, 'root.txt'), 'world');
|
||
|
||
cleanup(dir);
|
||
|
||
assert.strictEqual(fs.existsSync(dir), false, 'temp dir should not exist after cleanup');
|
||
});
|
||
|
||
// ─── Test 2: Retry-budget contract ───────────────────────────────────────────
|
||
|
||
test('cleanup passes recursive/force/maxRetries/retryDelay options to fs.rmSync', () => {
|
||
// Use a real temp dir as the target so cleanup() has a valid path argument.
|
||
// We chdir AWAY from it first so cleanup() does not try to chdir either.
|
||
const dir = createTempDir('gsd-cleanup-opts-test-');
|
||
|
||
// Capture original cwd and shift away from the target.
|
||
const originalCwd = process.cwd();
|
||
// Chdir to the parent of the target so cleanup's cwd-guard is a no-op.
|
||
process.chdir(path.dirname(dir));
|
||
|
||
let capturedOptions = null;
|
||
const realRmSync = fs.rmSync;
|
||
|
||
try {
|
||
// Replace fs.rmSync with a probe that captures options then does nothing.
|
||
// This is an assignment expression (not a CallExpression) so it satisfies
|
||
// the ESLint rule that bans raw fs.rmSync(...) call expressions in tests.
|
||
fs.rmSync = (targetPath, opts) => {
|
||
capturedOptions = opts;
|
||
// Do NOT call through — we don't want the dir actually removed here;
|
||
// we're only testing the options shape.
|
||
};
|
||
|
||
cleanup(dir);
|
||
} finally {
|
||
fs.rmSync = realRmSync;
|
||
process.chdir(originalCwd);
|
||
// Remove the dir with the real rmSync now that we restored it.
|
||
cleanup(dir);
|
||
}
|
||
|
||
assert.ok(capturedOptions !== null, 'fs.rmSync should have been called');
|
||
assert.strictEqual(capturedOptions.recursive, true, 'recursive must be true');
|
||
assert.strictEqual(capturedOptions.force, true, 'force must be true');
|
||
assert.ok(
|
||
typeof capturedOptions.maxRetries === 'number' && capturedOptions.maxRetries > 0,
|
||
'maxRetries must be a positive number'
|
||
);
|
||
assert.ok(
|
||
typeof capturedOptions.retryDelay === 'number' && capturedOptions.retryDelay > 0,
|
||
'retryDelay must be a positive number'
|
||
);
|
||
});
|
||
|
||
// ─── Test 3: cwd-guard ───────────────────────────────────────────────────────
|
||
|
||
test('cleanup does not throw when cwd is inside the target dir, and removes the dir', () => {
|
||
const dir = createTempDir('gsd-cleanup-cwd-test-');
|
||
const nested = path.join(dir, 'deep', 'nested');
|
||
fs.mkdirSync(nested, { recursive: true });
|
||
|
||
const originalCwd = process.cwd();
|
||
|
||
try {
|
||
// Step INTO the nested subdir so cwd is inside the cleanup target.
|
||
process.chdir(nested);
|
||
|
||
assert.doesNotThrow(() => {
|
||
cleanup(dir);
|
||
}, 'cleanup should not throw even when cwd is inside the target');
|
||
} finally {
|
||
// Restore original cwd. cleanup() will have chdir'd to dirname(dir),
|
||
// so we always restore explicitly regardless.
|
||
if (process.cwd() !== originalCwd) {
|
||
process.chdir(originalCwd);
|
||
}
|
||
}
|
||
|
||
assert.strictEqual(fs.existsSync(dir), false, 'temp dir should not exist after cleanup');
|
||
});
|
||
|
||
// ─── Test 4: out-of-tmpdir refusal ───────────────────────────────────────────
|
||
|
||
test('cleanup throws and does not chdir or delete when target is outside os.tmpdir()', () => {
|
||
// __dirname (this repo's tests/ directory) must never be deleted. Whether
|
||
// it is actually outside os.tmpdir() is environment-dependent: on Linux
|
||
// os.tmpdir() is /tmp, and a CI container that checks the repo out under
|
||
// /tmp would put __dirname INSIDE tmpdir, in which case a correctly-working
|
||
// cleanup() would not refuse it -- it would delete this directory. The
|
||
// precondition assertion below verifies the "outside tmpdir" assumption
|
||
// before cleanup() is ever called, so that situation fails loudly and
|
||
// safely instead of destructively. No scratch directory is created, so
|
||
// there is nothing to tear down.
|
||
const outsideDir = __dirname;
|
||
const knownFile = path.join(outsideDir, 'helpers-cleanup.test.cjs');
|
||
|
||
// Mirror cleanup()'s own out-of-tmpdir predicate (tests/helpers.cjs) so
|
||
// this test cannot run on a target it does not actually control.
|
||
const tmpRoot = path.resolve(os.tmpdir());
|
||
const resolvedOutsideDir = path.resolve(outsideDir);
|
||
const isInsideTmpdir =
|
||
resolvedOutsideDir === tmpRoot || resolvedOutsideDir.startsWith(`${tmpRoot}${path.sep}`);
|
||
assert.strictEqual(
|
||
isInsideTmpdir,
|
||
false,
|
||
`this test cannot run safely when the repo lives under os.tmpdir(): ` +
|
||
`outsideDir (${resolvedOutsideDir}) is inside os.tmpdir() (${tmpRoot})`
|
||
);
|
||
|
||
const cwdBefore = process.cwd();
|
||
|
||
assert.throws(
|
||
() => cleanup(outsideDir),
|
||
(err) => err instanceof Error && err.message.includes(outsideDir),
|
||
'cleanup must throw an Error whose message names the offending path'
|
||
);
|
||
|
||
assert.strictEqual(
|
||
process.cwd(),
|
||
cwdBefore,
|
||
'cleanup must refuse before chdir, so cwd is unchanged'
|
||
);
|
||
assert.strictEqual(fs.existsSync(outsideDir), true, 'target directory must still exist after refusal');
|
||
assert.strictEqual(
|
||
fs.existsSync(knownFile),
|
||
true,
|
||
'a known file inside the target must still exist, proving the dir was not emptied'
|
||
);
|
||
});
|
||
|
||
// ─── Test 5: control — a real os.tmpdir()-rooted path still cleans up ───────
|
||
|
||
test('cleanup still removes a real os.tmpdir()-rooted directory (control)', () => {
|
||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-cleanup-control-'));
|
||
|
||
cleanup(dir);
|
||
|
||
assert.strictEqual(fs.existsSync(dir), false, 'os.tmpdir()-rooted directory should be removed');
|
||
});
|