mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 06:24:41 +01:00
fix(pregate): literals skips a version-history row when the branch extended its table (GST-344)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
8be7f21385
commit
de838e3cc1
4 files changed
+83
-5
No files matched your search
@@ -0,0 +1,8 @@
|
||||
**pregate `literals` no longer flags a version-history table that the branch extended.** A test that keeps one row per past version of a constant (`[45, "<digest>"]`, then `[46, "<digest>"]`) used to fail `literals` on every bump, because the old row still holds the old value. Truther turned the check down to a warning over it, which also hid real stale tests.
|
||||
|
||||
### Fixed
|
||||
- `lib/pregate/checks/literals.ts`: a row that starts with the old value is skipped when the same table holds a new-value row the branch added (the file at the merge base lacks it). The scan continues to the next line holding the old value, so `expect(PROOF_CHECKS).toBe(45)` below an extended table still fails. A table the branch did not extend, or one that already held the new row, still fails.
|
||||
- docs/pregate.md describes the history-row rule and the table shapes it recognizes.
|
||||
|
||||
### Tests
|
||||
- `test/pregate-selection.test.ts`: an extended table, a newest-first table and a new file pass; an unextended table, a stale assertion below an extended table, an unextended table beside an extended one, and a table that already held the new row fail at the right line; a string-valued history passes; and an end-to-end `gstack-pregate --only literals` run fails before the table is extended and passes after.
|
||||
+7
-1
@@ -169,7 +169,13 @@ Two stages with separate clocks, both printed:
|
||||
- `literals`: for every changed exported constant or named timeout/retry
|
||||
literal, the whole test tree (nightly- and Heavy-only files included) is
|
||||
searched for the old value; a consumer the branch did not update fails with
|
||||
`file:line`.
|
||||
`file:line`. A version-history row, a line that starts a bracketed row with
|
||||
the old value (`[45, "<digest>"]`), is skipped when the same table (a run of
|
||||
consecutive lines starting with `[`) also holds a row for the new value that
|
||||
the branch added. A table the branch did not extend, a table that already
|
||||
held the new row at the merge base, and every other line holding the old
|
||||
value still fail. Rows in object form (`{ v: 45 }`) or spread over several
|
||||
lines are not recognized as history rows.
|
||||
|
||||
- `timing`: added or changed test lines that compare an elapsed wall-clock or
|
||||
CPU time against a bound, or sleep and then assert, print
|
||||
|
||||
@@ -66,10 +66,20 @@ function unquote(v: string): string { return /^['"`]/.test(v) ? v.slice(1, -1) :
|
||||
* English word in half the test tree, and a one-character old value (`2`)
|
||||
* sits in every test, so that value needs the constant's name (a wrapper
|
||||
* version 2 → 3 flagged 575 consumers through the stem alone).
|
||||
*
|
||||
* A version-history row (`[45, "<digest>"]`, the old value first in a
|
||||
* bracketed row) is skipped when the same table, a run of consecutive
|
||||
* `[…` lines, holds a new-value row that `baseText` (the file at the merge
|
||||
* base, null for a new file) lacks: the branch extended the table. A table it
|
||||
* did not extend, or one that already held the new row, still counts, and the
|
||||
* scan goes on past skipped rows to the first real consumer.
|
||||
*/
|
||||
export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral): string[] {
|
||||
export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null): string[] {
|
||||
const value = unquote(change.old);
|
||||
const valueRe = /^-?\d/.test(value) ? new RegExp(`(?<![\\w.])${escape(value)}(?![\\w.])`) : new RegExp(`['"\`]${escape(value)}['"\`]`);
|
||||
const literal = (v: string) => /^-?\d/.test(v) ? escape(v) : `['"\`]${escape(v)}['"\`]`;
|
||||
const valueRe = /^-?\d/.test(value) ? new RegExp(`(?<![\\w.])${escape(value)}(?![\\w.])`) : new RegExp(literal(value));
|
||||
const oldRowRe = new RegExp(`^\\s*\\[\\s*${literal(value)}\\s*,`);
|
||||
const newRowRe = new RegExp(`^\\s*\\[\\s*${literal(unquote(change.new))}\\s*,`, 'm');
|
||||
const base = path.basename(change.file);
|
||||
const stem = base.replace(/\.[^.]+$/, '');
|
||||
const moduleRe = new RegExp(`(?:(?:^|[/\\\\])${escape(stem)}|(?<![\\w-])${escape(base)})(?![\\w-])`, 'm');
|
||||
@@ -81,7 +91,16 @@ export function consumersOf(repoRoot: string, testFiles: readonly string[], chan
|
||||
try { text = fs.readFileSync(path.join(repoRoot, rel), 'utf8'); } catch { continue; }
|
||||
if (!(nameRe.test(text) || (!nameRequired && moduleRe.test(text)))) continue;
|
||||
const lines = text.split('\n');
|
||||
for (let i = 0; i < lines.length; i++) if (valueRe.test(lines[i]!)) { hits.push(`${rel}:${i + 1}`); break; }
|
||||
let extended: boolean | undefined;
|
||||
const skip = (i: number) => {
|
||||
if (!oldRowRe.test(lines[i]!)) return false;
|
||||
let a = i; while (a > 0 && /^\s*\[/.test(lines[a - 1]!)) a--;
|
||||
let b = i; while (b < lines.length - 1 && /^\s*\[/.test(lines[b + 1]!)) b++;
|
||||
if (!lines.slice(a, b + 1).some(l => newRowRe.test(l))) return false;
|
||||
extended ??= baseText !== undefined && !newRowRe.test(baseText(rel) ?? '');
|
||||
return extended;
|
||||
};
|
||||
for (let i = 0; i < lines.length; i++) if (valueRe.test(lines[i]!) && !skip(i)) { hits.push(`${rel}:${i + 1}`); break; }
|
||||
}
|
||||
return hits;
|
||||
}
|
||||
@@ -101,7 +120,10 @@ function run(ctx: PregateContext): CheckResult {
|
||||
const testFiles = listRepoFiles(ctx.repoRoot).filter(f => scansLiterals(f) && !f.startsWith('bin/'));
|
||||
const lines: string[] = [];
|
||||
for (const c of changes) {
|
||||
const hits = consumersOf(ctx.repoRoot, testFiles, c);
|
||||
const hits = consumersOf(ctx.repoRoot, testFiles, c, rel => {
|
||||
const r = spawnSync('git', ['show', `${ctx.mergeBase}:${rel}`], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024, env: ctx.env });
|
||||
return r.status === 0 ? r.stdout : null;
|
||||
});
|
||||
ctx.explain.push(`literals: ${c.file} ${c.name} ${c.old} → ${c.new}: ${hits.length} test file(s) still carry ${c.old}`);
|
||||
for (const h of hits) lines.push(`${h}: ${c.name} still ${c.old} (now ${c.new} in ${c.file})`);
|
||||
}
|
||||
|
||||
@@ -223,6 +223,34 @@ describe('literals (pure)', () => {
|
||||
expect(consumersOf(root, all, { name: 'TIMEOUT_MS', old: '5000', new: '9000', file: 'lib/retry.js' })).toEqual(['test/retry.test.ts:1', 'test/retry-path.test.ts:2', 'test/retry-bare.test.ts:2']);
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
test('consumersOf: a history row is skipped only when this branch added the new-value row to the same table; other old-value lines still count (GST-344)', () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-history-'));
|
||||
const nl = String.fromCharCode(10);
|
||||
const head = "import { PROOF_CHECKS } from '../src/proof-checks.ts';";
|
||||
const rows = (...ns: number[]) => ns.map(n => ` [${n}, "digest-${n}"],`);
|
||||
const files: Record<string, [string[], string[] | null]> = {
|
||||
'test/history.test.ts': [[head, 'const versions = [', ...rows(44, 45, 46), '];', 'expect(PROOF_CHECKS).toBe(46);'], [head, 'const versions = [', ...rows(44, 45), '];', 'expect(PROOF_CHECKS).toBe(45);']],
|
||||
'test/newest-first.test.ts': [[head, 'const versions = [', ...rows(46, 45, 44), '];'], [head, 'const versions = [', ...rows(45, 44), '];']],
|
||||
'test/unextended.test.ts': [[head, 'const versions = [', ...rows(44, 45), '];'], [head, 'const versions = [', ...rows(44, 45), '];']],
|
||||
'test/stale-later.test.ts': [[head, 'const versions = [', ...rows(44, 45, 46), '];', 'expect(PROOF_CHECKS).toBe(45);'], [head, 'const versions = [', ...rows(44, 45), '];', 'expect(PROOF_CHECKS).toBe(45);']],
|
||||
'test/two-tables.test.ts': [[head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45, 46), '];'], [head, 'const a = [', ...rows(44, 45), '];', 'const b = [', ...rows(45), '];']],
|
||||
'test/boundary.test.ts': [[head, 'test.each([', ...rows(45, 46), '])("%d", () => {});'], [head, 'test.each([', ...rows(45, 46), '])("%d", () => {});']],
|
||||
'test/new-file.test.ts': [[head, 'const versions = [', ...rows(45, 46), '];'], null],
|
||||
};
|
||||
for (const [rel, [now]] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), now.join(nl) + nl); }
|
||||
const baseText = (rel: string) => files[rel]?.[1]?.join(nl) ?? null;
|
||||
const lineOf = (rel: string, text: string) => `${rel}:${files[rel]![0].indexOf(text) + 1}`;
|
||||
expect(consumersOf(root, Object.keys(files), { name: 'PROOF_CHECKS', old: '45', new: '46', file: 'src/proof-checks.ts' }, baseText).sort()).toEqual([
|
||||
lineOf('test/boundary.test.ts', ' [45, "digest-45"],'),
|
||||
lineOf('test/stale-later.test.ts', 'expect(PROOF_CHECKS).toBe(45);'),
|
||||
lineOf('test/two-tables.test.ts', ' [45, "digest-45"],'),
|
||||
lineOf('test/unextended.test.ts', ' [45, "digest-45"],'),
|
||||
].sort());
|
||||
fs.writeFileSync(path.join(root, 'test/versions.test.ts'), ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", ' ["v4", \'b\'],', '];', ''].join(nl));
|
||||
const strings = (rel: string) => rel === 'test/versions.test.ts' ? ["import { SCHEMA } from '../src/schema.ts';", 'const history = [', " ['v3', 'a'],", '];'].join(nl) : null;
|
||||
expect(consumersOf(root, ['test/versions.test.ts'], { name: 'SCHEMA', old: "'v3'", new: "'v4'", file: 'src/schema.ts' }, strings)).toEqual([]);
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
test('runner summaries: bun, node --test, vitest, jest, pytest, zero-run', () => {
|
||||
expect(parseSummary(' 12 pass\n 1 fail\nRan 13 tests across 2 files.')).toEqual({ ran: 13, failed: 1 });
|
||||
expect(parseSummary('# tests 4\n# pass 4\n# fail 0')).toEqual({ ran: 4, failed: 0 });
|
||||
@@ -369,6 +397,20 @@ describe('gstack-pregate (bin)', () => {
|
||||
expect(pregate(repo, ['--stage', 'preflight', '--only', 'literals']).status).toBe(EXIT.ok);
|
||||
});
|
||||
|
||||
test('GST-344: a history table the branch extends passes literals through the merge-base read; one it does not extend fails', () => {
|
||||
const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => ` [${n}, 'digest-${n}'],`), '];', ''].join('\n');
|
||||
const repo = fixture({ baseFiles: { 'test/history.test.js': history(2, 3) } });
|
||||
const bump = { 'lib/retry.js': "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n", 'test/retry.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { MAX_RETRIES } = require('../lib/retry.js');\ntest('retries four times', () => assert.strictEqual(MAX_RETRIES, 4));\n" };
|
||||
commit(repo, bump);
|
||||
const stale = pregate(repo, ['--stage', 'preflight', '--only', 'literals']);
|
||||
expect(stale.status).toBe(EXIT.fail);
|
||||
expect(stale.out).toContain('literals: test/history.test.js:4: MAX_RETRIES still 3 (now 4 in lib/retry.js)');
|
||||
commit(repo, { 'test/history.test.js': history(2, 3, 4) });
|
||||
const r = pregate(repo, ['--stage', 'preflight', '--only', 'literals']);
|
||||
expect(r.out).toMatch(/^preflight {2}literals {2}pass /m);
|
||||
expect(r.status).toBe(EXIT.ok);
|
||||
});
|
||||
|
||||
test('secrets: a credential in an added line fails and cannot be downgraded; a committed-then-removed line is clean', () => {
|
||||
const repo = fixture({ pregate: { ...BASE_PREGATE, warn: ['secrets'] } });
|
||||
const bad = pregate(repo, ['--stage', 'preflight', '--only', 'secrets']);
|
||||
|
||||
Reference in new issue
Block a user