mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 02:30:39 +01:00
fix: pregate waives only removed tests; other removals owe what an edit owes (D3 narrowed by the controller)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
5a1577b3db
commit
39fce52f2d
8 files changed
+120
-165
No files matched your search
+4
-4
@@ -2,17 +2,17 @@
|
||||
|
||||
## [1.91.292.0] - 2026-10-28
|
||||
|
||||
**Deleting or renaming a test no longer fails pregate with PREGATE_NO_LANE.** A PR that removed a test file, or renamed one, failed `lanes` because the removed path had no test lane of its own, so cleanup PRs needed a hand-written `dependencies` entry to pass (#3529). Pregate now accounts for a removed path the way it accounts for an edit. A renamed test runs under its new name, a deleted test runs the remaining tests of the files it exercised, and a deleted JS, TS, Python or Dart file nothing still imports or names is waived. Every waived path is printed on its own line, so the reason is visible in the gate output.
|
||||
**Deleting or renaming a test no longer fails pregate with PREGATE_NO_LANE.** A PR that removed a test file, or renamed one, failed `lanes` because the removed path had no test lane of its own, so cleanup PRs needed a hand-written `dependencies` entry to pass (#3529). Pregate now accounts for a removed test the way it accounts for an edit. A renamed test runs under its new name, a deleted test runs the remaining tests of the files it exercised, and either one runs the tests of any file that still names it, such as a curation list. Every waived test is printed on its own line, so the reason is visible in the gate output. Removing any other file still needs a lane, as before.
|
||||
|
||||
### Fixed
|
||||
- **Deleted and renamed tests pass `lanes`**: each removed path that owes no lane prints `removed (no lane owed): <path> — <why>`, and the `lanes` detail counts `N removed path(s)`. A deleted JS, TS, Python, Dart or bats test (or a test renamed to a non-test path) runs its owning suite, the remaining tests that directly exercise the files its merge-base text exercised (`owning suite runs: …`); test helpers and fixtures are not subjects. Any removed test also runs the tests of each remaining non-Markdown file that still names it, such as a curation list (`files naming it run: …`), and fails closed when that file has no test.
|
||||
- **Deleted and renamed tests pass `lanes`**: each removed JS, TS, Python, Dart or bats test that nothing remaining imports prints `removed (no lane owed): <path> — <why>`, and the `lanes` detail counts `N removed path(s)`. A deleted test (or a test renamed to a non-test path) runs its owning suite, the remaining tests that directly exercise the files its merge-base text exercised (`owning suite runs: …`); test helpers and fixtures are not subjects. Any removed test also runs the tests of each remaining non-Markdown file that still names it (`files naming it run: …`), and fails closed when that file has no test.
|
||||
- **Lost coverage is named, not hidden**: when a deleted test was the last test of a file, the line says `coverage removed: <file> has no remaining test` and the detail adds `M subject(s) lost their last test`. A modified file whose only test the branch deleted still fails PREGATE_NO_LANE.
|
||||
|
||||
### Changed
|
||||
- A removal owes what an edit owes. These still fail PREGATE_NO_LANE until a test or a `dependencies` entry covers them. That covers a deleted or renamed manifest, lockfile, config, fixture, golden or workflow (renaming `package.json` into Markdown does not waive it), a file a runner loads by name (`*.config.ts`, `conftest.py`, `sitecustomize.py`, `__init__.py`), a Go, Ruby, shell, Rust or bin file and a deleted or renamed Go or Ruby test (the graph cannot see their importers or shared helpers), a file remaining code still imports, including by a relative, extensionless or dotted import or through a directory it reads (`removed, but still imported by …`), a file whose basename a remaining non-Markdown file mentions (a config's `preload = ["./setup.ts"]`, a computed `path.join(__dirname, 'lib', 'value.js')`), and a deleted test whose merge-base text cannot be read (a symlink, invalid UTF-8, over 1 MiB). A remaining test that reads a deleted file or directory is still selected.
|
||||
- Only removed tests are waived. These still fail PREGATE_NO_LANE until a test or a `dependencies` entry covers them: any removed non-test path, a renamed source file's old name included; a removed test that remaining code imports; a deleted or renamed Go or Ruby test (it may define helpers its package still uses); and a deleted test whose merge-base text cannot be read (a symlink, invalid UTF-8, over 1 MiB). A remaining test that reads a deleted file or directory is still selected.
|
||||
|
||||
### Gate integrity
|
||||
- New `test/pregate-removed-paths.test.ts` (41 cases) covers the waived paths and the fail-closed negative controls; no existing test, threshold or allowlist changed.
|
||||
- New `test/pregate-removed-paths.test.ts` (42 cases) covers the waived tests and the fail-closed negative controls; no existing test, threshold or allowlist changed.
|
||||
|
||||
## [1.91.291.0] - 2026-10-28
|
||||
|
||||
|
||||
+23
-38
@@ -195,44 +195,29 @@ Two stages with separate clocks, both printed:
|
||||
no lane, and a workspace-only Cargo.toml or Cargo.lock needs a `dependencies` entry),
|
||||
transitively through
|
||||
non-test files; deleted and renamed files select the tests that named them.
|
||||
A removed path (a deleted file, or a rename's old name) that no remaining
|
||||
test selects owes no lane when one of these holds, and prints
|
||||
`removed (no lane owed): <path> — <why>` (the `lanes` detail adds
|
||||
`N removed path(s)`). A JS, TS, Python, Dart or bats test renamed to another
|
||||
test path defers to its new name (`old name of <new>`). A deleted JS, TS, Python, Dart or bats test, or a
|
||||
test renamed to a non-test path, runs its owning suite, the remaining tests
|
||||
that directly exercise the files its merge-base text exercised (`owning suite
|
||||
runs: …`; test helpers and fixtures are not subjects), and names each file
|
||||
left with no test at all (`coverage removed: <file> has no remaining test`,
|
||||
and `M subject(s) lost their last test` in the detail). Either kind of
|
||||
removed test also runs the tests of each remaining non-Markdown file that
|
||||
mentions its basename, such as a curation list (`files naming it run: …`),
|
||||
and fails closed when such a file has no test (`still named by …, which no
|
||||
test covers`). A deleted JS, TS,
|
||||
Python or Dart file, or the old name of a renamed one, outside fixture,
|
||||
snapshot, golden, baseline and CI trees and `.gstack/` reads `nothing in
|
||||
the tree references it` when no remaining file imports it (directly or
|
||||
through a directory it reads), it is not a file a runner loads by name
|
||||
(`*.config.ts`, `.*rc.js`, `conftest.py`, `setup.py`, `noxfile.py`,
|
||||
`sitecustomize.py`, `usercustomize.py`, `__init__.py`, `__main__.py`),
|
||||
and no remaining non-Markdown file mentions its basename (a config naming it
|
||||
relative to itself, `preload = ["./setup.ts"]`, or a computed path such as
|
||||
`path.join(__dirname, 'lib', 'value.js')`), so a common basename such as
|
||||
`index.ts` needs a `dependencies` entry.
|
||||
Every other removal owes what an edit owes, so it is still `PREGATE_NO_LANE`
|
||||
until a test or a `dependencies` entry covers it: a deleted or renamed
|
||||
manifest, lockfile, config, fixture, golden or workflow (renaming one into a
|
||||
waived path such as Markdown does not waive the old name), a Go, Ruby, shell,
|
||||
Rust or bin file (the graph does not resolve their importers file by file),
|
||||
a deleted or renamed Go or Ruby test (it may define helpers its package
|
||||
still uses), a file
|
||||
remaining code still imports (`still imported by …`, even by a relative or
|
||||
dotted import) or a non-Markdown file still names (`still named by …`), and a
|
||||
deleted test whose merge-base text cannot be read (a symlink, invalid UTF-8,
|
||||
over 1 MiB). A modified file whose only test the branch deleted still has no
|
||||
lane, a remaining test that reads a deleted file or directory is selected,
|
||||
and a deleted Rust module selects its crate's tests. /review lists a deleted
|
||||
test as a gate edit (RH-1) either way.
|
||||
A removed test (a deleted one, or a rename's old name) that no remaining
|
||||
test selects owes no lane, and prints `removed (no lane owed): <path> —
|
||||
<why>` (the `lanes` detail adds `N removed path(s)`), when it is a JS, TS,
|
||||
Python, Dart or bats test and no remaining file imports it (directly or
|
||||
through a directory it reads; otherwise `still imported by …`). It runs
|
||||
the tests of each remaining non-Markdown file that mentions its basename,
|
||||
such as a curation list (`files naming it run: …`), and fails closed when
|
||||
such a file has no test (`still named by …, which no test covers`). A test
|
||||
renamed to another test path then defers to its new name (`old name of
|
||||
<new>`). A deleted test, or one renamed to a non-test path, also runs its
|
||||
owning suite, the remaining tests that directly exercise the files its
|
||||
merge-base text exercised (`owning suite runs: …`; test helpers and
|
||||
fixtures are not subjects), and names each file left with no test at all
|
||||
(`coverage removed: <file> has no remaining test`, and `M subject(s) lost
|
||||
their last test` in the detail). Every other removal owes what an edit
|
||||
owes, as before: a deleted or renamed Go or Ruby test (it may define
|
||||
helpers its package still uses), a deleted test whose merge-base text
|
||||
cannot be read (a symlink, invalid UTF-8, over 1 MiB), and any removed
|
||||
non-test path, a renamed source file's old name included, are still
|
||||
`PREGATE_NO_LANE` until a test or a `dependencies` entry covers them. A
|
||||
modified file whose only test the branch deleted still has no lane, and a
|
||||
remaining test that reads a deleted file or directory is selected. /review
|
||||
lists a deleted test as a gate edit (RH-1) either way.
|
||||
An extensionless JS/TS import (relative, a tsconfig/jsconfig `paths` alias or
|
||||
`baseUrl`) tries the exact path, then `.ts`, `.tsx`, `.js`, `.jsx`, `.mjs`,
|
||||
`.cjs`, `.mts`, `.cts`, then `index.ts`, `index.tsx`, `index.js`, `index.jsx`
|
||||
|
||||
+12
-14
@@ -3211,20 +3211,18 @@ versions, line endings or any non-version byte changed, one under a test,
|
||||
fixture, golden, `.gstack/` or workflow tree, or one added, deleted, renamed
|
||||
or re-moded, is still this error; `--explain` prints `not version-stamp-only (<reason>)`
|
||||
for it.
|
||||
A path the branch removed is usually not this error either: a renamed test, a
|
||||
deleted JS, TS, Python, Dart or bats test, and a deleted JS, TS, Python or Dart
|
||||
file nothing in the tree still imports or names print `removed (no lane owed):
|
||||
<path> — <why>` instead (a deleted test runs its owning suite or says `coverage
|
||||
removed: <file> has no remaining test`). A removal owes what an edit owes, so
|
||||
these are still this error: a deleted or renamed manifest, lockfile, config,
|
||||
fixture, golden or workflow, a file a runner loads by name (`*.config.ts`,
|
||||
`conftest.py`, `sitecustomize.py`), a Go, Ruby, shell, Rust or bin file, a Go
|
||||
or Ruby test (deleted or renamed), a file remaining code still imports or whose
|
||||
basename a non-Markdown file still mentions, a deleted test whose merge-base
|
||||
text cannot be read (a symlink, invalid UTF-8, over 1 MiB), and a modified file
|
||||
whose only test the branch deleted. `--explain` prints the reason as
|
||||
`<path>: removed, but …` (`<path>: removed test whose merge-base text could
|
||||
not be read …` for an unreadable deleted test).
|
||||
A test the branch removed is usually not this error either: a deleted or
|
||||
renamed JS, TS, Python, Dart or bats test that nothing remaining imports prints
|
||||
`removed (no lane owed): <path> — <why>` instead. It runs the tests of files
|
||||
that still name it, and a deleted test runs its owning suite or says
|
||||
`coverage removed: <file> has no remaining test`. These are still this error:
|
||||
a removed test that remaining code imports or that a file with no test still
|
||||
names, a deleted or renamed Go or Ruby test, a deleted test whose merge-base
|
||||
text cannot be read (a symlink, invalid UTF-8, over 1 MiB), any other removed
|
||||
path (a renamed source file's old name included), and a modified file whose
|
||||
only test the branch deleted. `--explain` prints the reason as `<path>:
|
||||
removed, but …` (`<path>: removed test whose merge-base text could not be
|
||||
read …` for an unreadable deleted test).
|
||||
|
||||
**Fix.** Add a test that names the file, or declare its lane (an explicit `[]`
|
||||
for files with no behaviour) under `dependencies` in `.gstack/pregate.json` (or the user-owned file the message names).
|
||||
|
||||
@@ -13,12 +13,11 @@
|
||||
* A release manifest whose diff against the merge base changes only version
|
||||
* values is waived as version-stamp-only (lib/pregate/stamp-only.ts) and named
|
||||
* on its own line; any other change to it still needs a lane.
|
||||
* A removed path (deleted, or a rename's old name) no remaining test selects
|
||||
* owes no lane when it is a renamed or deleted test, or deleted JS, TS, Python
|
||||
* or Dart code nothing remaining imports or names, and is named on its own
|
||||
* `removed (no lane owed)` line; a deleted test runs its owning suite or names
|
||||
* the files that lost their last test (issue #3529). Any other removal owes
|
||||
* what an edit owes.
|
||||
* A removed test (deleted, or a rename's old name) no remaining test selects
|
||||
* owes no lane when nothing remaining imports it, and is named on its own
|
||||
* `removed (no lane owed)` line; it runs the tests of files that still name it,
|
||||
* and a deleted test runs its owning suite or names the files that lost their
|
||||
* last test (issue #3529). Any other removal owes what an edit owes.
|
||||
* A test that reads the file still selects that test.
|
||||
* Incidents: p0-4-pipe-hidden-failure (`false | tail` read as green),
|
||||
* p0-4-windows-lane-unrun (a Windows-only change merged with its lane unrun).
|
||||
|
||||
@@ -41,9 +41,6 @@ function isMarkdown(rel: string): boolean {
|
||||
return MARKDOWN.test(rel);
|
||||
}
|
||||
|
||||
/** Fixture, snapshot and golden trees, CI workflows and actions, and `.gstack/`: files a test or the CI may read by a path the import graph cannot see. */
|
||||
export function isFixtureOrCiTree(rel: string): boolean { return PROTECTED_TREE.test(rel.replace(/\\/g, '/').replace(/^\.\//, '')); }
|
||||
|
||||
function isTestTree(rel: string): boolean {
|
||||
return TEST_TREE.test(rel);
|
||||
}
|
||||
|
||||
@@ -493,8 +493,8 @@ export function buildGraph(repoRoot: string, opts: BuildOptions = {}): Graph {
|
||||
for (const { target, via } of referencesIn(files, dirs, rel, source, config, graph.tests.has(rel) ? unresolved : undefined, packages)) addEdge(graph, target, rel, via);
|
||||
if (unresolved.length) graph.unresolved!.set(rel, unresolved);
|
||||
}
|
||||
const rust = rustEdges([...files], rel => readRepoFile(repoRoot, rel, maxBytes));
|
||||
for (const t of rust.tests) if (!gone.has(t)) graph.tests.add(t);
|
||||
const rust = rustEdges(all, rel => readRepoFile(repoRoot, rel, maxBytes));
|
||||
for (const t of rust.tests) graph.tests.add(t);
|
||||
for (const e of rust.edges) addEdge(graph, e.dependency, e.dependent, e.via);
|
||||
const django = djangoEdges({ files: [...files], tests: graph.tests, read: rel => pySources.get(rel) ?? readRepoFile(repoRoot, rel, maxBytes), code: pythonCode, resolveDotted: dotted => pythonPatchTarget(files, dotted) });
|
||||
for (const e of django) addEdge(graph, e.dependency, e.dependent, 'framework');
|
||||
|
||||
@@ -12,7 +12,7 @@ import { GENERATED_PATH } from '../regen';
|
||||
import { runBashSync } from '../script-invocation';
|
||||
import { matchesAny } from '../ship-policy';
|
||||
import { PREGATE_CONFIG_PATH } from './config';
|
||||
import { isDocsEmptyLane, isFixtureOrCiTree, isInertNoTests, isProtectedTree } from './docs-lane';
|
||||
import { isDocsEmptyLane, isInertNoTests, isProtectedTree } from './docs-lane';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { buildGraph, isTestFile, listRepoFiles, testsFor, type Graph } from './import-graph';
|
||||
@@ -105,61 +105,46 @@ function readWorkingTree(repoRoot: string, rel: string): { text: string | null;
|
||||
/** Tracked files matching a registry entry's output globs. */
|
||||
function outputsOf(all: readonly string[], globs: readonly string[]): string[] { return all.filter(f => matchesAny(globs, f)); }
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
/** Tracked files outside Markdown that mention `file`'s basename, where the import graph cannot see: a curation list, a durations manifest, a script or a workflow. */
|
||||
function namedBy(ctx: PregateContext, file: string): string[] {
|
||||
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. */
|
||||
/** Test languages whose importers the graph resolves file by file; a Go or Ruby test may define helpers its package still uses. */
|
||||
const RESOLVED_IMPORTS = /\.(?:[cm]?[jt]sx?|py|dart)$/;
|
||||
/** Files a runner or interpreter loads by name, which no import or path mention points at. */
|
||||
const LOADED_BY_NAME = /(?:^|\/)(?:[^/]*\.config\.[cm]?[jt]s|\.[^/]*rc\.[cm]?js|(?:conftest|sitecustomize|usercustomize|setup|noxfile|__init__|__main__)\.py)$/;
|
||||
|
||||
const shown = (files: readonly string[]) => `${files.slice(0, 8).join(' ')}${files.length > 8 ? ` … +${files.length - 8} more` : ''}`;
|
||||
|
||||
/**
|
||||
* 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, nor
|
||||
* is a Go or Ruby test (it may define helpers its package still uses). A removed test also runs the tests of every remaining
|
||||
* non-Markdown file that names its basename (a curation list, a durations manifest), and is refused when such a file has no
|
||||
* test. Beyond that, 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.
|
||||
* 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.
|
||||
* Why a removed test no remaining test selects owes no lane, and the tests it selects, or `refused` when it must fail closed as
|
||||
* no lane. Only a removed JS, TS, Python, Dart or bats test is waived; any other removed path, a renamed source's old name
|
||||
* included, owes what an edit owes. Nothing is waived while a remaining file imports it, directly or through a directory
|
||||
* reference. A removed test runs the tests of every remaining non-Markdown file that names its basename (a curation list, a
|
||||
* durations manifest), and is refused when such a file has no test. A test renamed to a test then defers to its new name. A
|
||||
* deleted test, or one renamed to a non-test path, also runs the tests that directly exercise the files it exercised and names
|
||||
* those left with no test.
|
||||
*/
|
||||
function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet<string>, file: string): { removed: Removed; suite: string[] } | { refused: string } {
|
||||
if (!isTestFile(file)) return { refused: 'removed, but only a removed test is waived; any other removal owes what an edit owes' };
|
||||
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 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();
|
||||
const importers = [...new Set(edges.map(e => e.from))].filter(f => !gone.has(f)).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;
|
||||
const namerSuite = new Set<string>();
|
||||
if (isTestFile(file)) {
|
||||
const namers = namedBy(ctx, file).filter(f => !gone.has(f));
|
||||
const uncovered = namers.filter(n => {
|
||||
const m = testsFor(graph, n, ctx.config.dependencies);
|
||||
for (const t of m.tests) namerSuite.add(t);
|
||||
return !m.tests.length;
|
||||
});
|
||||
if (uncovered.length) return { refused: `removed, but still named by ${shown(uncovered)}, which no test covers` };
|
||||
}
|
||||
const namerNote = namerSuite.size ? `; files naming it run: ${shown([...namerSuite].sort())}` : '';
|
||||
if (renamedTo && isTestFile(file) && isTestFile(renamedTo)) return { removed: { file, note: `old name of ${renamedTo}${namerNote}`, lostCoverage: [] }, suite: [...namerSuite] };
|
||||
if (!isTestFile(file)) {
|
||||
if (!RESOLVED_IMPORTS.test(file) || isFixtureOrCiTree(file) || LOADED_BY_NAME.test(file)) return { refused: 'removed, but not JS, TS, Python or Dart code whose importers the graph resolves (a manifest, config, fixture, workflow, file loaded by name, or Go, Ruby, shell, Rust or bin file owes what an edit owes)' };
|
||||
const names = namedBy(ctx, file);
|
||||
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: [] };
|
||||
}
|
||||
const uncovered = namedBy(ctx, file).filter(f => !gone.has(f)).filter(n => {
|
||||
const m = testsFor(graph, n, ctx.config.dependencies);
|
||||
for (const t of m.tests) namerSuite.add(t);
|
||||
return !m.tests.length;
|
||||
});
|
||||
if (uncovered.length) return { refused: `removed, but still named by ${shown(uncovered)}, which no test covers` };
|
||||
const namerNote = namerSuite.size ? `files naming it run: ${shown([...namerSuite].sort())}` : '';
|
||||
const renamedTo = ctx.touched.find(t => t.renamedFrom === file)?.path;
|
||||
if (renamedTo && isTestFile(renamedTo)) return { removed: { file, note: [`old name of ${renamedTo}`, namerNote].filter(Boolean).join('; '), lostCoverage: [] }, suite: [...namerSuite] };
|
||||
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${namerNote}`, lostCoverage: [] }, suite: [...namerSuite] };
|
||||
if (!subjects.length) return { removed: { file, note: ['it exercised no file still in the tree', namerNote].filter(Boolean).join('; '), lostCoverage: [] }, suite: [...namerSuite] };
|
||||
const suite = new Set<string>();
|
||||
const lostCoverage: string[] = [];
|
||||
for (const subject of subjects) {
|
||||
@@ -170,7 +155,7 @@ function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet<string
|
||||
const notes = [
|
||||
suite.size ? `owning suite runs: ${shown([...suite].sort())}` : '',
|
||||
lostCoverage.length ? `coverage removed: ${lostCoverage.join(' ')} ${lostCoverage.length === 1 ? 'has' : 'have'} no remaining test` : '',
|
||||
namerSuite.size ? `files naming it run: ${shown([...namerSuite].sort())}` : '',
|
||||
namerNote,
|
||||
].filter(Boolean);
|
||||
return { removed: { file, note: notes.join('; ') || 'its files keep indirect or declared coverage', lostCoverage }, suite: [...new Set([...suite, ...namerSuite])] };
|
||||
}
|
||||
|
||||
@@ -1,17 +1,16 @@
|
||||
/**
|
||||
* Paths a branch removes (deleted files and the old name of a rename) owe no lane
|
||||
* when only a renamed or deleted test, or JS, TS, Python or Dart code nothing
|
||||
* imports or names, goes (issue #3529, gbrain#7829); any other removal owes what
|
||||
* an edit owes. Each waived one
|
||||
* prints a visible `removed (no lane owed): <path> — <why>` line: a deleted test
|
||||
* runs its owning suite (the remaining tests of the files it exercised) or says
|
||||
* which subjects lost their last test; a modified file whose only test was
|
||||
* deleted still fails, and an unreadable deleted test fails closed.
|
||||
* A test a branch removes (deleted, or the old name of a rename) owes no lane
|
||||
* when nothing remaining imports it (issue #3529, gbrain#7829); any other
|
||||
* removal, a renamed source's old name included, owes what an edit owes, as on
|
||||
* main. Each waived test prints a visible `removed (no lane owed): <path> — <why>`
|
||||
* line: it runs the tests of files that still name it, and a deleted test runs
|
||||
* its owning suite (the remaining tests of the files it exercised) or says which
|
||||
* subjects lost their last test; a modified file whose only test was deleted
|
||||
* still fails, and an unreadable deleted test fails closed.
|
||||
*/
|
||||
import { afterEach, describe, expect, test } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { buildGraph, testsFor } from '../lib/pregate/import-graph';
|
||||
import { BASE_PREGATE, cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture';
|
||||
|
||||
const repos: FixtureRepo[] = [];
|
||||
@@ -57,15 +56,13 @@ describe('removed paths in lanes (#3529)', () => {
|
||||
expect(r.check.detail).toContain('1 removed path(s); 1 subject(s) lost their last test');
|
||||
});
|
||||
|
||||
test('deleting a file and its test together notes both and loses no coverage of a file that still exists', () => {
|
||||
test('deleting a file and its test together waives the test; the deleted file still owes a lane', () => {
|
||||
const repo = fixture();
|
||||
commit(repo, { 'test/util.test.js': null, 'lib/util.js': null });
|
||||
const r = lanes(repo);
|
||||
expect(r.check.status).toBe('pass');
|
||||
expect(r.removed).toEqual([
|
||||
'removed (no lane owed): lib/util.js — nothing in the tree references it',
|
||||
'removed (no lane owed): test/util.test.js — it exercised no file still in the tree',
|
||||
]);
|
||||
expect(r.check.code).toBe('PREGATE_NO_LANE');
|
||||
expect(r.check.detail).toContain('1 touched file(s) with no test lane: lib/util.js');
|
||||
expect(r.removed).toEqual(['removed (no lane owed): test/util.test.js — it exercised no file still in the tree']);
|
||||
expect(r.check.detail).not.toContain('lost their last test');
|
||||
});
|
||||
|
||||
@@ -78,23 +75,6 @@ describe('removed paths in lanes (#3529)', () => {
|
||||
expect(r.ran).toContain('test/util2.test.js');
|
||||
});
|
||||
|
||||
test('a renamed source keeps the lane of its new name; its old name is noted', () => {
|
||||
const repo = fixture();
|
||||
move(repo, 'lib/util.js', 'lib/util2.js', ['test/util.test.js', t => t.replace('../lib/util.js', '../lib/util2.js')]);
|
||||
const r = lanes(repo);
|
||||
expect(r.check.status).toBe('pass');
|
||||
expect(r.removed).toEqual(['removed (no lane owed): lib/util.js — old name of lib/util2.js; nothing in the tree references it']);
|
||||
expect(r.ran).toContain('test/util.test.js');
|
||||
});
|
||||
|
||||
test('a deleted file no test covered is noted, not failed', () => {
|
||||
const repo = fixture();
|
||||
commit(repo, { 'lib/orphan.js': null });
|
||||
const r = lanes(repo);
|
||||
expect(r.check.status).toBe('pass');
|
||||
expect(r.removed).toEqual(['removed (no lane owed): lib/orphan.js — nothing in the tree references it']);
|
||||
});
|
||||
|
||||
test('an unstaged deletion behaves like a committed one: the missing test is never selected', () => {
|
||||
const repo = fixture({ baseFiles: { 'test/util-v2.test.js': UTIL_V2 } });
|
||||
fs.rmSync(path.join(repo.work, 'test/util.test.js'));
|
||||
@@ -156,13 +136,6 @@ describe('removed paths in lanes (#3529)', () => {
|
||||
expect(lanes(repo).removed).toEqual(['removed (no lane owed): test/util.test.js — coverage removed: lib/util.js has no remaining test']);
|
||||
});
|
||||
|
||||
test('a deleted Rust module selects its crate\'s tests', () => {
|
||||
const repo = fixture({ baseFiles: { 'crates/demo/Cargo.toml': '[package]\nname = "demo"\nversion = "0.1.0"\n', 'crates/demo/src/lib.rs': 'pub mod gone;\n#[cfg(test)]\nmod tests {\n #[test]\n fn t() {}\n}\n', 'crates/demo/src/gone.rs': 'pub fn f() {}\n' } });
|
||||
commit(repo, { 'crates/demo/src/gone.rs': null });
|
||||
const g = buildGraph(repo.work, { extraFiles: ['crates/demo/src/gone.rs'] });
|
||||
expect(testsFor(g, 'crates/demo/src/gone.rs').tests).toEqual(['crates/demo/src/lib.rs']);
|
||||
});
|
||||
|
||||
describe('negative controls', () => {
|
||||
test('deleting every file of a fixture directory a remaining test reads selects that test', () => {
|
||||
const reader = "const test = require('node:test');\nconst fs = require('node:fs');\ntest('reads the fixture dir', () => { fs.readdirSync('test/fixtures/data'); });\n";
|
||||
@@ -190,38 +163,56 @@ describe('removed paths in lanes (#3529)', () => {
|
||||
expect(r.check.detail).toContain('scripts/setup.js');
|
||||
});
|
||||
|
||||
test.each([
|
||||
['a relative extensionless TS import', { 'src/d.ts': 'export const d = 1;\n', 'src/u.ts': "import { d } from './d';\nexport const u = d + 1;\n" }, 'src/d.ts', 'src/u.ts'],
|
||||
['a Python dotted import', { 'pkg/__init__.py': '', 'pkg/m.py': 'X = 1\n', 'app.py': 'from pkg import m\nprint(m.X)\n' }, 'pkg/m.py', 'app.py'],
|
||||
])('a deleted file untested code still imports by %s fails closed', (_how, files, deleted, importer) => {
|
||||
const repo = fixture({ baseFiles: files });
|
||||
commit(repo, { [deleted]: null });
|
||||
test('a deleted test that remaining code still imports fails closed', () => {
|
||||
const repo = fixture({ baseFiles: { 'lib/uses-test.js': "exports.cases = require('../test/util.test.js');\n" } });
|
||||
commit(repo, { 'test/util.test.js': null });
|
||||
const r = lanes(repo);
|
||||
expect(r.check.code).toBe('PREGATE_NO_LANE');
|
||||
expect(r.check.detail).toContain(deleted);
|
||||
expect(r.removed).toEqual([]);
|
||||
const why = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--explain']);
|
||||
expect(`${why.out}${why.err}`).toContain(`${deleted}: removed, but still imported by ${importer}`);
|
||||
expect(`${why.out}${why.err}`).toContain('test/util.test.js: removed, but still imported by lib/uses-test.js');
|
||||
});
|
||||
|
||||
const GO_MOD = 'module example.com/demo\n\ngo 1.22\n';
|
||||
const NOT_A_TEST = 'removed, but only a removed test is waived; any other removal owes what an edit owes';
|
||||
test.each([
|
||||
['a Go file its package still uses', { 'go.mod': GO_MOD, 'pkg/value.go': 'package pkg\n\nfunc value() int { return 1 }\n', 'pkg/main.go': 'package pkg\n\nfunc Main() int { return value() }\n' }, 'pkg/value.go', 'not JS, TS, Python or Dart'],
|
||||
['a Go test whose package may share its helpers', { 'go.mod': GO_MOD, 'pkg/a.go': 'package pkg\n', 'pkg/a_test.go': 'package pkg\n\nfunc helper() {}\n', 'pkg/b_test.go': 'package pkg\n' }, 'pkg/a_test.go', 'Go or Ruby test may define helpers'],
|
||||
['a Ruby file loaded by require_relative', { 'lib/value.rb': 'VALUE = 1\n', 'lib/main.rb': "require_relative 'value'\n" }, 'lib/value.rb', 'not JS, TS, Python or Dart'],
|
||||
['a shell helper a bin sources', { 'bin/helper.sh': 'greet() { echo hi; }\n', 'bin/tool': '#!/bin/sh\nDIR=$(dirname "$0")\n. "$DIR/helper.sh"\ngreet\n' }, 'bin/helper.sh', 'not JS, TS, Python or Dart'],
|
||||
['a config its runner loads by name', { 'vitest.config.ts': 'export default {};\n' }, 'vitest.config.ts', 'loaded by name'],
|
||||
['a Python startup file', { 'sitecustomize.py': 'import builtins\nbuiltins.FEATURE = True\n' }, 'sitecustomize.py', 'loaded by name'],
|
||||
['a setup file a nested config names relative to itself', { 'packages/app/setup.ts': 'export {};\n', 'packages/app/bunfig.toml': '[test]\npreload = ["./setup.ts"]\n' }, 'packages/app/setup.ts', 'still named by packages/app/bunfig.toml'],
|
||||
])('deleting %s fails closed', (_what, files, deleted, why) => {
|
||||
['a file nothing references', { 'lib/orphan.js': 'exports.o = 1;\n' }, 'lib/orphan.js'],
|
||||
['a file untested code imports without an extension', { 'src/d.ts': 'export const d = 1;\n', 'src/u.ts': "import { d } from './d';\nexport const u = d + 1;\n" }, 'src/d.ts'],
|
||||
['a module untested code imports by its dotted name', { 'pkg/__init__.py': '', 'pkg/m.py': 'X = 1\n', 'app.py': 'from pkg import m\nprint(m.X)\n' }, 'pkg/m.py'],
|
||||
['a module run by its dotted name', { 'tools/__init__.py': '', 'tools/cli.py': 'print(1)\n', 'Makefile': 'run:\n\tpython3 -m tools.cli\n' }, 'tools/cli.py'],
|
||||
['a Go file its package still uses', { 'go.mod': GO_MOD, 'pkg/value.go': 'package pkg\n\nfunc value() int { return 1 }\n', 'pkg/main.go': 'package pkg\n\nfunc Main() int { return value() }\n' }, 'pkg/value.go'],
|
||||
['a Ruby file loaded by require_relative', { 'lib/value.rb': 'VALUE = 1\n', 'lib/main.rb': "require_relative 'value'\n" }, 'lib/value.rb'],
|
||||
['a shell helper a bin sources', { 'bin/helper.sh': 'greet() { echo hi; }\n', 'bin/tool': '#!/bin/sh\nDIR=$(dirname "$0")\n. "$DIR/helper.sh"\ngreet\n' }, 'bin/helper.sh'],
|
||||
['a config its runner loads by name', { 'vitest.config.ts': 'export default {};\n' }, 'vitest.config.ts'],
|
||||
['a Python startup file', { 'sitecustomize.py': 'import builtins\nbuiltins.FEATURE = True\n' }, 'sitecustomize.py'],
|
||||
['a setup file a nested config names relative to itself', { 'packages/app/setup.ts': 'export {};\n', 'packages/app/bunfig.toml': '[test]\npreload = ["./setup.ts"]\n' }, 'packages/app/setup.ts'],
|
||||
])('deleting %s still owes a lane, as on main', (_what, files, deleted) => {
|
||||
const repo = fixture({ baseFiles: files });
|
||||
commit(repo, { [deleted]: null });
|
||||
const r = lanes(repo);
|
||||
expect(r.check.code).toBe('PREGATE_NO_LANE');
|
||||
expect(r.removed).toEqual([]);
|
||||
const explain = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--explain']);
|
||||
expect(explain.out + explain.err).toContain(deleted + ': removed');
|
||||
expect(explain.out + explain.err).toContain(why);
|
||||
expect(explain.out + explain.err).toContain(`${deleted}: ${NOT_A_TEST}`);
|
||||
});
|
||||
|
||||
test('deleting a Go test whose package may share its helpers fails closed', () => {
|
||||
const repo = fixture({ baseFiles: { 'go.mod': GO_MOD, 'pkg/a.go': 'package pkg\n', 'pkg/a_test.go': 'package pkg\n\nfunc helper() {}\n', 'pkg/b_test.go': 'package pkg\n' } });
|
||||
commit(repo, { 'pkg/a_test.go': null });
|
||||
const r = lanes(repo);
|
||||
expect(r.check.code).toBe('PREGATE_NO_LANE');
|
||||
expect(r.removed).toEqual([]);
|
||||
const explain = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--explain']);
|
||||
expect(explain.out + explain.err).toContain('pkg/a_test.go: removed, but a Go or Ruby test may define helpers');
|
||||
});
|
||||
|
||||
test('a renamed source\'s old name still owes a lane, as on main', () => {
|
||||
const repo = fixture();
|
||||
move(repo, 'lib/util.js', 'lib/util2.js', ['test/util.test.js', t => t.replace('../lib/util.js', '../lib/util2.js')]);
|
||||
const r = lanes(repo);
|
||||
expect(r.check.code).toBe('PREGATE_NO_LANE');
|
||||
expect(r.check.detail).toContain('lib/util.js');
|
||||
expect(r.removed).toEqual([]);
|
||||
});
|
||||
|
||||
const CURATION = "exports.SKIP = ['test/util.test.js'];\n";
|
||||
@@ -273,7 +264,7 @@ describe('removed paths in lanes (#3529)', () => {
|
||||
expect(r.check.code).toBe('PREGATE_NO_LANE');
|
||||
expect(r.removed).toEqual([]);
|
||||
const explain = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--explain']);
|
||||
expect(explain.out + explain.err).toContain('lib/plugins/a.js: removed, but still imported by bin/load.js');
|
||||
expect(explain.out + explain.err).toContain(`lib/plugins/a.js: ${NOT_A_TEST}`);
|
||||
});
|
||||
|
||||
test('a deleted file a tested loader reaches by a computed path fails closed', () => {
|
||||
@@ -285,7 +276,7 @@ describe('removed paths in lanes (#3529)', () => {
|
||||
expect(r.check.code).toBe('PREGATE_NO_LANE');
|
||||
expect(r.removed).toEqual([]);
|
||||
const explain = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--explain']);
|
||||
expect(explain.out + explain.err).toContain('lib/value.js: removed, but still named by loader.js');
|
||||
expect(explain.out + explain.err).toContain(`lib/value.js: ${NOT_A_TEST}`);
|
||||
});
|
||||
|
||||
test('a Go test renamed into another directory is refused: its old package may still call its helpers', () => {
|
||||
|
||||
Reference in new issue
Block a user