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
This commit is contained in:
Peter Steinberger
2026-07-24 00:55:52 -07:00
committed by GitHub
parent 5e6a15b1bd
commit f3c4b120e2
11 changed files with 47 additions and 55 deletions
-15
View File
@@ -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.
*/
+7 -13
View File
@@ -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,
});
+2 -2
View File
@@ -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;
+4 -1
View File
@@ -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) {
+1 -1
View File
@@ -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}`,
);
+5
View File
@@ -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;
}
+6 -6
View File
@@ -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"],
+1 -1
View File
@@ -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,
@@ -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,
},
})),
]);
});
+3 -1
View File
@@ -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", () => {
+10
View File
@@ -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(