mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-13 03:48:06 +01:00
fix(pregate): basename mentions and Go/Ruby test renames fail closed
Fix-check 2 (cycle 4, rule 23 grant from GStack Minder): a deleted file a tested loader reached by path.join(__dirname, 'lib', 'value.js') was waived, and a Go helper test renamed into another directory got the test-rename exemption. namedBy now greps the basename repo-wide (non-Markdown); the Go and Ruby test refusal runs before the rename exemption. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
e063ed28b9
commit
99cd40d27c
4 files changed
+44
-27
No files matched your search
@@ -106,20 +106,13 @@ function readWorkingTree(repoRoot: string, rel: string): { text: string | null;
|
||||
function outputsOf(all: readonly string[], globs: readonly string[]): string[] { return all.filter(f => matchesAny(globs, f)); }
|
||||
|
||||
/**
|
||||
* Tracked files outside Markdown that name `file` where the import graph cannot see (a config, a script or a workflow): by its
|
||||
* repo path anywhere, or by its path relative to an ancestor directory from a file under that directory (`preload = ["./setup.ts"]`).
|
||||
* Tracked files outside Markdown that mention `file`'s basename, where the import graph cannot see: a config naming it relative to
|
||||
* itself (`preload = ["./setup.ts"]`), a computed path (`path.join(__dirname, 'lib', 'value.js')`), a script or a workflow.
|
||||
*/
|
||||
function namedBy(ctx: PregateContext, file: string): string[] {
|
||||
const parts = file.split('/');
|
||||
const hits = new Set<string>();
|
||||
for (let i = 0; i < parts.length; i++) {
|
||||
const scope = parts.slice(0, i).join('/') || '.';
|
||||
const r = spawnSync('git', ['grep', '--untracked', '-l', '-F', '-e', parts.slice(i).join('/'), '--', scope, ':(exclude)*.md', ':(exclude)*.mdx'], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 60_000, maxBuffer: 16 * 1024 * 1024, env: ctx.env });
|
||||
if (r.status === 1) continue;
|
||||
if (r.status !== 0) return ['(git grep failed)'];
|
||||
for (const f of (r.stdout ?? '').split('\n')) if (f) hits.add(f);
|
||||
}
|
||||
return [...hits].sort();
|
||||
const r = spawnSync('git', ['grep', '--untracked', '-l', '-F', '-e', path.posix.basename(file), '--', '.', ':(exclude)*.md', ':(exclude)*.mdx'], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 60_000, maxBuffer: 16 * 1024 * 1024, env: ctx.env });
|
||||
if (r.status === 1) return [];
|
||||
return r.status === 0 ? (r.stdout ?? '').split('\n').filter(Boolean) : ['(git grep failed)'];
|
||||
}
|
||||
|
||||
/** Languages whose importers the graph resolves file by file; Go packages, Ruby requires, shell `source`, Rust modules and bins are not. */
|
||||
@@ -131,17 +124,18 @@ const shown = (files: readonly string[]) => `${files.slice(0, 8).join(' ')}${fil
|
||||
|
||||
/**
|
||||
* Why a removed path no remaining test selects owes no lane, and the owning-suite tests it selects, or `refused` when it must
|
||||
* fail closed as no lane. Nothing is waived while a remaining file imports it, directly or through a directory reference. A
|
||||
* test renamed to a test defers to its new name. A deleted JS, TS, Python, Dart or bats test, or one renamed to a non-test
|
||||
* path, runs the tests that directly exercise the files it exercised and names those left with no test at all. Any other
|
||||
* removed file, a rename's old name included, owes nothing only when it is JS, TS, Python or Dart outside fixture, golden and
|
||||
* CI trees, not loaded by name, and no remaining non-Markdown file names it: deleting a manifest, a config, a workflow or a Go
|
||||
* file owes what editing it owes.
|
||||
* fail closed as no lane. Nothing is waived while a remaining file imports it, directly or through a directory reference, nor
|
||||
* is a Go or Ruby test (it may define helpers its package still uses). Any other test renamed to a test defers to its new
|
||||
* name. A deleted JS, TS, Python, Dart or bats test, or one renamed to a non-test path, runs the tests that directly exercise
|
||||
* the files it exercised and names those left with no test at all. Any other removed file, a rename's old name included, owes
|
||||
* nothing only when it is JS, TS, Python or Dart outside fixture, golden and CI trees, not loaded by name, and no remaining
|
||||
* non-Markdown file mentions its basename: deleting a manifest, a config, a workflow or a Go file owes what editing it owes.
|
||||
*/
|
||||
function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet<string>, file: string): { removed: Removed; suite: string[] } | { refused: string } {
|
||||
const edges = [...graph.reverse].filter(([k]) => k === file || (k.endsWith('/**') && file.startsWith(k.slice(0, -2)))).flatMap(([, list]) => list);
|
||||
const importers = [...new Set(edges.map(e => e.from))].filter(f => !gone.has(f) && path.posix.basename(f) !== 'Cargo.toml').sort();
|
||||
if (importers.length) return { refused: `removed, but still imported by ${shown(importers)}` };
|
||||
if (isTestFile(file) && !RESOLVED_IMPORTS.test(file) && !file.endsWith('.bats')) return { refused: 'removed, but a Go or Ruby test may define helpers its package still uses, which the graph cannot see' };
|
||||
const renamedTo = ctx.touched.find(t => t.renamedFrom === file)?.path;
|
||||
if (renamedTo && isTestFile(file) && isTestFile(renamedTo)) return { removed: { file, note: `old name of ${renamedTo}`, lostCoverage: [] }, suite: [] };
|
||||
if (!isTestFile(file)) {
|
||||
@@ -150,7 +144,6 @@ function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet<string
|
||||
if (names.length) return { refused: `removed, but still named by ${shown(names)}` };
|
||||
return { removed: { file, note: renamedTo ? `old name of ${renamedTo}; nothing in the tree references it` : 'nothing in the tree references it', lostCoverage: [] }, suite: [] };
|
||||
}
|
||||
if (!RESOLVED_IMPORTS.test(file) && !file.endsWith('.bats')) return { refused: 'removed, but a Go or Ruby test may define helpers its package still uses, which the graph cannot see' };
|
||||
const subjects = graph.goneSubjects?.get(file);
|
||||
if (!subjects) return { refused: 'removed test whose merge-base text could not be read (not a regular file there, invalid UTF-8 or over 1 MiB)' };
|
||||
if (!subjects.length) return { removed: { file, note: 'it exercised no file still in the tree', lostCoverage: [] }, suite: [] };
|
||||
|
||||
Reference in new issue
Block a user