Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions scripts/check-test-revert-oracle.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ if (opts.ci && isDependabotDependencyOnly(process.env.PR_AUTHOR_LOGIN, entries))
process.exit(0);
}

const { production, test: testEntries, ignored, warnings } = classifyDiff(entries);
const { production, test: testEntries, ignored, inert, warnings } = classifyDiff(entries);
for (const w of warnings) console.log(` WARNING: ${w}`);

let prodPaths = production.map((e) => e.path);
Expand All @@ -280,7 +280,9 @@ if (opts.tests.length > 0) {
if (testPaths.length === 0) die(EXIT_NOTHING_CHECKED, '--test matched none of the branch\'s changed test files.');
}

console.log(` files: ${production.length} production, ${testEntries.length} test, ${ignored.length} ignored`);
console.log(
` files: ${production.length} production, ${testEntries.length} test, ${ignored.length} ignored, ${inert.length} inert`,
);

if (prodPaths.length === 0) {
const message = 'this branch changes no production files; there is nothing whose absence a test could notice.';
Expand Down
36 changes: 36 additions & 0 deletions scripts/lib/revert-oracle-inert.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

/**
* File kinds no runner in this repo (vitest, node --test, cargo, pytest) ever
* compiles or executes: an image's bytes, a font's glyphs, an archive's
* contents. Changing one — including deleting it — leaves nothing for a test
* to observe, so it is neither test nor production (#4137).
*
* Observed on two real branches before this existed: #4114 (five deleted
* PNGs) and #4117 (an 87-file archive of JSON/patch/PNG evidence) both
* tripped `check-test-revert-oracle.mjs`'s "changes production code and
* adds/changes NO test file" ABORT — a false positive about the classifier,
* not a finding about either branch, since no test can observe a deleted
* PNG's absence.
*
* Deliberately narrow: this must NOT swallow anything a runner builds or
* runs. `.json` stays production (e.g. `package.json` gates behaviour via
* scripts/deps) — only formats with no runner-observable content at all are
* listed here.
*/
const INERT_SUFFIXES = [
// images
'.png', '.jpg', '.jpeg', '.gif', '.svg', '.ico', '.webp', '.avif', '.bmp', '.tiff', '.tif',
// fonts
'.woff', '.woff2', '.ttf', '.otf', '.eot',
// other binaries with no observable behaviour of their own
'.zip', '.gz', '.tar', '.pdf',
];

/** @param {string} path */
export function isInertPath(path) {
const lower = path.toLowerCase();
return INERT_SUFFIXES.some((s) => lower.endsWith(s));
}
19 changes: 7 additions & 12 deletions scripts/lib/revert-oracle.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,14 @@
*/

import { parsePython, PYTEST_MISSING_PATTERN } from './revert-oracle-python.mjs';
import { isInertPath } from './revert-oracle-inert.mjs';
// ---------------------------------------------------------------------------
// Diff classification
// ---------------------------------------------------------------------------

/** Paths whose change can neither be reverted usefully nor observed by a test. */
const IGNORED_PREFIXES = ['.changeset/', '.github/', 'docs/', '.vscode/'];
const IGNORED_EXACT = new Set([
'pnpm-lock.yaml',
'package-lock.json',
'yarn.lock',
'Cargo.lock',
'CHANGELOG.md',
]);
const IGNORED_EXACT = new Set(['pnpm-lock.yaml', 'package-lock.json', 'yarn.lock', 'Cargo.lock', 'CHANGELOG.md']);
const IGNORED_SUFFIXES = ['.md', '.mdx', '.txt', '.snap.orig'];

/**
Expand Down Expand Up @@ -83,7 +78,7 @@ export function classifyPath(path) {
if (TEST_FILE_RE.test(path) || /(^|\/)(?:[^/]+_tests|tests)\.rs$/.test(path)) return 'test';
if (TEST_DIR_RE.test(path)) return 'test';
if (TEST_SEGMENT_RE.test(path)) return 'test';
return 'production';
return isInertPath(path) ? 'inert' : 'production';
}

/**
Expand All @@ -93,12 +88,12 @@ export function classifyDiff(entries) {
const production = [];
const test = [];
const ignored = [];
const inert = [];
const warnings = [];
for (const { status, path } of entries) {
const kind = classifyPath(path);
if (kind === 'production') production.push({ status, path });
else if (kind === 'test') test.push({ status, path });
else ignored.push({ status, path });
const bucket = kind === 'production' ? production : kind === 'test' ? test : kind === 'inert' ? inert : ignored;
bucket.push({ status, path });
}
if (production.some((e) => isRustFile(e.path))) {
warnings.push(
Expand All @@ -107,7 +102,7 @@ export function classifyDiff(entries) {
'as the code — expect INCONCLUSIVE and use --mutation for a surgical revert.',
);
}
return { production, test, ignored, warnings };
return { production, test, ignored, inert, warnings };
}

/** Parse `git diff --name-status -z`-free plain output. Renames carry two paths. */
Expand Down
63 changes: 63 additions & 0 deletions scripts/lib/revert-oracle.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,69 @@ test('classifyPath: the deploy-config rule does not swallow neighbouring code',
assert.equal(classifyPath('vercel.json.ts'), 'production');
});

