From 35695cb3bc90ecded67c97017bc2c3491c23f810 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 16 Aug 2026 23:24:11 -0700 Subject: [PATCH] fix(test): intersect scoped CLI filters with each lane's own scope (#125004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit narrowIncludePatterns returned the caller's CLI pattern whenever it overlapped a lane's include list, instead of intersecting the two. A directory argument therefore replaced the lane's curated scope with a broad glob: pnpm test src/plugins -> unit-fast include became ["src/plugins/**/*.test.*"] instead of its 60 curated files -> contracts-plugin include became ["src/plugins/**/*.test.*"] instead of ["src/plugins/contracts/**/*.test.ts"] Both lanes run isolate: false, so every re-admitted file shared a worker with unrelated files. unit-fast re-admitted exactly the files it excludes for being stateful (module mocking, dynamic import, filesystem state), and contracts-plugin pulled in every sibling test outside contracts/. That produced nondeterministic cross-file pollution: failures that moved between files and lanes run to run, reproduced in neither isolation nor CI. Keep the lane's own pattern when it is rooted deeper than the CLI selection, otherwise keep the CLI pattern. Equal-depth selections are unchanged, so the existing scoped-config expectations still hold. Coverage is unchanged; only duplicate execution is removed. `pnpm test src/plugins` went from 1171 file-executions (3.4x duplication over 346 files) to 344 — the two remaining files are .e2e.test.ts, excluded by the shared config by design. --- test/vitest-unit-fast-config.test.ts | 40 ++++++++++++++++++++++++++++ test/vitest/vitest.pattern-file.ts | 24 ++++++++++++----- 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/test/vitest-unit-fast-config.test.ts b/test/vitest-unit-fast-config.test.ts index d7bbb24092a7..120a48509862 100644 --- a/test/vitest-unit-fast-config.test.ts +++ b/test/vitest-unit-fast-config.test.ts @@ -4,6 +4,8 @@ import { beforeAll, describe, expect, it } from "vitest"; import { spawnNodeEvalSync } from "../src/test-utils/node-process.js"; import { cliProcessTestFiles } from "./vitest/vitest.cli-process-paths.mjs"; import { createCommandsLightVitestConfig } from "./vitest/vitest.commands-light.config.ts"; +import { createContractsPluginVitestConfig } from "./vitest/vitest.contracts-plugin.config.ts"; +import { pluginContractPatterns } from "./vitest/vitest.contracts-shared.ts"; import { createPluginSdkLightVitestConfig } from "./vitest/vitest.plugin-sdk-light.config.ts"; import { createUnitFastFakeTimersVitestConfig } from "./vitest/vitest.unit-fast-fake-timers.config.ts"; import { createUnitFastIsolatedVitestConfig } from "./vitest/vitest.unit-fast-isolated.config.ts"; @@ -196,6 +198,44 @@ describe("unit-fast vitest lane", () => { expect(testConfig.include).toContain("src/commands/status-overview-values.test.ts"); }); + it("keeps excluded stateful files out of directory-scoped CLI runs", () => { + // A directory argument must narrow the curated inventory, never replace it with the + // directory glob. The lane is non-isolated, so re-admitting an excluded stateful file + // pollutes whichever unrelated files share its worker. + const otherLaneFiles = new Set([ + ...getUnitFastTimerTestFiles(), + ...getUnitFastIsolatedTestFiles(), + ]); + for (const dir of ["src/plugins", "src/agents", "src/commands"]) { + const testConfig = requireTestConfig( + createUnitFastVitestConfig({}, { argv: ["node", "vitest", "run", dir] }), + ); + const include = testConfig.include as string[]; + const expected = unitFastTestFiles.filter( + (file) => file.startsWith(`${dir}/`) && !otherLaneFiles.has(file), + ); + + expect(include, dir).toEqual(expected); + expect( + include.filter((entry) => !isUnitFastTestFile(entry)), + `${dir} admitted non-unit-fast entries`, + ).toEqual([]); + } + + const pluginsInclude = requireTestConfig( + createUnitFastVitestConfig({}, { argv: ["node", "vitest", "run", "src/plugins"] }), + ).include as string[]; + expect(isUnitFastTestFile("src/plugins/install-persistence.test.ts")).toBe(false); + expect(pluginsInclude).not.toContain("src/plugins/install-persistence.test.ts"); + + // Glob-scoped lanes keep their own scope too: a parent-directory argument must not widen + // contracts-plugin from `contracts/` to every sibling test under `src/plugins`. + const contractsInclude = requireTestConfig( + createContractsPluginVitestConfig({}, ["node", "vitest", "run", "src/plugins"]), + ).include as string[]; + expect(contractsInclude).toEqual(pluginContractPatterns); + }); + it("keeps obvious stateful files out of the unit-fast lane", () => { expect(isUnitFastTestFile("src/plugin-sdk/temp-path.test.ts")).toBe(false); expect(isUnitFastTestFile("src/agents/openai-transport-stream.base.test.ts")).toBe(false); diff --git a/test/vitest/vitest.pattern-file.ts b/test/vitest/vitest.pattern-file.ts index dd6016b7e381..3b45f99f6109 100644 --- a/test/vitest/vitest.pattern-file.ts +++ b/test/vitest/vitest.pattern-file.ts @@ -132,13 +132,23 @@ function narrowIncludePatterns( return null; } - return [ - ...new Set( - candidatePatterns.filter((value) => - includePatterns.some((pattern) => patternsCouldOverlap(value, pattern)), - ), - ), - ]; + // Narrowing must intersect the CLI selection with the lane's own scope. When the lane is + // rooted deeper than the CLI selection, keep the lane: returning the caller's broader + // directory pattern re-admits files the lane never owns — `unit-fast` picks up the stateful + // files it excludes, and `contracts-*` picks up every sibling test outside `contracts/`. + // Both run `isolate: false`, so those extra files share a worker and pollute unrelated ones. + const narrowed = new Set(); + for (const candidate of candidatePatterns) { + const candidatePrefix = literalPrefixForGlobPattern(candidate); + for (const laneScope of includePatterns) { + if (!patternsCouldOverlap(candidate, laneScope)) { + continue; + } + const laneScopePrefix = literalPrefixForGlobPattern(laneScope); + narrowed.add(laneScopePrefix.length > candidatePrefix.length ? laneScope : candidate); + } + } + return [...narrowed]; } function isPlainRepoRelativePath(value: string): boolean {