test: simplify changed test routing

2026-05-06 07:50:43 +00:00 · 2026-04-26 23:58:09 +01:00
parent 199d5f765f
commit 89ab39ca64
12 changed files with 340 additions and 279 deletions
--- a/scripts/changed-lanes.mjs
+++ b/scripts/changed-lanes.mjs
@@ -67,7 +67,7 @@ export function createEmptyChangedLanes() {

 /**
 * @param {string[]} changedPaths
- * @param {{ packageJsonChangeKind?: "liveDockerTooling" | null }} [options]
+ * @param {{ packageJsonChangeKind?: "liveDockerTooling" | "tooling" | null }} [options]
 * @returns {ChangedLaneResult}
 */
 export function detectChangedLanes(changedPaths, options = {}) {
@@ -80,6 +80,8 @@ export function detectChangedLanes(changedPaths, options = {}) {
  let hasNonDocs = false;
  const packageJsonIsLiveDockerTooling =
    paths.includes("package.json") && options.packageJsonChangeKind === "liveDockerTooling";
+  const packageJsonIsTooling =
+    paths.includes("package.json") && options.packageJsonChangeKind === "tooling";

  if (paths.length === 0) {
    reasons.push("no changed paths");
@@ -88,6 +90,7 @@ export function detectChangedLanes(changedPaths, options = {}) {

  if (
    !packageJsonIsLiveDockerTooling &&
+    !packageJsonIsTooling &&
    paths.some((changedPath) => RELEASE_METADATA_PATHS.has(changedPath)) &&
    paths.every(
      (changedPath) => RELEASE_METADATA_PATHS.has(changedPath) || DOCS_PATH_RE.test(changedPath),
@@ -115,6 +118,12 @@ export function detectChangedLanes(changedPaths, options = {}) {
      continue;
    }

+    if (changedPath === "package.json" && packageJsonIsTooling) {
+      lanes.tooling = true;
+      reasons.push(`${changedPath}: package scripts`);
+      continue;
+    }
+
    if (LIVE_DOCKER_TOOLING_PATH_RE.test(changedPath)) {
      lanes.liveDockerTooling = true;
      reasons.push(`${changedPath}: live Docker tooling surface`);
@@ -195,39 +204,42 @@ export function detectChangedLanes(changedPaths, options = {}) {
 }

 /**
- * @param {{ base: string; head?: string; includeWorktree?: boolean }} params
+ * @param {{ base: string; head?: string; includeWorktree?: boolean; cwd?: string }} params
 * @returns {string[]}
 */
 export function listChangedPathsFromGit(params) {
  const base = params.base;
  const head = params.head ?? "HEAD";
+  const cwd = params.cwd ?? process.cwd();
  if (!base) {
    return [];
  }
-  const rangePaths = runGitNameOnlyDiff([`${base}...${head}`]);
+  const rangePaths = runGitNameOnlyDiff([`${base}...${head}`], cwd);
  if (params.includeWorktree === false) {
    return rangePaths;
  }
  return [
    ...new Set([
      ...rangePaths,
-      ...runGitNameOnlyDiff(["--cached", "--diff-filter=ACMR"]),
-      ...runGitNameOnlyDiff(["--diff-filter=ACMR"]),
-      ...runGitLsFiles(["--others", "--exclude-standard"]),
+      ...runGitNameOnlyDiff(["--cached", "--diff-filter=ACMR"], cwd),
+      ...runGitNameOnlyDiff(["--diff-filter=ACMR"], cwd),
+      ...runGitLsFiles(["--others", "--exclude-standard"], cwd),
    ]),
  ].toSorted((left, right) => left.localeCompare(right));
 }

-function runGitNameOnlyDiff(extraArgs) {
+function runGitNameOnlyDiff(extraArgs, cwd = process.cwd()) {
  const output = execFileSync("git", ["diff", "--name-only", ...extraArgs], {
+    cwd,
    stdio: ["ignore", "pipe", "pipe"],
    encoding: "utf8",
  });
  return output.split("\n").map(normalizeChangedPath).filter(Boolean);
 }

-function runGitLsFiles(extraArgs) {
+function runGitLsFiles(extraArgs, cwd = process.cwd()) {
  const output = execFileSync("git", ["ls-files", ...extraArgs], {
+    cwd,
    stdio: ["ignore", "pipe", "pipe"],
    encoding: "utf8",
  });
@@ -245,7 +257,10 @@ export function listStagedChangedPaths() {
 export function classifyPackageJsonChangeFromGit(params) {
  try {
    const { before, after } = readPackageJsonBeforeAfter(params);
-    return isLiveDockerPackageScriptOnlyChange(before, after) ? "liveDockerTooling" : null;
+    if (isLiveDockerPackageScriptOnlyChange(before, after)) {
+      return "liveDockerTooling";
+    }
+    return isPackageScriptOnlyChange(before, after) ? "tooling" : null;
  } catch {
    return null;
  }
@@ -265,6 +280,20 @@ export function isLiveDockerPackageScriptOnlyChange(before, after) {
  );
 }

+export function isPackageScriptOnlyChange(before, after) {
+  const beforePackage = JSON.parse(before);
+  const afterPackage = JSON.parse(after);
+  const beforeScripts = extractPackageScripts(beforePackage);
+  const afterScripts = extractPackageScripts(afterPackage);
+  const beforeStripped = stripPackageScripts(beforePackage);
+  const afterStripped = stripPackageScripts(afterPackage);
+
+  return (
+    stableJson(beforeStripped) === stableJson(afterStripped) &&
+    stableJson(beforeScripts) !== stableJson(afterScripts)
+  );
+}
+
 function readPackageJsonBeforeAfter(params) {
  const before = readGitText(params.staged ? "HEAD" : params.base, "package.json");
  if (params.staged) {
@@ -317,6 +346,17 @@ function stripLiveDockerPackageScripts(packageJson) {
  return clone;
 }

+function extractPackageScripts(packageJson) {
+  const scripts = packageJson?.scripts;
+  return scripts && typeof scripts === "object" && !Array.isArray(scripts) ? scripts : {};
+}
+
+function stripPackageScripts(packageJson) {
+  const clone = JSON.parse(JSON.stringify(packageJson));
+  delete clone.scripts;
+  return clone;
+}
+
 function stableJson(value) {
  if (Array.isArray(value)) {
    return `[${value.map(stableJson).join(",")}]`;
--- a/scripts/check-changed.mjs
+++ b/scripts/check-changed.mjs
@@ -14,12 +14,7 @@ import {
 } from "./lib/local-heavy-check-runtime.mjs";
 import { runManagedCommand } from "./lib/managed-child-process.mjs";
 import { createSparseTsgoSkipEnv } from "./lib/tsgo-sparse-guard.mjs";
-import { isCiLikeEnv } from "./lib/vitest-local-scheduling.mjs";
-import { resolveChangedTestTargetPlan } from "./test-projects.test-support.mjs";

-export const CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS = "600000";
-const VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY = "OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS";
-const VITEST_NO_OUTPUT_RETRY_ENV_KEY = "OPENCLAW_VITEST_NO_OUTPUT_RETRY";
 const LIVE_DOCKER_AUTH_SHELL_TARGETS = [
  "scripts/lib/live-docker-auth.sh",
  "scripts/test-live-acp-bind-docker.sh",
@@ -39,35 +34,6 @@ export function createChangedCheckChildEnv(baseEnv = process.env) {
  };
 }

-export function createChangedCheckVitestEnv(baseEnv = process.env) {
-  const resolvedBaseEnv = createChangedCheckChildEnv(baseEnv);
-  const env = {
-    ...resolvedBaseEnv,
-    [VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY]:
-      resolvedBaseEnv[VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY]?.trim() ||
-      CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS,
-    [VITEST_NO_OUTPUT_RETRY_ENV_KEY]:
-      resolvedBaseEnv[VITEST_NO_OUTPUT_RETRY_ENV_KEY]?.trim() || "0",
-  };
-
-  const hasWorkerOverride = Boolean(
-    (resolvedBaseEnv.OPENCLAW_VITEST_MAX_WORKERS ?? resolvedBaseEnv.OPENCLAW_TEST_WORKERS)?.trim(),
-  );
-  const hasParallelOverride = Boolean(resolvedBaseEnv.OPENCLAW_TEST_PROJECTS_PARALLEL?.trim());
-  const serialOverride = resolvedBaseEnv.OPENCLAW_TEST_PROJECTS_SERIAL?.trim();
-  if (
-    !isCiLikeEnv(resolvedBaseEnv) &&
-    !hasWorkerOverride &&
-    !hasParallelOverride &&
-    serialOverride !== "0"
-  ) {
-    env.OPENCLAW_TEST_PROJECTS_SERIAL = serialOverride || "1";
-    env.OPENCLAW_VITEST_MAX_WORKERS = "1";
-  }
-
-  return env;
-}
-
 export function createChangedCheckPlan(result, options = {}) {
  const commands = [];
  const baseEnv = createChangedCheckChildEnv(options.env ?? process.env);
@@ -93,10 +59,6 @@ export function createChangedCheckPlan(result, options = {}) {
  if (result.docsOnly) {
    return {
      commands,
-      testTargets: [],
-      runChangedTestsBroad: false,
-      runFullTests: false,
-      runExtensionTests: false,
      summary: "docs-only",
    };
  }
@@ -118,10 +80,6 @@ export function createChangedCheckPlan(result, options = {}) {
    add("root dependency ownership", ["deps:root-ownership:check"]);
    return {
      commands,
-      testTargets: [],
-      runChangedTestsBroad: false,
-      runFullTests: false,
-      runExtensionTests: false,
      summary: "release metadata",
    };
  }
@@ -132,10 +90,6 @@ export function createChangedCheckPlan(result, options = {}) {
    add("runtime import cycles", ["check:import-cycles"]);
    return {
      commands,
-      testTargets: [],
-      runChangedTestsBroad: false,
-      runFullTests: true,
-      runExtensionTests: false,
      summary: "all",
    };
  }
@@ -189,26 +143,10 @@ export function createChangedCheckPlan(result, options = {}) {
      OPENCLAW_DOCKER_ALL_DRY_RUN: "1",
      OPENCLAW_DOCKER_ALL_LIVE_MODE: "only",
    });
-    add(
-      "ACP bind unit tests",
-      ["test", "src/gateway/live-agent-probes.test.ts", "src/agents/acp-spawn.test.ts"],
-      createChangedCheckVitestEnv(baseEnv),
-    );
-    add("ACPX extension tests", ["test:extension", "acpx"], createChangedCheckVitestEnv(baseEnv));
  }

-  const testPlan = resolveChangedTestTargetPlan(result.paths);
-  const runExtensionTests = result.extensionImpactFromCore;
-  const testTargets = runExtensionTests
-    ? testPlan.targets.filter((target) => target !== "extensions")
-    : testPlan.targets;
-  const runChangedTestsBroad = testPlan.mode === "broad";
  return {
    commands,
-    testTargets,
-    runChangedTestsBroad,
-    runFullTests: false,
-    runExtensionTests,
    summary: Object.entries(lanes)
      .filter(([, enabled]) => enabled)
      .map(([lane]) => lane)
@@ -244,61 +182,6 @@ export async function runChangedCheck(result, options = {}) {
      }
    }

-    if (plan.runFullTests) {
-      const status = await runPnpm(
-        { name: "tests all", args: ["test"], env: createChangedCheckVitestEnv(childEnv) },
-        timings,
-      );
-      if (status !== 0) {
-        printSummary(timings, options);
-        return status;
-      }
-    } else if (plan.runChangedTestsBroad) {
-      const testArgs = options.explicitPaths
-        ? ["test"]
-        : ["test", "--changed", options.base ?? "origin/main"];
-      const status = await runPnpm(
-        {
-          name: options.explicitPaths ? "tests all" : "tests changed broad",
-          args: testArgs,
-          env: createChangedCheckVitestEnv(childEnv),
-        },
-        timings,
-      );
-      if (status !== 0) {
-        printSummary(timings, options);
-        return status;
-      }
-    } else if (plan.testTargets.length > 0) {
-      const status = await runPnpm(
-        {
-          name: "tests changed",
-          args: ["test", ...plan.testTargets],
-          env: createChangedCheckVitestEnv(childEnv),
-        },
-        timings,
-      );
-      if (status !== 0) {
-        printSummary(timings, options);
-        return status;
-      }
-    }
-
-    if (plan.runExtensionTests) {
-      const status = await runPnpm(
-        {
-          name: "tests extensions",
-          args: ["test:extensions"],
-          env: createChangedCheckVitestEnv(childEnv),
-        },
-        timings,
-      );
-      if (status !== 0) {
-        printSummary(timings, options);
-        return status;
-      }
-    }
-
    printSummary(timings, options);
    return 0;
  } finally {
@@ -314,17 +197,11 @@ function printPlan(result, plan, options) {
  const prefix = options.dryRun ? "[check:changed:dry-run]" : "[check:changed]";
  console.error(`${prefix} lanes=${plan.summary || "none"}`);
  if (result.extensionImpactFromCore) {
-    console.error(`${prefix} core contract changed; extension tests included`);
-  }
-  if (plan.runChangedTestsBroad) {
-    console.error(`${prefix} broad changed tests included`);
+    console.error(`${prefix} extension-impacting surface; extension typecheck included`);
  }
  for (const reason of result.reasons) {
    console.error(`${prefix} ${reason}`);
  }
-  if (plan.testTargets.length > 0) {
-    console.error(`${prefix} test targets=${plan.testTargets.length}`);
-  }
 }

 async function runPnpm(command, timings) {
--- a/scripts/test-projects.mjs
+++ b/scripts/test-projects.mjs
@@ -1,5 +1,6 @@
 import fs from "node:fs";
 import { performance } from "node:perf_hooks";
+import { formatMs } from "./lib/check-timing-summary.mjs";
 import { acquireLocalHeavyCheckLockSync } from "./lib/local-heavy-check-runtime.mjs";
 import {
  isCiLikeEnv,
@@ -271,6 +272,7 @@ async function runVitestSpecsParallel(specs, concurrency) {
 }

 async function main() {
+  const suiteStartedAt = performance.now();
  const args = process.argv.slice(2);
  const baseEnv = resolveLocalVitestEnv(process.env);
  const { targetArgs } = parseTestProjectsArgs(args, process.cwd());
@@ -309,6 +311,7 @@ async function main() {

  if (runSpecs.length === 0) {
    console.error("[test] no changed test targets; skipping Vitest.");
+    printTestSummary("skipped", 0, performance.now() - suiteStartedAt);
    return;
  }

@@ -360,8 +363,11 @@ async function main() {
        concurrency,
      );
      writeShardTimings(timings, process.cwd(), baseEnv);
-      console.error(
-        `[test] completed ${parallelSpecs.length} Vitest shards; Vitest summaries above are per-shard, not aggregate totals.`,
+      printTestSummary(
+        parallelExitCode === 0 ? "passed" : "failed",
+        parallelSpecs.length,
+        performance.now() - suiteStartedAt,
+        "Vitest summaries above are per-shard, not aggregate totals.",
      );
      releaseLockOnce();
      if (parallelExitCode !== 0) {
@@ -378,18 +384,24 @@ async function main() {
    if (!result) {
      return;
    }
+    if (result.timing) {
+      timings.push(result.timing);
+    }
    if (result.code !== 0) {
      exitCode = exitCode || result.code;
      if (spec.continueOnFailure !== true) {
+        printTestSummary("failed", timings.length, performance.now() - suiteStartedAt);
        releaseLockOnce();
        process.exit(result.code);
      }
    }
-    if (result.timing) {
-      timings.push(result.timing);
-    }
  }
  writeShardTimings(timings, process.cwd(), baseEnv);
+  printTestSummary(
+    exitCode === 0 ? "passed" : "failed",
+    timings.length,
+    performance.now() - suiteStartedAt,
+  );

  releaseLockOnce();
  if (exitCode !== 0) {
@@ -397,6 +409,13 @@ async function main() {
  }
 }

+function printTestSummary(status, shardCount, durationMs, detail) {
+  const suffix = detail ? `; ${detail}` : "";
+  console.error(
+    `[test] ${status} ${shardCount} Vitest shard${shardCount === 1 ? "" : "s"} in ${formatMs(durationMs)}${suffix}`,
+  );
+}
+
 main().catch((error) => {
  releaseLockOnce();
  console.error(error);
--- a/scripts/test-projects.test-support.d.mts
+++ b/scripts/test-projects.test-support.d.mts
@@ -17,7 +17,7 @@ export type VitestRunSpec = {
 export type ChangedTestTargetOptions = {
  cwd?: string;
  env?: Record<string, string | undefined>;
-  focused?: boolean;
+  broad?: boolean;
 };

 export const DEFAULT_TEST_PROJECTS_VITEST_NO_OUTPUT_TIMEOUT_MS: string;
--- a/scripts/test-projects.test-support.mjs
+++ b/scripts/test-projects.test-support.mjs
@@ -1,4 +1,3 @@
-import { execFileSync } from "node:child_process";
 import fs from "node:fs";
 import os from "node:os";
 import path from "node:path";
@@ -40,7 +39,10 @@ import {
  isBoundaryTestFile,
  isBundledPluginDependentUnitTestFile,
 } from "../test/vitest/vitest.unit-paths.mjs";
-import { detectChangedLanes } from "./changed-lanes.mjs";
+import {
+  detectChangedLanes,
+  listChangedPathsFromGit as listChangedPathsFromGitSource,
+} from "./changed-lanes.mjs";
 import { isCiLikeEnv, resolveLocalFullSuiteProfile } from "./lib/vitest-local-scheduling.mjs";
 import { resolveVitestCliEntry, resolveVitestNodeArgs } from "./run-vitest.mjs";

@@ -207,7 +209,7 @@ const VITEST_CONFIG_BY_KIND = {
  utils: UTILS_VITEST_CONFIG,
  wizard: WIZARD_VITEST_CONFIG,
 };
-const BROAD_CHANGED_RERUN_PATTERNS = [
+const BROAD_CHANGED_FALLBACK_PATTERNS = [
  /^package\.json$/u,
  /^pnpm-lock\.yaml$/u,
  /^test\/setup(?:\.shared|\.extensions|-openclaw-runtime)?\.ts$/u,
@@ -305,7 +307,7 @@ const SOURCE_ROOTS_FOR_IMPORT_GRAPH = ["src", "extensions", "packages", "ui/src"
 const IMPORTABLE_FILE_EXTENSIONS = [".ts", ".tsx", ".mts", ".cts"];
 const IMPORT_SPECIFIER_PATTERN =
  /\b(?:import|export)\s+(?:type\s+)?(?:[^'"]*?\s+from\s+)?["']([^"']+)["']|\bimport\s*\(\s*["']([^"']+)["']\s*\)/gu;
-const FOCUSED_CHANGED_ENV_KEY = "OPENCLAW_TEST_CHANGED_FOCUSED";
+const BROAD_CHANGED_ENV_KEY = "OPENCLAW_TEST_CHANGED_BROAD";
 const VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY = "OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS";
 const VITEST_NO_OUTPUT_RETRY_ENV_KEY = "OPENCLAW_VITEST_NO_OUTPUT_RETRY";
 export const DEFAULT_TEST_PROJECTS_VITEST_NO_OUTPUT_TIMEOUT_MS = "180000";
@@ -594,36 +596,7 @@ function resolveChannelContractTargetKind(relative) {
 }

 function listChangedPathsFromGit(baseRef, cwd) {
-  return [
-    ...new Set([
-      ...runGitNameOnlyDiff(cwd, [`${baseRef}...HEAD`]),
-      ...runGitNameOnlyDiff(cwd, ["--cached", "--diff-filter=ACMR"]),
-      ...runGitNameOnlyDiff(cwd, ["--diff-filter=ACMR"]),
-      ...runGitLsFiles(cwd, ["--others", "--exclude-standard"]),
-    ]),
-  ].toSorted((left, right) => left.localeCompare(right));
-}
-
-function runGitNameOnlyDiff(cwd, extraArgs) {
-  return execFileSync("git", ["diff", "--name-only", ...extraArgs], {
-    cwd,
-    encoding: "utf8",
-    stdio: ["ignore", "pipe", "pipe"],
-  })
-    .split("\n")
-    .map((line) => normalizePathPattern(line.trim()))
-    .filter((line) => line.length > 0);
-}
-
-function runGitLsFiles(cwd, extraArgs) {
-  return execFileSync("git", ["ls-files", ...extraArgs], {
-    cwd,
-    encoding: "utf8",
-    stdio: ["ignore", "pipe", "pipe"],
-  })
-    .split("\n")
-    .map((line) => normalizePathPattern(line.trim()))
-    .filter((line) => line.length > 0);
+  return listChangedPathsFromGitSource({ base: baseRef, cwd });
 }

 function extractChangedBaseRef(args) {
@@ -665,7 +638,7 @@ function shouldKeepBroadChangedRun(changedPaths) {
  return changedPaths.some((changedPath) =>
    PRECISE_SOURCE_TEST_TARGETS.has(changedPath)
      ? false
-      : BROAD_CHANGED_RERUN_PATTERNS.some((pattern) => pattern.test(changedPath)),
+      : BROAD_CHANGED_FALLBACK_PATTERNS.some((pattern) => pattern.test(changedPath)),
  );
 }

@@ -685,8 +658,8 @@ function resolveToolingTestTargets(changedPath) {
  return TOOLING_SOURCE_TEST_TARGETS.get(changedPath) ?? TOOLING_TEST_TARGETS.get(changedPath);
 }

-function shouldUseFocusedChangedTargets(env = process.env) {
-  const value = env[FOCUSED_CHANGED_ENV_KEY]?.trim().toLowerCase();
+function shouldUseBroadChangedTargets(env = process.env) {
+  const value = env[BROAD_CHANGED_ENV_KEY]?.trim().toLowerCase();
  return ["1", "true", "yes", "on"].includes(value ?? "");
 }

@@ -741,7 +714,8 @@ export function resolveChangedTestTargetPlan(changedPaths, options = {}) {
    return { mode: "targets", targets: toolingTargets };
  }
  const changedLanes = detectChangedLanes(changedPaths);
-  const focused = options.focused ?? shouldUseFocusedChangedTargets(options.env ?? {});
+  const env = options.env ?? {};
+  const useBroadFallback = options.broad ?? shouldUseBroadChangedTargets(env);
  const targets = [];
  for (const changedPath of changedPaths) {
    const preciseTargets = resolvePreciseChangedTestTargets(changedPath, options);
@@ -749,20 +723,21 @@ export function resolveChangedTestTargetPlan(changedPaths, options = {}) {
      targets.push(...preciseTargets);
      continue;
    }
-    if (focused) {
+    const needsBroadFallback = shouldKeepBroadChangedRun([changedPath]) || changedLanes.lanes.all;
+    if (needsBroadFallback) {
+      if (useBroadFallback) {
+        return { mode: "broad", targets: [] };
+      }
      continue;
    }
-    if (shouldKeepBroadChangedRun([changedPath]) || changedLanes.lanes.all) {
-      return { mode: "broad", targets: [] };
-    }
    if (isRoutableChangedTarget(changedPath)) {
      targets.push(changedPath);
    }
  }
-  if (!focused && changedLanes.lanes.all) {
+  if (useBroadFallback && changedLanes.lanes.all) {
    return { mode: "broad", targets: [] };
  }
-  if (!focused && changedLanes.extensionImpactFromCore) {
+  if (useBroadFallback && changedLanes.extensionImpactFromCore) {
    targets.push("extensions");
  }
  return { mode: "targets", targets: [...new Set(targets)] };