chore(tests): lint rule — cap test files per production module at 2 (#3738)

* chore(tests): lint rule — cap test files per production module at 2

Adds scripts/lint-test-file-count.cjs with a ratcheted allowlist
(scripts/lint-test-file-count.allowlist.json) capturing today's
30 violating clusters as a ceiling. New entries blocked at PR time;
reductions ratchet automatically.

Wires into .github/workflows/test.yml as a new step in lint-tests.

Refs #3737

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(tests): add docs-exempt to changeset fragment

Internal CI lint rule — no user-facing docs impact.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-20 20:37:07 -04:00
committed by GitHub
parent 479839147e
commit 6313baad63
6 changed files with 462 additions and 0 deletions

View File

@@ -0,0 +1,6 @@
---
type: Added
pr: 3733
---
<!-- docs-exempt: internal CI lint rule only — adds scripts/, tests/, and a workflow step; no user-facing commands, output, or configuration changed -->
**New CI lint rule `lint-test-file-count` prevents per-feature test-file proliferation** — a recurring pattern where one production module (e.g. `phase.cjs`, `init.cjs`) accumulates 5–20 separate test files over time as PRs add issue-stamped files (`bug-NNNN-*.test.cjs`, `feat-NNNN-*.test.cjs`) next to an existing primary. The rule scans `sdk/src/query/`, `sdk/src/`, `get-shit-done/bin/lib/`, and `bin/` for production modules, then counts matching test files in `tests/` and `sdk/src/**/`. Each module is capped at 2 (primary + one integration). Existing over-limit clusters are frozen in `scripts/lint-test-file-count.allowlist.json` at their current count (30 modules; `phase` is the worst at 20 files). The allowlist ratchets downward automatically — reducing a cluster is always allowed; increasing it requires a PR-description justification. Added `"lint:test-file-count"` npm script and a `Lint — test file count per module` step in the `lint-tests` CI job.

View File

@@ -30,6 +30,9 @@ jobs:
- name: Lint — no source-grep tests
shell: bash
run: node scripts/lint-no-source-grep.cjs
- name: Lint — test file count per module
shell: bash
run: node scripts/lint-test-file-count.cjs
- name: Lint — command contract (ADR-0002)
shell: bash
run: node scripts/lint-command-contract.cjs

View File

@@ -76,6 +76,7 @@
"lint:descriptions": "node scripts/lint-descriptions.cjs",
"lint:skill-deps": "node scripts/lint-skill-deps.cjs",
"lint:tests": "node scripts/lint-no-source-grep.cjs",
"lint:test-file-count": "node scripts/lint-test-file-count.cjs",
"lint:pr-checks": "node scripts/lint-pr-check-project-dir.cjs",
"lint:changeset": "node scripts/changeset/lint.cjs",
"lint:docs": "node scripts/lint-docs-required.cjs",

View File

@@ -0,0 +1,35 @@
{
"_doc": "Baseline of modules currently exceeding the 2-test-file limit. Each entry locks in TODAY's count as the ceiling. Reductions are ratcheted automatically — when a cluster drops to ≤ 2, remove its entry. New entries require justification in PR description.",
"modules": {
"phase": { "current": 20, "issue": "TBD" },
"worktree": { "current": 13, "issue": "TBD" },
"milestone": { "current": 10, "issue": "TBD" },
"roadmap": { "current": 9, "issue": "TBD" },
"verify": { "current": 9, "issue": "TBD" },
"install": { "current": 9, "issue": "TBD" },
"init": { "current": 8, "issue": "TBD" },
"state": { "current": 9, "issue": "TBD" },
"config": { "current": 8, "issue": "TBD" },
"graphify": { "current": 7, "issue": "TBD" },
"progress": { "current": 5, "issue": "TBD" },
"cli": { "current": 5, "issue": "TBD" },
"surface": { "current": 5, "issue": "TBD" },
"commit": { "current": 4, "issue": "TBD" },
"frontmatter": { "current": 4, "issue": "TBD" },
"index": { "current": 4, "issue": "TBD" },
"intel": { "current": 4, "issue": "TBD" },
"mvp": { "current": 4, "issue": "TBD" },
"install-profiles": { "current": 4, "issue": "TBD" },
"audit-open": { "current": 3, "issue": "TBD" },
"config-schema": { "current": 3, "issue": "TBD" },
"profile": { "current": 3, "issue": "TBD" },
"prompt-budget": { "current": 3, "issue": "TBD" },
"uat": { "current": 3, "issue": "TBD" },
"validate": { "current": 3, "issue": "TBD" },
"workstream": { "current": 3, "issue": "TBD" },
"gsd-tools": { "current": 3, "issue": "TBD" },
"runtime-artifact-layout":{ "current": 3, "issue": "TBD" },
"security": { "current": 3, "issue": "TBD" },
"gsd-sdk": { "current": 3, "issue": "TBD" }
}
}

View File

@@ -0,0 +1,190 @@
#!/usr/bin/env node
/**
* lint-test-file-count.cjs — max 2 test files per production module.
*
* Scans sdk/src/query/, sdk/src/, get-shit-done/bin/lib/, bin/ for production
* modules, then counts matching test files in tests/ and sdk/src (recursive). Cap is 2
* (primary + one integration). Over-limit clusters must be in the allowlist at
* their frozen count (ratchet: may only decrease). --json emits structured output.
*
* Verdicts: OK_UNDER_LIMIT | OK_IN_ALLOWLIST | FAIL_EXCEEDS_LIMIT |
* FAIL_EXCEEDS_ALLOWLIST | HINT_CAN_REMOVE_FROM_ALLOWLIST
*/
'use strict';
const fs = require('fs');
const path = require('path');
const ROOT = path.join(__dirname, '..');
const PROD_DIRS = [
path.join(ROOT, 'sdk', 'src', 'query'),
path.join(ROOT, 'sdk', 'src'),
path.join(ROOT, 'get-shit-done', 'bin', 'lib'),
path.join(ROOT, 'bin'),
];
const TEST_DIRS = [
path.join(ROOT, 'tests'),
path.join(ROOT, 'sdk', 'src'),
];
const ALLOWLIST_PATH = path.join(__dirname, 'lint-test-file-count.allowlist.json');
const MAX_FILES = 2;
const Verdict = Object.freeze({
OK_UNDER_LIMIT: 'OK_UNDER_LIMIT',
OK_IN_ALLOWLIST: 'OK_IN_ALLOWLIST',
FAIL_EXCEEDS_LIMIT: 'FAIL_EXCEEDS_LIMIT',
FAIL_EXCEEDS_ALLOWLIST: 'FAIL_EXCEEDS_ALLOWLIST',
HINT_CAN_REMOVE_FROM_ALLOWLIST: 'HINT_CAN_REMOVE_FROM_ALLOWLIST',
});
function isTestFile(name) {
return name.endsWith('.test.ts') || name.endsWith('.test.cjs');
}
function listFiles(dir, pred) {
try {
return fs.readdirSync(dir, { withFileTypes: true })
.filter(e => e.isFile() && pred(e.name))
.map(e => path.join(dir, e.name));
} catch (_) { return []; }
}
function findTestFilesRecursive(dir) {
const out = [];
let entries;
try { entries = fs.readdirSync(dir, { withFileTypes: true }); }
catch (_) { return out; }
for (const e of entries) {
const full = path.join(dir, e.name);
if (e.isDirectory()) out.push(...findTestFilesRecursive(full));
else if (isTestFile(e.name)) out.push(full);
}
return out;
}
function prodPrefix(filename) {
return filename.replace(/\.(cjs|ts|js)$/, '');
}
// Strip .test.{cjs,ts} and .integration.test.ts, then strip issue stamps.
function testEffectivePrefix(testName) {
const bare = testName
.replace(/\.integration\.test\.(ts|cjs)$/, '')
.replace(/\.test\.(ts|cjs)$/, '');
const m = bare.match(/^(?:feat|bug|enh|fix)-\d+(?:-\d+)*-(.+)$/);
return m ? m[1] : bare;
}
function collectProdPrefixes() {
const map = new Map();
for (const dir of PROD_DIRS) {
for (const f of listFiles(dir, n =>
!isTestFile(n) &&
!/\.(generated|md|json)(\.|$)/.test(n) &&
/\.(ts|cjs|js)$/.test(n)
)) {
const prefix = prodPrefix(path.basename(f));
if (!map.has(prefix)) map.set(prefix, f);
}
}
return map;
}
function collectAllTestFiles() {
const seen = new Set();
const all = [];
for (const dir of TEST_DIRS) {
for (const f of findTestFilesRecursive(dir)) {
if (!seen.has(f)) { seen.add(f); all.push(f); }
}
}
return all;
}
function buildTestMap(prodPrefixes, allTestFiles) {
const map = new Map([...prodPrefixes.keys()].map(p => [p, []]));
for (const tf of allTestFiles) {
const ep = testEffectivePrefix(path.basename(tf));
for (const prefix of prodPrefixes.keys()) {
if (ep === prefix || ep.startsWith(prefix + '-')) {
map.get(prefix).push(tf);
break;
}
}
}
return map;
}
function loadAllowlist() {
try { return JSON.parse(fs.readFileSync(ALLOWLIST_PATH, 'utf-8')).modules || {}; }
catch (_) { return {}; }
}
function evaluateLint({ prefix, testFiles, allowlist }) {
const count = testFiles.length;
const entry = allowlist[prefix];
const ceiling = entry ? entry.current : null;
if (entry !== undefined) {
if (count <= MAX_FILES) return { verdict: Verdict.HINT_CAN_REMOVE_FROM_ALLOWLIST, prefix, count, ceiling, files: testFiles };
if (count <= ceiling) return { verdict: Verdict.OK_IN_ALLOWLIST, prefix, count, ceiling, files: testFiles };
return { verdict: Verdict.FAIL_EXCEEDS_ALLOWLIST, prefix, count, ceiling, files: testFiles };
}
if (count <= MAX_FILES) return { verdict: Verdict.OK_UNDER_LIMIT, prefix, count, ceiling: null, files: testFiles };
return { verdict: Verdict.FAIL_EXCEEDS_LIMIT, prefix, count, ceiling: null, files: testFiles };
}
function run() {
const jsonMode = process.argv.includes('--json');
const prodPrefixes = collectProdPrefixes();
const allTestFiles = collectAllTestFiles();
const testMap = buildTestMap(prodPrefixes, allTestFiles);
const allowlist = loadAllowlist();
const results = [];
for (const [prefix, files] of testMap) {
if (files.length === 0) continue;
results.push(evaluateLint({ prefix, testFiles: files, allowlist }));
}
const failures = results.filter(r =>
r.verdict === Verdict.FAIL_EXCEEDS_LIMIT || r.verdict === Verdict.FAIL_EXCEEDS_ALLOWLIST);
const hints = results.filter(r => r.verdict === Verdict.HINT_CAN_REMOVE_FROM_ALLOWLIST);
if (jsonMode) {
console.log(JSON.stringify({ ok: failures.length === 0, results, failures, hints }, null, 2));
process.exit(failures.length > 0 ? 1 : 0);
}
if (failures.length === 0) {
const inAllowlist = results.filter(r => r.verdict === Verdict.OK_IN_ALLOWLIST).length;
console.log(`ok lint-test-file-count: ${results.length} module(s) checked, 0 failures` +
(inAllowlist > 0 ? `, ${inAllowlist} allowlisted` : '') +
(hints.length > 0 ? `, ${hints.length} hint(s)` : ''));
for (const h of hints) {
console.log(` hint: "${h.prefix}" is allowlisted at ${h.ceiling} but now has ${h.count} — remove from allowlist`);
}
process.exit(0);
}
process.stderr.write(`\nERROR lint-test-file-count: ${failures.length} module(s) exceed the test-file limit\n\n`);
for (const f of failures) {
const tag = f.verdict === Verdict.FAIL_EXCEEDS_LIMIT
? `${f.count} files (limit ${MAX_FILES})`
: `${f.count} files (allowlist ceiling ${f.ceiling})`;
process.stderr.write(` ${f.prefix}: ${tag}\n`);
for (const tf of f.files) process.stderr.write(` ${path.relative(ROOT, tf)}\n`);
}
process.stderr.write('\nFix: consolidate test files per module (one primary + one integration).\n');
process.stderr.write('Or add the module to scripts/lint-test-file-count.allowlist.json with PR justification.\n\n');
process.exit(1);
}
module.exports = {
Verdict, evaluateLint, testEffectivePrefix, prodPrefix,
_collectProdPrefixes: collectProdPrefixes,
_collectAllTestFiles: collectAllTestFiles,
_buildTestMap: buildTestMap,
_loadAllowlist: loadAllowlist,
};
if (require.main === module) run();

View File

@@ -0,0 +1,227 @@
'use strict';
/**
* Tests for scripts/lint-test-file-count.cjs
*
* Uses node --test + the exported evaluateLint() pure function.
* Also exercises the CLI via --json mode to verify end-to-end wiring.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { spawnSync } = require('child_process');
const ROOT = path.join(__dirname, '..');
const LINT_SCRIPT = path.join(ROOT, 'scripts', 'lint-test-file-count.cjs');
const {
Verdict,
evaluateLint,
testEffectivePrefix,
} = require(LINT_SCRIPT);
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
function makeFiles(prefix, names) {
return names.map(n => `/fake/tests/${n}`);
}
function runCliJson(extraArgs = []) {
const result = spawnSync(
process.execPath,
[LINT_SCRIPT, '--json', ...extraArgs],
{ encoding: 'utf8' }
);
const parsed = JSON.parse(result.stdout);
return { status: result.status, data: parsed };
}
// ---------------------------------------------------------------------------
// evaluateLint — core verdict logic
// ---------------------------------------------------------------------------
describe('evaluateLint — OK_UNDER_LIMIT', () => {
test('1-file module passes', () => {
const result = evaluateLint({
prefix: 'my-module',
testFiles: makeFiles('my-module', ['my-module.test.cjs']),
allowlist: {},
});
assert.strictEqual(result.verdict, Verdict.OK_UNDER_LIMIT);
assert.strictEqual(result.count, 1);
assert.strictEqual(result.ceiling, null);
});
test('2-file module passes (primary + integration)', () => {
const result = evaluateLint({
prefix: 'my-module',
testFiles: makeFiles('my-module', [
'my-module.test.cjs',
'my-module.integration.test.ts',
]),
allowlist: {},
});
assert.strictEqual(result.verdict, Verdict.OK_UNDER_LIMIT);
assert.strictEqual(result.count, 2);
});
});
describe('evaluateLint — FAIL_EXCEEDS_LIMIT', () => {
test('3-file module fails when not in allowlist', () => {
const result = evaluateLint({
prefix: 'my-module',
testFiles: makeFiles('my-module', [
'my-module.test.cjs',
'my-module-edge-case.test.cjs',
'my-module-regression.test.cjs',
]),
allowlist: {},
});
assert.strictEqual(result.verdict, Verdict.FAIL_EXCEEDS_LIMIT);
assert.strictEqual(result.count, 3);
assert.strictEqual(result.ceiling, null);
});
});
describe('evaluateLint — allowlist behaviour', () => {
test('3-file module allowlisted at 3 passes (OK_IN_ALLOWLIST)', () => {
const result = evaluateLint({
prefix: 'phase',
testFiles: makeFiles('phase', [
'phase.test.cjs',
'phase-edge.test.cjs',
'phase-regression.test.cjs',
]),
allowlist: { phase: { current: 3, issue: 'TBD' } },
});
assert.strictEqual(result.verdict, Verdict.OK_IN_ALLOWLIST);
assert.strictEqual(result.count, 3);
assert.strictEqual(result.ceiling, 3);
});
test('2-file module allowlisted at 3 emits HINT_CAN_REMOVE_FROM_ALLOWLIST', () => {
const result = evaluateLint({
prefix: 'phase',
testFiles: makeFiles('phase', [
'phase.test.cjs',
'phase-edge.test.cjs',
]),
allowlist: { phase: { current: 3, issue: 'TBD' } },
});
assert.strictEqual(result.verdict, Verdict.HINT_CAN_REMOVE_FROM_ALLOWLIST);
assert.strictEqual(result.count, 2);
assert.strictEqual(result.ceiling, 3);
});
test('4-file module allowlisted at 3 fails (FAIL_EXCEEDS_ALLOWLIST)', () => {
const result = evaluateLint({
prefix: 'phase',
testFiles: makeFiles('phase', [
'phase.test.cjs',
'phase-a.test.cjs',
'phase-b.test.cjs',
'phase-c.test.cjs',
]),
allowlist: { phase: { current: 3, issue: 'TBD' } },
});
assert.strictEqual(result.verdict, Verdict.FAIL_EXCEEDS_ALLOWLIST);
assert.strictEqual(result.count, 4);
assert.strictEqual(result.ceiling, 3);
});
test('ratchet: count equal to ceiling passes', () => {
const result = evaluateLint({
prefix: 'init',
testFiles: makeFiles('init', [
'init.test.cjs',
'init-manager.test.cjs',
'init-manager-deps.test.cjs',
]),
allowlist: { init: { current: 3, issue: 'TBD' } },
});
assert.strictEqual(result.verdict, Verdict.OK_IN_ALLOWLIST);
});
});
// ---------------------------------------------------------------------------
// testEffectivePrefix — issue-stamp stripping
// ---------------------------------------------------------------------------
describe('testEffectivePrefix', () => {
test('normal test file returns bare prefix', () => {
assert.strictEqual(testEffectivePrefix('query-dispatch.test.cjs'), 'query-dispatch');
});
test('integration test file returns bare prefix', () => {
assert.strictEqual(testEffectivePrefix('init.integration.test.ts'), 'init');
});
test('bug-stamped file strips stamp', () => {
assert.strictEqual(testEffectivePrefix('bug-1736-local-install-commands.test.cjs'), 'local-install-commands');
});
test('feat-stamped file strips stamp', () => {
assert.strictEqual(testEffectivePrefix('feat-3347-graphify-auto-update-config.test.cjs'), 'graphify-auto-update-config');
});
test('enh-stamped file strips stamp', () => {
assert.strictEqual(testEffectivePrefix('enh-100-phase-runner-edge.test.cjs'), 'phase-runner-edge');
});
test('fix-stamped file strips stamp', () => {
assert.strictEqual(testEffectivePrefix('fix-200-config-merge.test.cjs'), 'config-merge');
});
test('double-numbered stamp is stripped correctly', () => {
assert.strictEqual(testEffectivePrefix('bug-2550-2552-discuss-phase-context.test.cjs'), 'discuss-phase-context');
});
});
// ---------------------------------------------------------------------------
// CLI — JSON mode end-to-end
// ---------------------------------------------------------------------------
describe('CLI --json', () => {
test('script parses without syntax errors', () => {
const result = spawnSync(process.execPath, ['--check', LINT_SCRIPT], { encoding: 'utf8' });
assert.strictEqual(result.status, 0, result.stderr);
});
test('exits 0 against real repo (allowlist covers all current violations)', () => {
const { status, data } = runCliJson();
assert.strictEqual(status, 0, `Expected clean run; failures: ${JSON.stringify(data.failures)}`);
assert.strictEqual(data.ok, true);
assert.strictEqual(data.failures.length, 0);
});
test('--json output has required fields', () => {
const { data } = runCliJson();
assert.ok(Array.isArray(data.results), 'results must be array');
assert.ok(Array.isArray(data.failures), 'failures must be array');
assert.ok(Array.isArray(data.hints), 'hints must be array');
assert.ok(typeof data.ok === 'boolean', 'ok must be boolean');
});
test('each result has verdict, prefix, count, ceiling, files', () => {
const { data } = runCliJson();
for (const r of data.results) {
assert.ok(typeof r.verdict === 'string', `verdict missing on ${r.prefix}`);
assert.ok(typeof r.prefix === 'string', 'prefix must be string');
assert.ok(typeof r.count === 'number', 'count must be number');
assert.ok(Array.isArray(r.files), 'files must be array');
}
});
test('all verdicts are valid enum values', () => {
const valid = new Set(Object.values(Verdict));
const { data } = runCliJson();
for (const r of data.results) {
assert.ok(valid.has(r.verdict), `Unknown verdict "${r.verdict}" on prefix "${r.prefix}"`);
}
});
});