fix(ratchet): add --update drift repair + inherited-drift guidance to regression-name lint (#971)
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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/<module>.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/<module>.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);
|
||||
|
||||
@@ -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/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user