diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index 6454bf961..e6d5b32d6 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -45,6 +45,13 @@ identity ratchet (`npm run lint:regression-names`, part of `npm run lint:ci`): - A **new** `bug-*` file fails CI — fold it into the owning module's file. - **Deleting/consolidating** a grandfathered file requires pruning its allowlist entry, so the baseline only ever shrinks. +- **Inherited drift** (the failure names files your PR didn't add — e.g. the + base branch merged `bug-*` files without feeding the allowlist, or you + rebased and carried a pre-rebase allowlist): run + `node scripts/lint-regression-test-names.cjs --update` and commit the + regenerated allowlist. Snapshot artifacts like this allowlist (and + `docs/INVENTORY.md`) must be regenerated **after** rebasing, never carried + through a rebase. The ratchet deliberately covers only `bug-*`. Files named `feat-NNNN-*` / `enh-NNNN-*` are *feature* test files — one (or one per suite) per feature is diff --git a/scripts/lint-regression-test-names.cjs b/scripts/lint-regression-test-names.cjs index 724f81b52..f75293f99 100644 --- a/scripts/lint-regression-test-names.cjs +++ b/scripts/lint-regression-test-names.cjs @@ -24,6 +24,16 @@ * entry from scripts/lint-regression-test-names.allowlist.json so the * baseline only ever shrinks. * + * ## --update (allowlist drift repair) + * + * `node scripts/lint-regression-test-names.cjs --update` regenerates the + * allowlist from the files currently in tests/ and reports what changed. + * Use it when the failure is INHERITED drift, not your own new file — e.g. + * the base branch merged bug-* files without feeding the allowlist (the + * #947/#948/#950 race after the ratchet landed), or after a rebase. The + * allowlist is a snapshot artifact: regenerate it AFTER rebasing, never + * carry a pre-rebase copy through. + * * See docs/TESTING-SUITES.md ("Regression tests") for the placement policy. */ @@ -42,12 +52,38 @@ const ALLOWLIST_PATH = const BUG_FILE_RE = /^bug-\d+.*\.test\.cjs$/; function main() { + const args = process.argv.slice(2); + const update = args.includes('--update'); + const unknown = args.filter(a => a !== '--update'); + if (unknown.length > 0) { + throw new ExitError(2, `lint-regression-test-names: unknown argument(s): ${unknown.join(', ')}`); + } + const current = fs .readdirSync(TESTS_DIR) .filter(f => BUG_FILE_RE.test(f)) .sort(); const known = JSON.parse(fs.readFileSync(ALLOWLIST_PATH, 'utf8')); + if (update) { + const knownSet = new Set(known); + const currentSet = new Set(current); + const added = current.filter(f => !knownSet.has(f)); + const pruned = known.filter(f => !currentSet.has(f)); + if (added.length === 0 && pruned.length === 0) { + console.log(`lint-regression-test-names --update: allowlist already in sync (${current.length} entries)`); + return; + } + fs.writeFileSync(ALLOWLIST_PATH, JSON.stringify(current, null, 2) + '\n'); + console.log( + `lint-regression-test-names --update: ${known.length} -> ${current.length} entries` + + (added.length ? ` | grandfathered: ${added.join(', ')}` : '') + + (pruned.length ? ` | pruned: ${pruned.join(', ')}` : '') + ); + console.log('Commit the regenerated allowlist with your change.'); + return; + } + const failures = []; const { novel } = assertWithinAllowlist({ label: 'regression-test-names', @@ -61,10 +97,13 @@ function main() { for (const msg of failures) console.error(msg); if (novel.length > 0) { console.error( - '\nNew bug-NNNN test files are no longer accepted. Add the regression ' + - "case to the owning module's test file (e.g. a describe('regressions') " + - 'block in tests/.test.cjs) instead of creating a new file. ' + - 'See docs/TESTING-SUITES.md.' + '\nIf this PR added the file(s) above: new bug-NNNN test files are not ' + + "accepted — add the regression case to the owning module's test file " + + "(e.g. a describe('regressions') block in tests/.test.cjs) instead.\n" + + 'If the file(s) came from the base branch (inherited allowlist drift, ' + + 'e.g. after a rebase): run ' + + '`node scripts/lint-regression-test-names.cjs --update` and commit the ' + + 'regenerated allowlist. See docs/TESTING-SUITES.md.' ); } throw new ExitError(1); diff --git a/tests/lint-regression-test-names.test.cjs b/tests/lint-regression-test-names.test.cjs index 430d57d4f..9fbd46e27 100644 --- a/tests/lint-regression-test-names.test.cjs +++ b/tests/lint-regression-test-names.test.cjs @@ -19,13 +19,13 @@ let sandbox; let fixtureCount = 0; -function runLint({ files, allowlist }) { +function runLint({ files, allowlist, args = [] }) { const testsDir = path.join(sandbox, `tests-${fixtureCount++}`); fs.mkdirSync(testsDir, { recursive: true }); for (const f of files) fs.writeFileSync(path.join(testsDir, f), ''); const allowlistPath = path.join(testsDir, 'allowlist.json'); fs.writeFileSync(allowlistPath, JSON.stringify(allowlist)); - return spawnSync(process.execPath, [SCRIPT], { + const r = spawnSync(process.execPath, [SCRIPT, ...args], { cwd: ROOT, encoding: 'utf8', env: { @@ -34,6 +34,8 @@ function runLint({ files, allowlist }) { GSD_LINT_REGRESSION_ALLOWLIST: allowlistPath, }, }); + r.allowlistPath = allowlistPath; + return r; } describe('lint-regression-test-names', () => { @@ -93,4 +95,48 @@ describe('lint-regression-test-names', () => { const r = spawnSync(process.execPath, [SCRIPT], { cwd: ROOT, encoding: 'utf8' }); assert.strictEqual(r.status, 0, `stderr: ${r.stderr}\nstdout: ${r.stdout}`); }); + + test('novel-offender failure names the --update drift-repair path', () => { + const r = runLint({ + files: ['bug-500-inherited.test.cjs'], + allowlist: [], + }); + assert.notStrictEqual(r.status, 0); + assert.match(r.stderr, /--update/); + }); + + test('--update regenerates the allowlist from the tests dir (grandfather + prune)', () => { + const r = runLint({ + files: ['bug-100-kept.test.cjs', 'bug-200-new.test.cjs'], + allowlist: ['bug-100-kept.test.cjs', 'bug-300-gone.test.cjs'], + args: ['--update'], + }); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + assert.match(r.stdout, /grandfathered: bug-200-new\.test\.cjs/); + assert.match(r.stdout, /pruned: bug-300-gone\.test\.cjs/); + assert.deepStrictEqual( + JSON.parse(fs.readFileSync(r.allowlistPath, 'utf8')), + ['bug-100-kept.test.cjs', 'bug-200-new.test.cjs'], + ); + }); + + test('--update is a no-op when already in sync', () => { + const r = runLint({ + files: ['bug-100-kept.test.cjs'], + allowlist: ['bug-100-kept.test.cjs'], + args: ['--update'], + }); + assert.strictEqual(r.status, 0, `stderr: ${r.stderr}`); + assert.match(r.stdout, /already in sync/); + assert.deepStrictEqual( + JSON.parse(fs.readFileSync(r.allowlistPath, 'utf8')), + ['bug-100-kept.test.cjs'], + ); + }); + + test('unknown arguments are rejected', () => { + const r = runLint({ files: [], allowlist: [], args: ['--frobnicate'] }); + assert.strictEqual(r.status, 2); + assert.match(r.stderr, /unknown argument/); + }); });