diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d3820202..a04824999 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,14 +5,14 @@ **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. ### Fixed -- **Deleted and renamed tests pass `lanes`**: each removed path that owes no lane prints `removed (no lane owed): — `, 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. +- **Deleted and renamed tests pass `lanes`**: each removed path that owes no lane prints `removed (no lane owed): — `, 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. - **Lost coverage is named, not hidden**: when a deleted test was the last test of a file, the line says `coverage removed: 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. ### Gate integrity -- New `test/pregate-removed-paths.test.ts` (38 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` (41 cases) covers the waived paths and the fail-closed negative controls; no existing test, threshold or allowlist changed. ## [1.91.291.0] - 2026-10-28 diff --git a/docs/pregate.md b/docs/pregate.md index c379236c1..68dfd8cff 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -204,7 +204,11 @@ Two stages with separate clocks, both printed: 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: has no remaining test`, - and `M subject(s) lost their last test` in the detail). A deleted JS, TS, + 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 diff --git a/lib/pregate/lane-selection.ts b/lib/pregate/lane-selection.ts index 83240304d..f5aefde2c 100644 --- a/lib/pregate/lane-selection.ts +++ b/lib/pregate/lane-selection.ts @@ -125,11 +125,13 @@ 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, 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. + * 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. */ function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet, 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); @@ -137,7 +139,18 @@ function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet t.renamedFrom === file)?.path; - if (renamedTo && isTestFile(file) && isTestFile(renamedTo)) return { removed: { file, note: `old name of ${renamedTo}`, lostCoverage: [] }, suite: [] }; + const namerSuite = new Set(); + 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); @@ -146,7 +159,7 @@ function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet(); const lostCoverage: string[] = []; for (const subject of subjects) { @@ -157,8 +170,9 @@ function removedPath(ctx: PregateContext, graph: Graph, gone: ReadonlySet { expect(explain.out + explain.err).toContain(why); }); + const CURATION = "exports.SKIP = ['test/util.test.js'];\n"; + const CURATION_TEST = "const test = require('node:test');\nconst assert = require('node:assert');\nconst fs = require('node:fs');\nconst path = require('node:path');\nconst { SKIP } = require('../scripts/curation.js');\ntest('every entry names a real test', () => assert.deepStrictEqual(SKIP.filter(f => !fs.existsSync(path.join(__dirname, '..', f))), []));\n"; + test.each([ + ['deleted test', (repo: FixtureRepo) => commit(repo, { 'test/util.test.js': null })], + ['test renamed to another test', (repo: FixtureRepo) => move(repo, 'test/util.test.js', 'test/util2.test.js')], + ])('a %s that a tested curation list still names runs that list\'s test, which fails loudly', (_how, change) => { + const repo = fixture({ baseFiles: { 'scripts/curation.js': CURATION, 'test/curation.test.js': CURATION_TEST } }); + change(repo); + const r = lanes(repo); + expect(r.ran).toContain('test/curation.test.js'); + expect(r.check.status).toBe('fail'); + expect(r.removed[0]).toContain('files naming it run: test/curation.test.js'); + }); + + test('a removed test a file no test covers still names fails closed', () => { + const repo = fixture({ baseFiles: { 'scripts/skip-list.txt': '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.removed).toEqual([]); + const explain = runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--explain']); + expect(explain.out + explain.err).toContain('test/util.test.js: removed, but still named by scripts/skip-list.txt, which no test covers'); + }); + test('a rename into a waived path does not waive the old name: package.json renamed to Markdown fails closed', () => { const repo = fixture(); move(repo, 'package.json', 'docs/package.md');