Files
msd-core/tests/helpers-cleanup.test.cjs
Tom Boucher a28dcec981 chore(#597): replace count-based ratchet guards with AST lint + named-set allowlists (#603)
The windows-test-parity ratchet greps test source for fs.rmSync-without-
maxRetries (and six other Windows-portability anti-patterns), failing when an
integer offender COUNT exceeds a frozen baseline (rmSync: 95). A count ratchet
is a Goodhart metric: fixing one offender and adding another keeps the count
constant, so a new defect slips through green. Replace it — and every other
count ratchet in the repo — with a layered, masking-proof design.

Behavioral seam test
- tests/helpers-cleanup.test.cjs proves helpers.cleanup() carries the Windows
  EBUSY retry budget. cleanup() delegates retries to Node's fs.rmSync via
  maxRetries (it owns no loop), so the test asserts the option contract
  (recursive/force/maxRetries>0/retryDelay>0) + real-FS removal + the cwd-guard,
  rather than a loop that does not exist. The EBUSY risk is now tested ONCE at
  the helper, not approximated textually at every call site.

Write-time ESLint rule (AST-accurate, replaces the grep)
- eslint-rules/no-raw-rmsync-in-tests.cjs (error in tests/**/*.test.cjs) bans
  raw fs.rmSync, steering to cleanup(). Catches member, computed (fs['rmSync']),
  destructured and aliased forms; escape hatch is inline
  `// eslint-disable-next-line local/no-raw-rmsync-in-tests -- <reason>` only.
- Migrated 336 raw fs.rmSync teardown calls across ~116 test files to cleanup().
  ~18 genuinely load-bearing sites (mid-test SUT/fault-injection removals,
  error-swallowing or name-colliding local teardown helpers) keep the raw call
  with an inline eslint-disable + reason.

Shared anti-ratchet primitive
- scripts/lib/allowlist-ratchet.cjs:
  - assertWithinAllowlist: fails on NOVEL ids (new offender introduced) AND on
    STALE ids (a known offender was fixed but not pruned) — identity, not count,
    and a ratchet DOWN toward zero.
  - assertTightCeiling: a size/length budget whose ceiling must stay within a
    grace band of the high-water mark, so budgets may only tighten, never creep.

Ratchets converted onto the primitive
- windows-test-parity-guard.test.cjs: rmSync rule deleted (now ESLint-enforced);
  the remaining six patterns moved from integer baselines to named-set
  allowlists with ratchet-down.
- scripts/lint-test-file-count.{cjs,allowlist.json}: per-module integer counts →
  named filename sets (closes the swap-a-file-keep-the-count blind spot); a
  module dropping under cap now FAILS to force pruning its allowlist entry.
- enh-2790 skill-count `<= 63` → named skill allowlist (ratchets toward ~58).

Size budgets hardened (tighten-only)
- agent-size / workflow-size / feat-3039 help-tiered: ceilings lowered to the
  current high-water mark and an assertTightCeiling anti-creep check added per
  tier. Fixed external-contract limits (description ≤100 chars, agent ≤100 KB)
  are intentionally left as-is — they are not grandfathered creeping budgets.

No user-facing behavior change (tests + tooling only); no USER_FACING_PREFIXES
touched, so no changeset fragment is required.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-01 22:43:49 -04:00

102 lines
3.9 KiB
JavaScript
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
/**
* 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');
});