// WHY "inert" IS ITS OWN KIND, NOT `ignored` (#4137).
//
// A changed file that no runner in this repo claims as a test AND no runner
// compiles or executes as source has no observable behaviour at all: an
// image's bytes, a font's glyphs, an archive's contents. Classifying such a
// file as `production` made the oracle ABORT with "changes production code
// and adds/changes NO test file" on branches like #4114 (five deleted PNGs)
// and #4117 (an 87-file archive) — there is no test that could possibly
// accompany a deleted PNG, so the ABORT was a false positive about the
// classifier, not a finding about the branch.
test('classifyPath: images, fonts, and other binaries are inert, not production', () => {
assert.equal(classifyPath('apps/viewer/public/favicon-192x192.png'), 'inert');
assert.equal(classifyPath('apps/viewer/public/favicon.ico'), 'inert');
assert.equal(classifyPath('apps/viewer/src/assets/logo.svg'), 'inert');
assert.equal(classifyPath('apps/landing/public/hero.webp'), 'inert');
assert.equal(classifyPath('apps/viewer/public/fonts/inter.woff2'), 'inert');
assert.equal(classifyPath('apps/viewer/public/fonts/inter.ttf'), 'inert');
assert.equal(classifyPath('scripts/perf/evidence/report.pdf'), 'inert');
assert.equal(classifyPath('scripts/perf/evidence/archive.zip'), 'inert');
});

test('classifyPath: inertness is about the file kind, not the operation — a deleted image is still inert', () => {
// classifyPath is purely path-based; classifyDiff carries the `status` field
// through unchanged. A pure deletion of an inert file must not become
// production just because the "operation" is a delete (must-not-regress in
// #4137: inertness is about the file kind, not about the operation).
const { production, inert } = classifyDiff(parseNameStatus('D\tapps/viewer/public/favicon.png'));
assert.deepEqual(production, []);
assert.deepEqual(inert.map((e) => e.path), ['apps/viewer/public/favicon.png']);
});

test('classifyPath: inertness must NOT cover anything a runner compiles or executes', () => {
// Extensions a runner in this repo actually builds/executes stay production
// even though they sound "asset-like" or are commonly bundled alongside
// assets — this is the narrowing #4137 must preserve.
assert.equal(classifyPath('packages/core/package.json'), 'production');
assert.equal(classifyPath('crates/ifc-lite-geom/src/walk.rs'), 'production');
assert.equal(classifyPath('packages/renderer/src/device.ts'), 'production');
assert.equal(classifyPath('tools/ifcopenshell_reference/canonical.py'), 'production');
});

test('classifyDiff: a mixed diff keeps inert files out of BOTH production and test, without giving the real change a free pass', () => {
const entries = parseNameStatus(
[
'M\tpackages/renderer/src/device.ts',
'A\tapps/viewer/public/favicon-512x512.png',
].join('\n'),
);
const { production, test: tests, inert } = classifyDiff(entries);
assert.deepEqual(production.map((e) => e.path), ['packages/renderer/src/device.ts']);
assert.deepEqual(tests, []);
assert.deepEqual(inert.map((e) => e.path), ['apps/viewer/public/favicon-512x512.png']);
});

test('classifyDiff: a diff of ONLY inert files has no production entries (script-level effect: NOT APPLICABLE, same as #4024 for test-only diffs)', () => {
const entries = parseNameStatus(
['D\tapps/viewer/public/favicon-192x192.png', 'D\tapps/viewer/public/favicon-512x512.png'].join('\n'),
);
const { production, inert } = classifyDiff(entries);
assert.deepEqual(production, []);
assert.equal(inert.length, 2);
});

test('classifyDiff splits a real branch shape and never puts a test in production', () => {
const entries = parseNameStatus(
[
Expand Down
Loading