From f3c4b120e2cc2f223ec09b573c8defa3cd0f084a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 24 Jul 2026 00:55:52 -0700 Subject: [PATCH] fix(tooling): prevent silent test filters and local dependency reconciliation (#113236) * fix(tooling): harden changed checks and test filters * test(tooling): align temp report with helper lanes --- scripts/changed-lanes.mjs | 15 ------------ scripts/check-changed.mjs | 20 ++++++---------- scripts/lib/changed-path-facts.mjs | 4 ++-- scripts/run-vitest.mjs | 5 +++- scripts/test-projects.mjs | 2 +- scripts/test-projects.test-support.mjs | 5 ++++ test/scripts/changed-lanes.test.ts | 12 +++++----- test/scripts/changed-path-facts.test.ts | 2 +- .../report-test-temp-creations.test.ts | 23 +++++++------------ test/scripts/run-vitest.test.ts | 4 +++- test/scripts/test-projects.test.ts | 10 ++++++++ 11 files changed, 47 insertions(+), 55 deletions(-) diff --git a/scripts/changed-lanes.mjs b/scripts/changed-lanes.mjs index 043feec0f67b..f4fcce9c512b 100644 --- a/scripts/changed-lanes.mjs +++ b/scripts/changed-lanes.mjs @@ -1,4 +1,3 @@ -// Classifies changed files into CI lanes and release metadata scopes. import { execFileSync } from "node:child_process"; import { appendFileSync, existsSync, readFileSync } from "node:fs"; import { booleanFlag, parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs"; @@ -85,16 +84,10 @@ export function createEmptyChangedLanes() { }; } -/** @internal Shared repository-script contract. */ export function isChangedLaneTestPath(changedPath) { return getChangedPathFacts(normalizeChangedPath(changedPath)).isChangedLaneTest; } -/** - * @param {string[]} changedPaths - * @param {{ packageJsonChangeKind?: "liveDockerTooling" | "tooling" | null }} [options] - * @returns {ChangedLaneResult} - */ /** * Classifies a list of changed paths into docs, app, extension, core, and tooling lanes. * @internal Shared repository-script contract. @@ -256,10 +249,6 @@ export function detectChangedLanes(changedPaths, options = {}) { }; } -/** - * @param {{ paths: string[]; base: string; head?: string; staged?: boolean; mergeHeadFirstParent?: boolean }} params - * @returns {ChangedLaneResult} - */ /** * Classifies changed paths with optional package.json before/after contents. * @internal Shared repository-script contract. @@ -283,10 +272,6 @@ export function detectChangedLanesForPaths(params) { return detectChangedLanes(params.paths, { packageJsonChangeKind }); } -/** - * @param {{ base: string; head?: string; includeWorktree?: boolean; cwd?: string; mergeHeadFirstParent?: boolean }} params - * @returns {string[]} - */ /** * Lists changed paths from git for a base/head comparison. */ diff --git a/scripts/check-changed.mjs b/scripts/check-changed.mjs index de110a9e3432..d150fc340fd9 100644 --- a/scripts/check-changed.mjs +++ b/scripts/check-changed.mjs @@ -217,8 +217,8 @@ function changedCheckDiffRefsReady({ base, head, cwd = process.cwd() }) { export function buildChangedCheckCrabboxArgs(argv = [], options = {}) { const delegatedArgv = buildDelegatedChangedCheckArgv(argv, options); return [ - "crabbox:run", - "--", + "scripts/crabbox-wrapper.mjs", + "run", "--provider", "blacksmith-testbox", "--blacksmith-org", @@ -253,17 +253,11 @@ function buildDelegatedChangedCheckArgv(argv, options = {}) { return argv; } const stagedPaths = listStagedChangedPaths(options.cwd); - const next = []; - if (args.timed) { - next.push("--timed"); - } + const timedArgs = args.timed ? ["--timed"] : []; if (stagedPaths.length === 0) { - next.push("--no-changes"); - return next; + return [...timedArgs, "--no-changes"]; } - next.push("--base", "HEAD", "--head", "HEAD"); - next.push("--", ...stagedPaths); - return next; + return [...timedArgs, "--base", "HEAD", "--head", "HEAD", "--", ...stagedPaths]; } export function shouldRunShrinkwrapGuard(paths) { @@ -362,9 +356,9 @@ export function createShrinkwrapGuardCommand(paths) { } async function runChangedCheckViaCrabbox(argv = [], env = process.env) { - console.error("[check:changed] delegating to Blacksmith Testbox via `pnpm crabbox:run`."); + console.error("[check:changed] delegating to Blacksmith Testbox via the Node wrapper."); return await runManagedCommand({ - bin: "pnpm", + bin: "node", args: buildChangedCheckCrabboxArgs(argv), env, }); diff --git a/scripts/lib/changed-path-facts.mjs b/scripts/lib/changed-path-facts.mjs index f39ffe1835fc..4b3c2c51775a 100644 --- a/scripts/lib/changed-path-facts.mjs +++ b/scripts/lib/changed-path-facts.mjs @@ -12,9 +12,9 @@ const ROOT_GLOBAL_PATH_RE = /^(?:package\.json$|pnpm-lock\.yaml$|pnpm-workspace\.yaml$|tsdown\.config\.ts$|vitest\.config\.ts$)/u; const LEGACY_ROOT_ASSET_PATH_RE = /^assets\//u; const CHANGED_LANE_TEST_PATH_RE = - /(?:^|\/)(?:test|__tests__)\/|(?:\.|\/)(?:test|spec|e2e|browser\.test)\.[cm]?[jt]sx?$/u; + /(?:^|\/)(?:test|__tests__)\/|(?:\.|\/)(?:test|spec|e2e|browser\.test)\.[cm]?[jt]sx?$|(?:^|\/)[^/]+\.test-(?:helpers|support)\.[cm]?[jt]sx?$/u; const TEST_ONLY_PATH_RE = - /(^test\/|\/test\/|\/tests\/|(?:^|\/)[^/]+\.(?:test|spec|test-utils|test-support|test-harness|e2e-harness)\.[cm]?[jt]sx?$)/u; + /(^test\/|\/test\/|\/tests\/|(?:^|\/)[^/]+\.(?:test|spec|test-utils|test-(?:helpers|support|harness)|e2e-harness)\.[cm]?[jt]sx?$)/u; const NATIVE_ONLY_PATH_RE = /^(?:apps\/android\/|apps\/ios\/|apps\/macos\/|apps\/macos-mlx-tts\/|apps\/shared\/|apps\/swabble\/|Swabble\/|appcast\.xml$)/u; diff --git a/scripts/run-vitest.mjs b/scripts/run-vitest.mjs index 1f9cf0c5df57..2fc831e358f2 100644 --- a/scripts/run-vitest.mjs +++ b/scripts/run-vitest.mjs @@ -540,7 +540,10 @@ function isExplicitProjectRouterTargetArg(arg, cwd = process.cwd(), fsImpl = fs) return true; } const filePath = path.isAbsolute(arg) ? arg : path.resolve(cwd, arg); - return fsImpl.existsSync(filePath) && isDelegableBroadProjectRouterTarget(arg, cwd); + return fsImpl.existsSync(filePath) + ? isDelegableBroadProjectRouterTarget(arg, cwd) + : path.extname(arg) === "" && + /^(?:src|test|extensions|ui|packages|apps)\//u.test(toRepoRelativeArg(arg, cwd)); } function collectExplicitFileTargetArgs(argv, predicate = isExplicitFileTargetArg) { diff --git a/scripts/test-projects.mjs b/scripts/test-projects.mjs index 390e777e4f45..380902239e27 100644 --- a/scripts/test-projects.mjs +++ b/scripts/test-projects.mjs @@ -260,7 +260,7 @@ async function main() { const unmatchedExplicitTargets = findUnmatchedExplicitTestTargets(args, process.cwd()); if (unmatchedExplicitTargets.length > 0) { for (const unmatched of unmatchedExplicitTargets) { - const suffix = unmatched.includePattern ? ` (${unmatched.includePattern})` : ""; + const suffix = unmatched.includePattern ? ` (tried: ${unmatched.includePattern})` : ""; console.error( `[test] explicit test target matched no test files: ${unmatched.target}${suffix}`, ); diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 0cef2963fd5e..a5a07c43ed1c 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -2888,6 +2888,8 @@ function isPathLikeTargetArg(arg, cwd) { isFileLikeTarget(arg) || isVitestConfigPathLikeTarget(relative) || isExistingPathTarget(arg, cwd) || + (path.posix.extname(relative) === "" && + /^(?:src|test|extensions|ui|packages|apps)\//u.test(relative)) || Boolean(resolveExplicitTestPrefixTargets(arg, cwd)?.length) ); } @@ -3146,6 +3148,9 @@ export function findUnmatchedExplicitTestTargets(args, cwd = process.cwd()) { unmatched.push({ target: targetArg, reason: "path-does-not-exist", + ...(path.posix.extname(relative) === "" + ? { includePattern: `${relative}{,.*}.{test,spec}.{js,jsx,ts,tsx,mjs,cjs,mts,cts}` } + : {}), }); continue; } diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index 36334f4d45df..0f595aed3580 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -282,7 +282,7 @@ describe("scripts/changed-lanes", () => { commitAll(dir, "initial"); const binDir = path.join(dir, "bin"); mkdirSync(binDir, { recursive: true }); - writeFileSync(path.join(binDir, "pnpm"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + writeFileSync(path.join(binDir, "node"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); const result = spawnSync(process.execPath, [path.join(repoRoot, "scripts/check-changed.mjs")], { cwd: dir, @@ -312,7 +312,7 @@ describe("scripts/changed-lanes", () => { writeRepoFile(dir, "node_modules/typescript/package.json", '{"name":"typescript"}\n'); const binDir = path.join(dir, "bin"); mkdirSync(binDir, { recursive: true }); - writeFileSync(path.join(binDir, "pnpm"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); + writeFileSync(path.join(binDir, "node"), "#!/bin/sh\nexit 0\n", { mode: 0o755 }); const result = spawnSync( process.execPath, @@ -939,8 +939,8 @@ describe("scripts/changed-lanes", () => { expect(changedCheckRequiresRemote(result)).toBe(true); expect(buildChangedCheckCrabboxArgs(["--base", "origin/main", "--head", "HEAD"])).toEqual([ - "crabbox:run", - "--", + "scripts/crabbox-wrapper.mjs", + "run", "--provider", "blacksmith-testbox", "--blacksmith-org", @@ -1092,7 +1092,7 @@ describe("scripts/changed-lanes", () => { it.each([ { name: "routes core test-only changes to core test lanes only", - path: "packages/normalization-core/src/string-normalization.test.ts", + path: "packages/normalization-core/src/string-normalization.test-support.ts", expected: { lanes: { coreTests: true }, includes: ["tsgo:core:test"], @@ -1110,7 +1110,7 @@ describe("scripts/changed-lanes", () => { }, { name: "routes extension test-only changes to extension test lanes only", - path: "extensions/discord/src/index.test.ts", + path: "extensions/discord/src/index.test-helpers.ts", expected: { lanes: { extensionTests: true }, includes: ["tsgo:extensions:test"], diff --git a/test/scripts/changed-path-facts.test.ts b/test/scripts/changed-path-facts.test.ts index fccfd5bb6356..d8dfd89d14a8 100644 --- a/test/scripts/changed-path-facts.test.ts +++ b/test/scripts/changed-path-facts.test.ts @@ -29,7 +29,7 @@ describe("changed path facts", () => { }); it("preserves test and native-only predicates independently from surfaces", () => { - expect(getChangedPathFacts("extensions/slack/src/index.test.ts")).toMatchObject({ + expect(getChangedPathFacts("extensions/slack/src/index.test-support.ts")).toMatchObject({ surface: "extension", isChangedLaneTest: true, isTestOnly: true, diff --git a/test/scripts/report-test-temp-creations.test.ts b/test/scripts/report-test-temp-creations.test.ts index 21ec1b8b6143..9446510de78a 100644 --- a/test/scripts/report-test-temp-creations.test.ts +++ b/test/scripts/report-test-temp-creations.test.ts @@ -93,24 +93,17 @@ describe("report-test-temp-creations", () => { reason: "new mkdtemp temp directory creation", source: bareTempSource, }, - { - file: "test/helper.test-support.mjs", + ...[ + "test/helper.test-support.mjs", + "test/helpers/temp-fixture.ts", + "packages/foo/__tests__/helper.ts", + "extensions/discord/src/monitor/message-handler.test-helpers.ts", + ].map((file) => ({ + file, line: 2, reason: "new mkdtemp temp directory creation", source: mkdtempSource, - }, - { - file: "test/helpers/temp-fixture.ts", - line: 2, - reason: "new mkdtemp temp directory creation", - source: mkdtempSource, - }, - { - file: "packages/foo/__tests__/helper.ts", - line: 2, - reason: "new mkdtemp temp directory creation", - source: mkdtempSource, - }, + })), ]); }); diff --git a/test/scripts/run-vitest.test.ts b/test/scripts/run-vitest.test.ts index d388f79317b5..5d886b9b85e4 100644 --- a/test/scripts/run-vitest.test.ts +++ b/test/scripts/run-vitest.test.ts @@ -279,7 +279,7 @@ describe("scripts/run-vitest", () => { ]); }); - it("delegates bare explicit directories and globs to the project router", () => { + it("delegates bare explicit directories, globs, and extensionless prefixes", () => { expect(resolveTestProjectsDelegationArgs(["test/scripts"])).toEqual(["test/scripts"]); expect( resolveTestProjectsDelegationArgs(["run", "test/scripts", "--reporter=verbose"]), @@ -290,6 +290,8 @@ describe("scripts/run-vitest", () => { expect(resolveTestProjectsDelegationArgs(["src/agents/**/*.ts"])).toBeNull(); expect(resolveTestProjectsDelegationArgs(["src/**/*.test.ts"])).toBeNull(); expect(resolveTestProjectsDelegationArgs(["./src"])).toBeNull(); + const prefix = "extensions/telegram/src/format"; + expect(resolveTestProjectsDelegationArgs([prefix])).toEqual([prefix]); }); it("delegates mixed filters when an explicit file target is present", () => { diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index c90e846271a5..041fdd6baf88 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -4925,6 +4925,16 @@ describe("scripts/test-projects full-suite sharding", () => { ]); }); + it("rejects unmatched extensionless test prefixes with the attempted pattern", () => { + const target = "extensions/telegram/src/no-such-prefix"; + const [unmatched] = findUnmatchedExplicitTestTargets([target]); + expect(unmatched).toEqual({ + target, + reason: "path-does-not-exist", + includePattern: `${target}{,.*}.{test,spec}.{js,jsx,ts,tsx,mjs,cjs,mts,cts}`, + }); + }); + it("rejects watch mode with multiple explicit leaf project config targets", () => { expect(() => buildVitestRunPlans(