fix(cli): stop review test-efficacy tests depending on ambient tmpdir vitest (#8537)

Two tests failed on hosts where vitest resolves up-tree from os.tmpdir()
(observed on self-hosted CI where a node_modules above TMPDIR provides
one): findVitestBin's "cannot be resolved" case never threw, and
runControlMutant's "cannot run" case executed the probe for real instead
of throwing.

Make the failure conditions host-deterministic while keeping every
assertion: findVitestBin accepts an injected resolver (default
unchanged) so the MODULE_NOT_FOUND case is forced directly, and the
runControlMutant test plants a shadow vitest whose exports hide
package.json, which wins resolution from any ancestor install and makes
the run fail deterministically.
This commit is contained in:
Shaojin Wen 2026-08-04 22:31:02 +08:00 committed by GitHub
parent 6e9ecc41e4
commit eacc85e846
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 38 additions and 7 deletions

View file

@ -183,8 +183,20 @@ describe('planTestEfficacy', () => {
describe('findVitestBin', () => {
it('names the search root when vitest cannot be resolved', () => {
const worktree = mkdtempSync(join(tmpdir(), 'no-vitest-'));
// A bare tmpdir answers "not found" only when nothing up-tree happens to
// provide vitest — a node_modules above the runner's TMPDIR (observed on
// self-hosted CI) would resolve one and the throw never fires. Inject the
// MODULE_NOT_FOUND itself so the test asks the same question on every
// host instead of depending on the ambient filesystem.
const vitestNotInstalled = () => {
const err = new Error(
"Cannot find module 'vitest/package.json'",
) as NodeJS.ErrnoException;
err.code = 'MODULE_NOT_FOUND';
throw err;
};
expect(() => findVitestBin(worktree)).toThrow(
expect(() => findVitestBin(worktree, vitestNotInstalled)).toThrow(
`vitest not found searching up from ${worktree}`,
);
});
@ -314,11 +326,26 @@ describe('runControlMutant', () => {
try {
const original = 'import { it } from "vitest";\nit("t", () => {});\n';
writeFileSync(join(dir, 'a.test.ts'), original);
// No vitest resolvable from this tmpdir, so the run THROWS out of the
// suite helper — which is the point: the restore lives in a `finally`
// and has to survive that path, or the control leaves an injected
// The control throws only when the vitest run never starts. A bare
// tmpdir has no vitest only when nothing up-tree provides one — a
// node_modules above the runner's TMPDIR (observed on self-hosted CI)
// would resolve vitest and the run would actually execute. Plant a
// shadow vitest whose `exports` hides its package.json: the innermost
// node_modules wins resolution on every host, so findVitestBin surfaces
// the ERR_PACKAGE_PATH_NOT_EXPORTED and the restore's `finally` has to
// survive exactly that path — or the control leaves an injected
// always-failing test behind in a file every later mutant run uses.
// (The caller's outer catch is what turns the throw into inconclusive.)
const vitestDir = join(dir, 'node_modules', 'vitest');
mkdirSync(vitestDir, { recursive: true });
writeFileSync(
join(vitestDir, 'package.json'),
JSON.stringify({
name: 'vitest',
exports: { '.': './index.js' },
}),
);
writeFileSync(join(vitestDir, 'index.js'), '');
expect(() => runControlMutant(dir, 'a.test.ts')).toThrow();
expect(readFileSync(join(dir, 'a.test.ts'), 'utf8')).toBe(original);
} finally {

View file

@ -1258,11 +1258,15 @@ function gitOut(cwd: string, ...args: string[]): string {
* same up-tree walk Node uses for the probe's own imports, so it also survives
* non-hoisted layouts.
*/
export function findVitestBin(worktree: string): string {
const req = createRequire(join(worktree, 'noop.js'));
export function findVitestBin(
worktree: string,
resolveModule: (specifier: string) => string = createRequire(
join(worktree, 'noop.js'),
).resolve,
): string {
let pkgPath: string;
try {
pkgPath = req.resolve('vitest/package.json');
pkgPath = resolveModule('vitest/package.json');
} catch (error) {
// Only a genuine MODULE_NOT_FOUND is "vitest not found". A present vitest
// whose `exports` no longer exposes `./package.json` throws