mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 23:37:53 +01:00
fix(pregate): strays matches only the documented scratch names; literals ignores a named literal the branch only adds (GST-1211)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
bf2bcf14b0
commit
fe05b3ccf5
6 files changed
+62
-9
No files matched your search
@@ -0,0 +1,8 @@
|
||||
**Two `gstack-pregate` preflight false positives fixed (GST-1211).** Both stopped /ship on real work in gbrain-evals.
|
||||
|
||||
### Fixed
|
||||
- **`strays`:** the scratch-name check matched three prefixes the docs never listed (`temp`, `debug-` and `probe`), so real files like `probe.py` and `temporal-graph-shipped.test.ts` failed preflight. It now matches only the documented names: `zz-*`, `tmp*`, `scratch*`, plus editor leftovers (`*.orig`, `*.rej`, `*.swp`, `*.swo`, `*.bak`, `*.tmp`, `*~`, `.DS_Store`, `Thumbs.db`), which docs/pregate.md and docs/troubleshooting.md now list too.
|
||||
- **`literals`:** a named literal the branch only adds, such as `limit: 300` in a new object, was read as an existing `limit: 10` elsewhere in the file changing to 300, and every test that mentioned 10 was flagged. The check now compares every value under a name, so an added literal isn't a change. Editing an existing value still fails, and so does changing one of two equal values under the same name, which the old last-value check could miss.
|
||||
|
||||
### Tests
|
||||
- `test/pregate-selection.test.ts`: `probe.py` and `temporal-*.test.ts` pass `strays` while `zz-notes.md`, `tmp-x.ts`, `scratch.py` and `util.js.orig` still fail; a new object literal beside `limit: 10` passes `literals` while editing that value still fails; the pure `changedLiterals` case keeps an exported constant change.
|
||||
+5
-2
@@ -201,9 +201,12 @@ Two stages with separate clocks, both printed:
|
||||
file is read whole from disk. A file over the size limit, or whose full diff
|
||||
does not parse, is scanned on its added lines alone, without that context.
|
||||
- `strays`: a new file at the repository root, a new top-level directory, or a
|
||||
scratch name (`zz-*`, `tmp*`, `scratch*`) anywhere.
|
||||
scratch name (`zz-*`, `tmp*`, `scratch*`) or editor leftover (`*.orig`,
|
||||
`*.rej`, `*.swp`, `*.swo`, `*.bak`, `*.tmp`, `*~`, `.DS_Store`, `Thumbs.db`)
|
||||
anywhere.
|
||||
- `literals`: for every changed exported constant or named timeout/retry
|
||||
literal, the whole test tree (nightly- and Heavy-only files included) is
|
||||
literal (a named literal the branch only adds, such as `limit: 300` in a new
|
||||
object, is not a change), the whole test tree (nightly- and Heavy-only files included) is
|
||||
searched for the old value in files that name the constant or, when the old
|
||||
value is longer than one character, its module (the module's file name not
|
||||
followed by another extension, or a path ending
|
||||
|
||||
@@ -3770,7 +3770,9 @@ anchored pattern and a reason ([docs/pregate.md](pregate.md#allowlist));
|
||||
### `PREGATE_STRAYS`
|
||||
|
||||
**Meaning.** A file was added at the repository root, in a new top-level
|
||||
directory, or under a scratch name (`zz-*`, `tmp*`, `scratch*`).
|
||||
directory, or under a scratch name (`zz-*`, `tmp*`, `scratch*`) or an editor
|
||||
leftover name (`*.orig`, `*.rej`, `*.swp`, `*.swo`, `*.bak`, `*.tmp`, `*~`,
|
||||
`.DS_Store`, `Thumbs.db`).
|
||||
|
||||
**Fix.** Delete or move it. An intentional root file goes in `strays.allow`.
|
||||
|
||||
|
||||
@@ -57,10 +57,10 @@ export function constantsIn(text: string): Map<string, string> {
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Named numeric call sites (`timeout: 5000`) keyed by name; the last value per name wins, which is enough to spot a change. */
|
||||
export function callLiteralsIn(text: string): Map<string, string> {
|
||||
const out = new Map<string, string>();
|
||||
for (const m of text.matchAll(CALL_PATTERN)) out.set(m[1]!, m[2]!);
|
||||
/** Named numeric call sites (`timeout: 5000`) keyed by name, every value in source order. */
|
||||
export function callLiteralsIn(text: string): Map<string, string[]> {
|
||||
const out = new Map<string, string[]>();
|
||||
for (const m of text.matchAll(CALL_PATTERN)) out.set(m[1]!, [...(out.get(m[1]!) ?? []), m[2]!]);
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -69,7 +69,13 @@ export function changedLiterals(file: string, oldText: string, newText: string):
|
||||
const oldC = constantsIn(oldText); const newC = constantsIn(newText);
|
||||
for (const [name, old] of oldC) { const now = newC.get(name); if (now !== undefined && now !== old) out.push({ name, old, new: now, file }); }
|
||||
const oldCall = callLiteralsIn(oldText); const newCall = callLiteralsIn(newText);
|
||||
for (const [name, old] of oldCall) { const now = newCall.get(name); if (now !== undefined && now !== old && !oldC.has(name)) out.push({ name, old, new: now, file }); }
|
||||
for (const [name, olds] of oldCall) {
|
||||
if (oldC.has(name)) continue;
|
||||
const added = [...(newCall.get(name) ?? [])];
|
||||
const removed = olds.filter(v => { const i = added.indexOf(v); if (i >= 0) added.splice(i, 1); return i < 0; });
|
||||
const seen = new Set<string>();
|
||||
removed.forEach((old, i) => { if (i < added.length && !seen.has(old)) { seen.add(old); out.push({ name, old, new: added[i]!, file }); } });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ import type { CheckDef, CheckResult, PregateContext } from '../types';
|
||||
|
||||
/** Conventional root files a repo adds without a declaration. */
|
||||
export const ROOT_OK = /^(?:README|LICENSE|LICENCE|NOTICE|CHANGELOG|CONTRIBUTING|CODE_OF_CONDUCT|SECURITY|AUTHORS|VERSION|Makefile|Dockerfile|Procfile|Gemfile|Rakefile|Cargo\.(?:toml|lock)|go\.(?:mod|sum)|package(?:-lock)?\.json|bun\.lockb?|pnpm-lock\.yaml|yarn\.lock|tsconfig[\w.-]*\.json|bunfig\.toml|pyproject\.toml|pubspec\.(?:yaml|lock)|analysis_options\.yaml|dart_test\.yaml|requirements[\w.-]*\.txt|setup\.(?:py|cfg)|\.[\w.-]+|[\w.-]+\.config\.[cm]?[jt]s|[\w.-]+\.md)$/i;
|
||||
export const STRAY_NAME = /^(?:zz-|tmp|temp|scratch|debug-|probe)|\.(?:orig|rej|swp|swo|bak|tmp)$|~$|^\.DS_Store$|^Thumbs\.db$/i;
|
||||
export const STRAY_NAME = /^(?:zz-|tmp|scratch)|\.(?:orig|rej|swp|swo|bak|tmp)$|~$|^\.DS_Store$|^Thumbs\.db$/i;
|
||||
|
||||
function run(ctx: PregateContext): CheckResult {
|
||||
const inputs = baseInputs(ctx);
|
||||
|
||||
@@ -201,6 +201,13 @@ describe('literals (pure)', () => {
|
||||
{ name: 'MAX', old: '3', new: '4', file: 'f.ts' }, { name: 'timeout', old: '5000', new: '9000', file: 'f.ts' },
|
||||
]);
|
||||
});
|
||||
|
||||
test('GST-1211: a key inside a newly added object literal is not a changed literal; an edited call-site value and an exported constant still are', () => {
|
||||
const before = 'export const MAX = 3;\nconst page = { limit: 10 };\n';
|
||||
expect(changedLiterals('f.ts', before, `${before}const query = { selection: { order: 'asc', limit: 300 } };\n`)).toEqual([]);
|
||||
expect(changedLiterals('f.ts', before, 'export const MAX = 3;\nconst page = { limit: 20 };\n')).toEqual([{ name: 'limit', old: '10', new: '20', file: 'f.ts' }]);
|
||||
expect(changedLiterals('f.ts', before, 'export const MAX = 4;\nconst page = { limit: 10 };\nconst query = { limit: 300 };\n')).toEqual([{ name: 'MAX', old: '3', new: '4', file: 'f.ts' }]);
|
||||
});
|
||||
// Value: protects=a sibling file that shares the module's stem (`postgres-unit-arms.txt` beside `postgres-unit-arms.ts`) is not the
|
||||
// module, so a test that only names it is not a consumer, while an extensionless import, a JS/TS import extension, the module's own
|
||||
// file name and a path ending at the stem (then a quote, space, bracket, sentence end or `.d.ts`) still are, and neither `<file>.snap` nor a JS twin of a
|
||||
@@ -519,6 +526,21 @@ describe('gstack-pregate (bin)', () => {
|
||||
expect(w.out).toMatch(/^preflight {2}strays {4}warn .*downgraded to warn by origin\/main@/m);
|
||||
});
|
||||
|
||||
test('GST-1211: probe.py and temporal-*.test.ts are not scratch names; zz-*, tmp*, scratch* and editor leftovers still are', () => {
|
||||
const repo = fixture({ pregate: { ...BASE_PREGATE, warn: [] } });
|
||||
commit(repo, { 'docs/comparison-systems/ext-temporal-graph/shipped-0.31/probe.py': 'print(1)\n', 'test/eval/temporal-graph-shipped.test.ts': 'export {};\n' });
|
||||
const ok = pregate(repo, ['--stage', 'preflight', '--only', 'strays']);
|
||||
expect(ok.status).toBe(EXIT.ok);
|
||||
expect(ok.out).toMatch(/^preflight {2}strays {4}pass /m);
|
||||
commit(repo, { 'docs/zz-notes.md': '# n\n', 'lib/tmp-x.ts': 'export {};\n', 'test/scratch.py': 'pass\n', 'lib/util.js.orig': 'x\n' });
|
||||
const r = pregate(repo, ['--stage', 'preflight', '--only', 'strays']);
|
||||
expect(r.status).toBe(EXIT.fail);
|
||||
expect(r.out).toMatch(/^preflight {2}strays {4}fail .*4 stray file\(s\)/m);
|
||||
for (const f of ['docs/zz-notes.md', 'lib/tmp-x.ts', 'test/scratch.py', 'lib/util.js.orig']) expect(r.out).toContain(`strays: ${f}: scratch name (${f.split('/').pop()})`);
|
||||
expect(r.out).not.toContain('probe.py: scratch name');
|
||||
expect(r.out).not.toContain('temporal-graph-shipped.test.ts: scratch name');
|
||||
});
|
||||
|
||||
test('p0-4-heavy-old-constant: a nightly-only test still asserting the old literal fails literals with file:line', () => {
|
||||
const repo = fixture();
|
||||
commit(repo, { 'lib/retry.js': "exports.MAX_RETRIES = 3;\nexports.TIMEOUT_MS = 9000;\nexports.retry = (fn) => fn();\n" });
|
||||
@@ -532,6 +554,18 @@ describe('gstack-pregate (bin)', () => {
|
||||
expect(pregate(repo, ['--stage', 'preflight', '--only', 'literals']).status).toBe(EXIT.ok);
|
||||
});
|
||||
|
||||
test('GST-1211: a new object literal beside an existing `limit: 10` passes literals; editing that existing value still fails', () => {
|
||||
const repo = fixture({ baseFiles: { 'lib/query.js': 'exports.page = () => ({ limit: 10 });\n', 'test/query.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { page } = require('../lib/query.js');\ntest('pages by ten', () => assert.strictEqual(page().limit, 10));\n" } });
|
||||
commit(repo, { 'lib/query.js': "exports.page = () => ({ limit: 10 });\nexports.selection = { order: 'asc', limit: 300 };\n" });
|
||||
const ok = pregate(repo, ['--stage', 'preflight', '--only', 'literals']);
|
||||
expect(ok.status).toBe(EXIT.ok);
|
||||
expect(ok.out).not.toContain('test/query.test.js');
|
||||
commit(repo, { 'lib/query.js': "exports.page = () => ({ limit: 20 });\nexports.selection = { order: 'asc', limit: 300 };\n" });
|
||||
const r = pregate(repo, ['--stage', 'preflight', '--only', 'literals']);
|
||||
expect(r.status).toBe(EXIT.fail);
|
||||
expect(r.out).toContain('literals: test/query.test.js:4: limit still 10 (now 20 in lib/query.js)');
|
||||
});
|
||||
|
||||
// Value: protects=a real stale one-character constant still fails literals end to end after the #3492 narrowing, and an incidental
|
||||
// digit in the same file is not what it reports; fails_when=the statement binding drops a named assertion; why_new=only
|
||||
// multi-character values had a bin-level control; seam=none; no_claim=two-character values
|
||||
|
||||
Reference in new issue
Block a user