Address PR review feedback (#1780)
- P2: filter directly-changed test files to paths that still exist on disk (existingChangedTestFilesInPackage) so deleted/renamed .test paths from `git diff` never reach `vitest run` positionally; all-deletions diff falls into the delegate-to-gate path. - P1: make heavy-package delegation gate-coverage-aware (GATE_COVERED_MEMORY_ENVELOPE_PACKAGES). Engine delegation keeps the accurate "curated engine-core subset ran above" note; dashboard delegation now warns that the gate runs no dashboard tests and names the CI full-suite backstop, so the coverage gap is loud instead of a silent false-green. - +4 regression tests (115/115). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -45,6 +45,8 @@ import {
|
||||
partitionScopedAffectedPackages,
|
||||
isTestFilePath,
|
||||
changedSourceFilesAffectingPackage,
|
||||
existingChangedTestFilesInPackage,
|
||||
GATE_COVERED_MEMORY_ENVELOPE_PACKAGES,
|
||||
} from "../test-changed.mjs";
|
||||
|
||||
import { deriveBudgetMs } from "../lib/run-vitest-watchdog.mjs";
|
||||
@@ -1867,3 +1869,68 @@ test("changedSourceFilesAffectingPackage: out-of-graph and irrelevant paths stay
|
||||
[],
|
||||
);
|
||||
});
|
||||
|
||||
// FNXC:TestInfrastructure 2026-06-26-09:15: `git diff --name-only` lists deleted /
|
||||
// renamed-away `.test` paths; those must NOT reach the positional `vitest run <file>`
|
||||
// call (they would fail the bounded lane on a missing file). Regression for the P2
|
||||
// deletion case: filter the directly-changed test files to ones still on disk, and
|
||||
// an all-deletions diff must yield [] so the caller delegates to the gate.
|
||||
test("existingChangedTestFilesInPackage: keeps live in-package test files, drops deleted ones", () => {
|
||||
const tmp = mkdtempSync(path.join(tmpdir(), "fusion-changed-tests-"));
|
||||
try {
|
||||
const liveRel = "packages/engine/src/__tests__/live.test.ts";
|
||||
const deletedRel = "packages/engine/src/__tests__/deleted.test.ts";
|
||||
mkdirSync(path.join(tmp, "packages/engine/src/__tests__"), { recursive: true });
|
||||
writeFileSync(path.join(tmp, liveRel), "// live\n");
|
||||
// deletedRel intentionally NOT written to disk (simulates a removed test)
|
||||
assert.deepEqual(
|
||||
existingChangedTestFilesInPackage([deletedRel, liveRel], "packages/engine", { projectRoot: tmp }),
|
||||
[liveRel],
|
||||
);
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("existingChangedTestFilesInPackage: all-deletions diff yields empty (delegate-to-gate path)", () => {
|
||||
const tmp = mkdtempSync(path.join(tmpdir(), "fusion-changed-tests-"));
|
||||
try {
|
||||
// No test files written: every changed test path was a deletion.
|
||||
assert.deepEqual(
|
||||
existingChangedTestFilesInPackage(
|
||||
["packages/engine/src/__tests__/gone-a.test.ts", "packages/engine/src/__tests__/gone-b.test.ts"],
|
||||
"packages/engine",
|
||||
{ projectRoot: tmp },
|
||||
),
|
||||
[],
|
||||
);
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("existingChangedTestFilesInPackage: excludes non-test and out-of-package paths", () => {
|
||||
const tmp = mkdtempSync(path.join(tmpdir(), "fusion-changed-tests-"));
|
||||
try {
|
||||
const inPkgSource = "packages/engine/src/self-healing.ts";
|
||||
const otherPkgTest = "packages/dashboard/src/__tests__/x.test.ts";
|
||||
mkdirSync(path.join(tmp, "packages/engine/src"), { recursive: true });
|
||||
mkdirSync(path.join(tmp, "packages/dashboard/src/__tests__"), { recursive: true });
|
||||
writeFileSync(path.join(tmp, inPkgSource), "// src\n");
|
||||
writeFileSync(path.join(tmp, otherPkgTest), "// other\n");
|
||||
assert.deepEqual(
|
||||
existingChangedTestFilesInPackage([inPkgSource, otherPkgTest], "packages/engine", { projectRoot: tmp }),
|
||||
[],
|
||||
);
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
// FNXC:TestInfrastructure 2026-06-26-09:15: the merge gate re-covers a delegated
|
||||
// engine lane (curated engine-core subset) but runs NO dashboard tests. Lock that
|
||||
// asymmetry so the delegation messaging never overclaims dashboard gate coverage.
|
||||
test("GATE_COVERED_MEMORY_ENVELOPE_PACKAGES: engine covered, dashboard not", () => {
|
||||
assert.equal(GATE_COVERED_MEMORY_ENVELOPE_PACKAGES.has(ENGINE_SCOPED_AFFECTED_PACKAGE), true);
|
||||
assert.equal(GATE_COVERED_MEMORY_ENVELOPE_PACKAGES.has(DASHBOARD_SCOPED_AFFECTED_PACKAGE), false);
|
||||
});
|
||||
|
||||
@@ -1337,6 +1337,19 @@ export const SCOPED_AFFECTED_MEMORY_ENVELOPES = Object.freeze({
|
||||
}),
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TestInfrastructure 2026-06-26-09:15:
|
||||
Which heavy memory-envelope packages the merge gate (`pnpm test:gate`) genuinely
|
||||
re-covers when the wide-fan-out guard delegates their cross-cutting coverage.
|
||||
The gate runs `@fusion/engine test:core` (a curated engine-core allow-list) plus
|
||||
the CI-shape test — it runs NO `@fusion/dashboard` tests. So a delegated engine
|
||||
lane still gets a real (curated subset) safety net, but a delegated dashboard
|
||||
lane gets ZERO gate coverage and would be a silent false-green. Treat dashboard
|
||||
delegation as a loud "not covered by the gate; CI full-suite.yml is the backstop"
|
||||
warning instead of a reassuring "delegated to the gate" message.
|
||||
*/
|
||||
export const GATE_COVERED_MEMORY_ENVELOPE_PACKAGES = Object.freeze(new Set([ENGINE_SCOPED_AFFECTED_PACKAGE]));
|
||||
|
||||
export function prependNodeOption(currentOptions, option) {
|
||||
return [option, currentOptions || ""].join(" ").trim();
|
||||
}
|
||||
@@ -1399,6 +1412,35 @@ export function isTestFilePath(file) {
|
||||
return /\.(test|spec)\.[cm]?[jt]sx?$/.test(file);
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TestInfrastructure 2026-06-26-09:15:
|
||||
`changedFiles` comes from `git diff --name-only`, which lists DELETED and
|
||||
renamed-away `.test`/`.spec` paths alongside live ones. Those paths no longer
|
||||
exist on disk, but the wide-fan-out guard passes its picks positionally to
|
||||
`vitest run <file>` (via `path.relative`). A removed test path reaching Vitest
|
||||
makes the bounded changed lane fail on a file that is gone instead of treating
|
||||
the deletion as the no-test-left case. Filter the directly-changed test files
|
||||
to paths that still EXIST on disk; if every changed test in the package was a
|
||||
deletion the result is empty, and the caller must then take the same
|
||||
delegate-to-the-gate path as the "no changed test files" case rather than
|
||||
handing Vitest an empty/garbage positional set.
|
||||
*/
|
||||
/**
|
||||
* Directly-changed, still-on-disk test files inside a package directory.
|
||||
* @param {string[]|null|undefined} changedFiles repo-relative diff paths
|
||||
* @param {string} pkgDir repo-relative package dir (e.g. "packages/engine")
|
||||
* @param {{ projectRoot?: string }} [opts]
|
||||
* @returns {string[]}
|
||||
*/
|
||||
export function existingChangedTestFilesInPackage(changedFiles, pkgDir, { projectRoot = rootDir } = {}) {
|
||||
return (changedFiles ?? []).filter(
|
||||
(file) =>
|
||||
isTestFilePath(file) &&
|
||||
(file === pkgDir || file.startsWith(`${pkgDir}/`)) &&
|
||||
existsSync(path.join(projectRoot, file)),
|
||||
);
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:TestInfrastructure 2026-06-25-14:30:
|
||||
Why this guard exists (root cause of "pnpm test takes >15min and gets killed"):
|
||||
@@ -1692,21 +1734,38 @@ export async function main(argv = process.argv.slice(2)) {
|
||||
});
|
||||
if (wideSource.length > 0) {
|
||||
const pkgDir = packageDirByName.get(pkg) ?? `packages/${pkg.replace(/^@[^/]+\//, "")}`;
|
||||
explicitChangedTestFiles = (changedFiles ?? []).filter(
|
||||
(file) => isTestFilePath(file) && (file === pkgDir || file.startsWith(`${pkgDir}/`)),
|
||||
);
|
||||
// Filter to test files that still EXIST on disk: `git diff --name-only`
|
||||
// includes deleted/renamed-away `.test` paths, and a removed path passed
|
||||
// positionally to `vitest run` (line ~1720) would fail the bounded lane
|
||||
// on a file that no longer exists. An all-deletions diff yields an empty
|
||||
// list, which falls into the same delegate-to-gate `continue` below as
|
||||
// the no-changed-tests case (FNXC:TestInfrastructure 2026-06-26-09:15).
|
||||
explicitChangedTestFiles = existingChangedTestFilesInPackage(changedFiles, pkgDir);
|
||||
notFullyTestedPackages.add(pkg);
|
||||
// FNXC:TestInfrastructure 2026-06-26-09:15: the gate re-covers a delegated
|
||||
// engine lane (curated engine-core subset) but runs NO dashboard tests, so
|
||||
// a delegated dashboard lane is uncovered. Don't claim "delegated to the
|
||||
// gate" for packages the gate doesn't run — warn loudly and name the real
|
||||
// backstop (CI full-suite.yml / `pnpm test:full`) so the gap is visible.
|
||||
const gateCovered = GATE_COVERED_MEMORY_ENVELOPE_PACKAGES.has(pkg);
|
||||
const wideSourceDesc = `${wideSource[0]}${wideSource.length > 1 ? `, +${wideSource.length - 1} more` : ""}`;
|
||||
const delegationNote = gateCovered
|
||||
? "delegating wider `vitest --changed` coverage to the merge-gate suite (curated engine-core subset ran above)."
|
||||
: `the merge gate does NOT run ${pkg} tests, so this wider coverage is NOT re-run here; ` +
|
||||
"CI full-suite.yml (non-blocking, on push to main) is the backstop. Run `pnpm test:full` for the full sweep.";
|
||||
if (explicitChangedTestFiles.length === 0) {
|
||||
console.log(
|
||||
`[test-changed] ${pkg}: a changed non-test source file (${wideSource[0]}${wideSource.length > 1 ? `, +${wideSource.length - 1} more` : ""}) ` +
|
||||
const log = gateCovered ? console.log : console.warn;
|
||||
log(
|
||||
`[test-changed] ${pkg}: a changed non-test source file (${wideSourceDesc}) ` +
|
||||
"would fan `vitest --changed` out to ~the full suite at this heavy 1-worker lane; " +
|
||||
"delegating cross-cutting coverage to the merge-gate suite (ran above). Run `pnpm test:full` for the full sweep.",
|
||||
`no directly-changed ${pkg} test file to run, so ${delegationNote}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
console.log(
|
||||
`[test-changed] ${pkg}: changed non-test source detected; running ONLY the ${explicitChangedTestFiles.length} directly-changed test file(s) ` +
|
||||
"and delegating wider `vitest --changed` coverage to the merge-gate suite (ran above).",
|
||||
const log = gateCovered ? console.log : console.warn;
|
||||
log(
|
||||
`[test-changed] ${pkg}: changed non-test source detected; running ONLY the ${explicitChangedTestFiles.length} directly-changed test file(s); ` +
|
||||
delegationNote,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user