From 57761ebe8cb3dbd29f883f8bdade6902eb7361b9 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 15 Jul 2026 01:16:47 -0700 Subject: [PATCH] perf(ci): balance node test shards and gate heavy CI lanes by changed scope (#108091) --- .github/workflows/ci.yml | 113 +++------ scripts/ci-run-node-test-shard.d.mts | 41 ++++ scripts/ci-run-node-test-shard.mjs | 191 +++++++++++++++ scripts/lib/ci-changed-node-test-plan.d.mts | 6 +- scripts/lib/ci-changed-node-test-plan.mjs | 142 ++++++++---- scripts/lib/ci-node-test-plan.mjs | 218 +++++++++++++----- .../scripts/ci-changed-node-test-plan.test.ts | 104 +++++++-- test/scripts/ci-node-test-plan.test.ts | 93 +++++--- test/scripts/ci-run-node-test-shard.test.ts | 176 ++++++++++++++ test/scripts/ci-workflow-guards.test.ts | 9 +- 10 files changed, 849 insertions(+), 244 deletions(-) create mode 100644 scripts/ci-run-node-test-shard.d.mts create mode 100644 scripts/ci-run-node-test-shard.mjs create mode 100644 test/scripts/ci-run-node-test-shard.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 258c442c8c3f..cde117a9dee5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -540,8 +540,22 @@ jobs: console.warn(`Changed Node test planning failed; using compact full suite: ${error}`); } } + // Targeted (narrow-diff) PRs skip the heavy packaging lanes unless the + // diff touches surfaces those lanes exist to prove: test-only diffs + // cannot change dist bytes, and QA smoke re-proves packaging/scenario + // surfaces that unit shards plus build smokes already cover elsewhere. + const changedScopeHasBuildImpact = + changedNodeTestShards === null || + typeof changedNodeTestPlan.hasBuildArtifactAffectingChange !== "function" || + changedNodeTestPlan.hasBuildArtifactAffectingChange(changedPaths); + const changedScopeHasQaImpact = + changedNodeTestShards === null || + typeof changedNodeTestPlan.hasQaSmokeAffectingChange !== "function" || + changedNodeTestPlan.hasQaSmokeAffectingChange(changedPaths); + const runBuildArtifacts = runNodeFull && changedScopeHasBuildImpact; const runQaSmokeCi = runNodeFull && + changedScopeHasQaImpact && (!frozenTarget || existsSync("extensions/qa-lab/src/ci-smoke-plan.ts")); const nodeTestShards = runNodeFull ? (changedNodeTestShards @@ -571,8 +585,12 @@ jobs: const nodeTestNonDistShards = nodeTestShards.filter((shard) => !shard.requires_dist); const nodeTestDistShards = nodeTestShards.filter((shard) => shard.requires_dist); // Targeted jobs cannot discover repository-scanning boundary tests - // through imports, so preserve the existing full boundary gate. - const runNodeCoreDist = changedNodeTestShards !== null || nodeTestDistShards.length > 0; + // through imports. Keep the full boundary gate inside build-artifacts + // when that lane runs; test-only targeted plans carry their own + // nondist changed-boundary shard instead. + const runNodeCoreDist = + (changedNodeTestShards !== null && runBuildArtifacts) || + nodeTestDistShards.length > 0; const channelContractShards = runNodeFull ? createChannelContractTestShards() : []; const protocolCoverageRequested = runNode || runIosBuild || runAndroid; const runProtocolEventCoverage = @@ -587,7 +605,7 @@ jobs: run_android: runAndroid, run_skills_python: runSkillsPython, run_windows: runWindows, - run_build_artifacts: runNodeFull, + run_build_artifacts: runBuildArtifacts, run_checks_fast_core: checksFastCoreTasks.length > 0, run_checks_fast: runNodeFull, historical_target: historicalTarget, @@ -1297,10 +1315,10 @@ jobs: ;; contracts-plugins-ci-routing) pnpm test:contracts:plugins - pnpm test src/commands/status.scan-result.test.ts src/scripts/ci-changed-scope.test.ts test/scripts/changed-lanes.test.ts test/scripts/ci-changed-node-test-plan.test.ts test/scripts/ci-workflow-guards.test.ts test/scripts/run-vitest.test.ts test/scripts/test-projects.test.ts + pnpm test src/commands/status.scan-result.test.ts src/scripts/ci-changed-scope.test.ts test/scripts/changed-lanes.test.ts test/scripts/ci-changed-node-test-plan.test.ts test/scripts/ci-run-node-test-shard.test.ts test/scripts/ci-workflow-guards.test.ts test/scripts/run-vitest.test.ts test/scripts/test-projects.test.ts ;; ci-routing) - pnpm test src/commands/status.scan-result.test.ts src/scripts/ci-changed-scope.test.ts test/scripts/changed-lanes.test.ts test/scripts/ci-changed-node-test-plan.test.ts test/scripts/ci-workflow-guards.test.ts test/scripts/run-vitest.test.ts test/scripts/test-projects.test.ts + pnpm test src/commands/status.scan-result.test.ts src/scripts/ci-changed-scope.test.ts test/scripts/changed-lanes.test.ts test/scripts/ci-changed-node-test-plan.test.ts test/scripts/ci-run-node-test-shard.test.ts test/scripts/ci-workflow-guards.test.ts test/scripts/run-vitest.test.ts test/scripts/test-projects.test.ts ;; max-lines-ratchet) if ! has_package_script "check:max-lines-ratchet"; then @@ -1682,83 +1700,8 @@ jobs: OPENCLAW_VITEST_SHARD_NAME: ${{ matrix.shard_name }} OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS: "300000" OPENCLAW_VITEST_NO_OUTPUT_RETRY: "1" - OPENCLAW_TEST_PROJECTS_PARALLEL: "2" shell: bash - run: | - set -euo pipefail - node --input-type=module <<'EOF' - import { spawnSync } from "node:child_process"; - import { writeFileSync } from "node:fs"; - import { join } from "node:path"; - - const targets = JSON.parse(process.env.OPENCLAW_NODE_TEST_TARGETS_JSON ?? "null"); - if (Array.isArray(targets) && targets.length > 0) { - // One file per process preserves the isolation boundaries encoded by - // full-suite include-pattern shards while keeping one runner job. - for (const target of targets) { - const result = spawnSync( - "pnpm", - ["exec", "node", "scripts/test-projects.mjs", target], - { env: process.env, stdio: "inherit" }, - ); - if (result.status !== 0) { - process.exit(result.status ?? 1); - } - } - process.exit(0); - } - - const groups = JSON.parse(process.env.OPENCLAW_NODE_TEST_GROUPS_JSON ?? "null"); - const plans = Array.isArray(groups) && groups.length > 0 - ? groups - : [{ - configs: JSON.parse(process.env.OPENCLAW_NODE_TEST_CONFIGS_JSON ?? "[]"), - env: JSON.parse(process.env.OPENCLAW_NODE_TEST_ENV_JSON ?? "null"), - includePatterns: JSON.parse( - process.env.OPENCLAW_NODE_TEST_INCLUDE_PATTERNS_JSON ?? "null", - ), - shard_name: process.env.OPENCLAW_VITEST_SHARD_NAME, - }]; - for (const plan of plans) { - const configs = plan.configs; - if (!Array.isArray(configs) || configs.length === 0) { - console.error("Missing node test shard configs"); - process.exit(1); - } - const childEnv = { - ...process.env, - ...(plan.shard_name ? { OPENCLAW_VITEST_SHARD_NAME: plan.shard_name } : {}), - }; - if (plan.env && typeof plan.env === "object" && !Array.isArray(plan.env)) { - for (const [key, value] of Object.entries(plan.env)) { - if (typeof value === "string") { - childEnv[key] = value; - } - } - } - if (Array.isArray(plan.includePatterns) && plan.includePatterns.length > 0) { - const includeFile = join( - process.env.RUNNER_TEMP ?? ".", - `node-test-include-${process.env.GITHUB_JOB ?? "local"}-${Date.now()}.json`, - ); - writeFileSync(includeFile, JSON.stringify(plan.includePatterns), "utf8"); - childEnv.OPENCLAW_VITEST_INCLUDE_FILE = includeFile; - } else { - delete childEnv.OPENCLAW_VITEST_INCLUDE_FILE; - } - const result = spawnSync( - "pnpm", - ["exec", "node", "scripts/test-projects.mjs", ...configs], - { - env: childEnv, - stdio: "inherit", - }, - ); - if ((result.status ?? 1) !== 0) { - process.exit(result.status ?? 1); - } - } - EOF + run: node scripts/ci-run-node-test-shard.mjs # Types, lint, and format check shards. check-shard: @@ -1794,7 +1737,9 @@ jobs: runner: blacksmith-4vcpu-ubuntu-2404 - check_name: check-test-types task: test-types - runner: blacksmith-4vcpu-ubuntu-2404 + # Slowest static lane (~3.5min on 4 vCPU); extra cores shorten the + # PR critical path for roughly the same billed core-minutes. + runner: blacksmith-16vcpu-ubuntu-2404 steps: - *linux_node_checkout_step - name: Setup Node environment @@ -1904,7 +1849,9 @@ jobs: - check_name: check-additional-boundaries-a group: boundaries boundary_shard: 1/4 - runner: blacksmith-8vcpu-ubuntu-2404 + # This shard runs ~4min on 8 vCPU and regularly owns the PR wall + # clock; extra cores shorten it for similar billed core-minutes. + runner: blacksmith-16vcpu-ubuntu-2404 - check_name: check-additional-boundaries-bcd group: boundaries boundary_shard: 2/4,3/4,4/4 diff --git a/scripts/ci-run-node-test-shard.d.mts b/scripts/ci-run-node-test-shard.d.mts new file mode 100644 index 000000000000..9bb7848645ff --- /dev/null +++ b/scripts/ci-run-node-test-shard.d.mts @@ -0,0 +1,41 @@ +export type ShardTargetPlan = { + kind: "target"; + name: string; + target: string; +}; + +export type ShardGroupPlan = { + kind: "group"; + name: string; + plan: { + configs: string[]; + env?: Record | null; + includePatterns?: string[] | null; + shard_name?: string; + }; +}; + +export type ShardPlan = ShardTargetPlan | ShardGroupPlan; + +export function resolveShardPlans(env?: Record): ShardPlan[]; + +export function buildChildEnv( + entry: ShardPlan, + baseEnv: Record, + scratchDir: string, + index: number, +): Record; + +export function runShardPlans( + plans: ShardPlan[], + options?: { + concurrency?: number; + env?: Record; + runChild?: ( + args: string[], + childEnv: Record, + label: string, + ) => Promise; + scratchDir?: string; + }, +): Promise; diff --git a/scripts/ci-run-node-test-shard.mjs b/scripts/ci-run-node-test-shard.mjs new file mode 100644 index 000000000000..8d24d02ef90e --- /dev/null +++ b/scripts/ci-run-node-test-shard.mjs @@ -0,0 +1,191 @@ +// Runs one CI node test shard job: either explicit changed-test targets or a +// list of packed group plans. Extracted from .github/workflows/ci.yml so the +// execution policy is unit-testable and plans can run concurrently. +import { spawn } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { StringDecoder } from "node:string_decoder"; +import { isDirectRunUrl } from "./lib/direct-run.mjs"; +import { acquireLocalHeavyCheckLockSync } from "./lib/local-heavy-check-runtime.mjs"; + +// Two concurrent plans halve the serial tail of packed jobs. Children run with +// inner test-projects parallelism 1 so a job never exceeds two Vitest runs; +// stacking outer and inner parallelism oversubscribes the 4 vCPU runner class. +const PLAN_CONCURRENCY = 2; +const FS_MODULE_CACHE_PATH_ENV_KEY = "OPENCLAW_VITEST_FS_MODULE_CACHE_PATH"; + +function parseJsonEnv(env, name, fallback = null) { + try { + return JSON.parse(env[name] ?? "null") ?? fallback; + } catch { + return fallback; + } +} + +export function resolveShardPlans(env = process.env) { + const targets = parseJsonEnv(env, "OPENCLAW_NODE_TEST_TARGETS_JSON"); + if (Array.isArray(targets) && targets.length > 0) { + // One target per child process preserves the isolation boundaries encoded + // by full-suite include-pattern shards while keeping one runner job. + return targets.map((target) => ({ kind: "target", name: target, target })); + } + + const groups = parseJsonEnv(env, "OPENCLAW_NODE_TEST_GROUPS_JSON"); + const plans = + Array.isArray(groups) && groups.length > 0 + ? groups + : [ + { + configs: parseJsonEnv(env, "OPENCLAW_NODE_TEST_CONFIGS_JSON", []), + env: parseJsonEnv(env, "OPENCLAW_NODE_TEST_ENV_JSON"), + includePatterns: parseJsonEnv(env, "OPENCLAW_NODE_TEST_INCLUDE_PATTERNS_JSON"), + shard_name: env.OPENCLAW_VITEST_SHARD_NAME, + }, + ]; + return plans.map((plan) => ({ + kind: "group", + name: plan.shard_name ?? plan.configs?.[0] ?? "group", + plan, + })); +} + +export function buildChildEnv(entry, baseEnv, scratchDir, index) { + const childEnv = { + ...baseEnv, + // Concurrent children must not share a Vitest module cache directory; + // shared caches race with ENOTEMPTY when two runs rewrite the same entries. + [FS_MODULE_CACHE_PATH_ENV_KEY]: join(scratchDir, `vitest-cache-${index}`), + OPENCLAW_TEST_PROJECTS_PARALLEL: "1", + // This wrapper holds the repo heavy-check lock; children skipping it is + // what lets two plans run concurrently instead of serializing on the lock. + OPENCLAW_TEST_HEAVY_CHECK_LOCK_HELD: "1", + }; + if (entry.kind === "target") { + return childEnv; + } + const plan = entry.plan; + if (plan.shard_name) { + childEnv.OPENCLAW_VITEST_SHARD_NAME = plan.shard_name; + } + if (plan.env && typeof plan.env === "object" && !Array.isArray(plan.env)) { + for (const [key, value] of Object.entries(plan.env)) { + if (typeof value === "string") { + childEnv[key] = value; + } + } + } + if (Array.isArray(plan.includePatterns) && plan.includePatterns.length > 0) { + const includeFile = join(scratchDir, `node-test-include-${index}.json`); + writeFileSync(includeFile, JSON.stringify(plan.includePatterns), "utf8"); + childEnv.OPENCLAW_VITEST_INCLUDE_FILE = includeFile; + } else { + delete childEnv.OPENCLAW_VITEST_INCLUDE_FILE; + } + return childEnv; +} + +const MAX_PENDING_LINE_CHARS = 1_000_000; + +function relayChildStream(stream, label) { + const decoder = new StringDecoder("utf8"); + let pending = ""; + const writeLine = (line) => { + if (!process.stdout.write(`[shard:${label}] ${line}\n`)) { + stream.pause(); + process.stdout.once("drain", () => stream.resume()); + } + }; + stream.on("data", (chunk) => { + pending += decoder.write(chunk); + const lines = pending.split("\n"); + pending = lines.pop() ?? ""; + for (const line of lines) { + writeLine(line); + } + if (pending.length > MAX_PENDING_LINE_CHARS) { + writeLine(pending); + pending = ""; + } + }); + return () => { + pending += decoder.end(); + if (pending !== "") { + writeLine(pending); + pending = ""; + } + }; +} + +function runChild(args, childEnv, label) { + return new Promise((resolve) => { + const child = spawn("pnpm", ["exec", "node", "scripts/test-projects.mjs", ...args], { + env: childEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + // Stream with a per-line label instead of buffering: children can run + // whole suites for hours and verbose output must not accumulate on the + // wrapper heap. Backpressure pauses the child stream while stdout drains, + // and an oversized newline-free tail is force-flushed so the pending + // partial line stays bounded too. + const flushers = [child.stdout, child.stderr].map((stream) => relayChildStream(stream, label)); + process.stdout.write(`[shard:${label}] begin\n`); + child.on("close", (code) => { + for (const flush of flushers) { + flush(); + } + process.stdout.write(`[shard:${label}] end (exit ${code ?? 1})\n`); + resolve(code ?? 1); + }); + child.on("error", (error) => { + process.stdout.write(`[shard:${label}] failed to spawn: ${error}\n`); + resolve(1); + }); + }); +} + +export async function runShardPlans(plans, options = {}) { + const baseEnv = options.env ?? process.env; + const concurrency = Math.max(1, options.concurrency ?? PLAN_CONCURRENCY); + const runner = options.runChild ?? runChild; + const scratchDir = options.scratchDir ?? mkdtempSync(join(tmpdir(), "openclaw-node-shard-")); + + let nextIndex = 0; + let exitCode = 0; + const workers = Array.from({ length: concurrency }, async () => { + while (nextIndex < plans.length && exitCode === 0) { + const index = nextIndex; + nextIndex += 1; + const entry = plans[index]; + const args = entry.kind === "target" ? [entry.target] : entry.plan.configs; + if (!Array.isArray(args) || args.length === 0) { + console.error(`Missing node test shard configs for ${entry.name}`); + exitCode = exitCode || 1; + return; + } + const childEnv = buildChildEnv(entry, baseEnv, scratchDir, index); + const code = await runner(args, childEnv, entry.name); + if (code !== 0) { + // Stop scheduling new plans after a failure; the in-flight sibling + // finishes so its buffered output still lands in the job log. + exitCode = exitCode || code; + } + } + }); + await Promise.all(workers); + return exitCode; +} + +if (isDirectRunUrl(process.argv[1], import.meta.url)) { + const plans = resolveShardPlans(); + const releaseLock = acquireLocalHeavyCheckLockSync({ + cwd: process.cwd(), + env: process.env, + toolName: "test", + }); + try { + process.exitCode = await runShardPlans(plans); + } finally { + releaseLock(); + } +} diff --git a/scripts/lib/ci-changed-node-test-plan.d.mts b/scripts/lib/ci-changed-node-test-plan.d.mts index 509ab5e51b0e..1b1d8d7e7626 100644 --- a/scripts/lib/ci-changed-node-test-plan.d.mts +++ b/scripts/lib/ci-changed-node-test-plan.d.mts @@ -4,10 +4,14 @@ export type ChangedNodeTestShard = { requiresDist: boolean; runner: string; shardName: string; - targets: string[]; + targets?: string[]; }; export function createChangedNodeTestShards( changedPaths: string[], options?: { cwd?: string }, ): ChangedNodeTestShard[] | null; + +export function hasBuildArtifactAffectingChange(changedPaths: string[]): boolean; + +export function hasQaSmokeAffectingChange(changedPaths: string[]): boolean; diff --git a/scripts/lib/ci-changed-node-test-plan.mjs b/scripts/lib/ci-changed-node-test-plan.mjs index a4557d1a7162..6172d3d6b5e9 100644 --- a/scripts/lib/ci-changed-node-test-plan.mjs +++ b/scripts/lib/ci-changed-node-test-plan.mjs @@ -12,8 +12,11 @@ import { createNodeTestShards } from "./ci-node-test-plan.mjs"; import { buildPluginSdkEntrySources, publicPluginSdkEntrypoints } from "./plugin-sdk-entries.mjs"; const DEFAULT_NODE_TEST_RUNNER = "blacksmith-8vcpu-ubuntu-2404"; -const DIST_DEPENDENT_CONFIGS = new Set(["test/vitest/vitest.boundary.config.ts"]); -const MAX_CHANGED_NODE_TEST_TARGETS = 20; +const MAX_CHANGED_NODE_TEST_TARGETS = 96; +// Each target runs in its own child process (isolation contract), so bound the +// serial tail per job; the shard runner overlaps two children at a time. +const CHANGED_NODE_TEST_TARGETS_PER_JOB = 12; +const BOUNDARY_NODE_TEST_CONFIG = "test/vitest/vitest.boundary.config.ts"; const publicPluginSdkEntrySources = Object.values( buildPluginSdkEntrySources(publicPluginSdkEntrypoints), ); @@ -28,8 +31,49 @@ const splitNodeTestConfigs = new Set( fullNodeTestShards.filter((shard) => shard.includePatterns).flatMap((shard) => shard.configs), ); +function isTestOnlyPath(changedPath) { + return isTestFileTarget(changedPath) || changedPath.startsWith("test/"); +} + /** - * Builds one bounded PR job from precise changed-test targets. + * True when any changed path can influence built dist/packaging bytes. + * Test-only diffs cannot change what `build:ci-artifacts` produces, so the + * manifest may skip the build-artifacts lane for them. + */ +export function hasBuildArtifactAffectingChange(changedPaths) { + return changedPaths.some((changedPath) => !isTestOnlyPath(changedPath)); +} + +const QA_SMOKE_CRITICAL_RE = + /^(?:extensions\/qa-lab|qa)\/|^scripts\/(?:build-all\.mjs|package-openclaw-for-docker\.mjs)$|^(?:package\.json|pnpm-lock\.yaml|npm-shrinkwrap\.json)$|^ui\//u; + +/** + * True when a changed path touches the QA smoke packaging/scenario surface. + * Deliberate product tradeoff: targeted PRs outside this surface skip QA smoke + * and rely on import-selected shards plus build-artifact CLI smokes. Targeting + * only fires for narrow non-SDK-impacting diffs, and QA smoke still runs on + * every node-scoped main push, so a missed regression breaks main visibly + * instead of shipping silently. + */ +export function hasQaSmokeAffectingChange(changedPaths) { + return changedPaths.some((changedPath) => QA_SMOKE_CRITICAL_RE.test(changedPath)); +} + +function createBoundaryShard() { + // Boundary tests scan the source tree (including test files) and build + // their own fixtures; they do not consume the built dist artifact. When the + // build-artifacts lane is skipped, this shard keeps that coverage. + return { + checkName: "checks-node-changed-boundary", + configs: [BOUNDARY_NODE_TEST_CONFIG], + requiresDist: false, + runner: DEFAULT_NODE_TEST_RUNNER, + shardName: "changed-boundary", + }; +} + +/** + * Builds bounded PR jobs from precise changed-test targets. * Null means the caller must fail safe to the compact full-suite plan. */ export function createChangedNodeTestShards(changedPaths, options = {}) { @@ -38,8 +82,16 @@ export function createChangedNodeTestShards(changedPaths, options = {}) { return null; } - // Deletions cannot be reconstructed reliably from the post-merge tree. - if (changedPaths.some((changedPath) => !existsSync(path.join(cwd, changedPath)))) { + const livePaths = []; + const deletedPaths = []; + for (const changedPath of changedPaths) { + (existsSync(path.join(cwd, changedPath)) ? livePaths : deletedPaths).push(changedPath); + } + // Deleted test files cannot regress runtime behavior, so they never block + // targeting. Deleted source files cannot be import-graphed from the merged + // tree and no live-path heuristic proves their consumers are covered, so + // any source deletion keeps the full-suite plan. + if (deletedPaths.some((deletedPath) => !isTestFileTarget(deletedPath))) { return null; } @@ -53,8 +105,8 @@ export function createChangedNodeTestShards(changedPaths, options = {}) { // Fail safe when a core change reaches a public SDK entrypoint indirectly. if ( detectChangedLanes(changedPaths).extensionImpactFromCore || - (changedPaths.some((changedPath) => changedPath.startsWith("src/")) && - hasImportGraphImpactOnTargets(changedPaths, publicPluginSdkEntrySources, cwd)) + (livePaths.some((changedPath) => changedPath.startsWith("src/")) && + hasImportGraphImpactOnTargets(livePaths, publicPluginSdkEntrySources, cwd)) ) { return null; } @@ -67,22 +119,25 @@ export function createChangedNodeTestShards(changedPaths, options = {}) { forceFullImportGraph: true, includeExtensionImpact: false, }); - const plan = resolveTargetPlan(changedPaths); + const plan = + livePaths.length > 0 ? resolveTargetPlan(livePaths) : { mode: "targets", targets: [] }; // Aggregate resolution must not let one precise path hide another path that // contributes no tests. Partial plans silently drop coverage. if ( - changedPaths.some((changedPath) => { + livePaths.some((changedPath) => { const changedPathPlan = resolveTargetPlan([changedPath]); return changedPathPlan.mode !== "targets" || changedPathPlan.targets.length === 0; }) ) { return null; } + if (plan.mode !== "targets") { + return null; + } + const targets = [...new Set(plan.targets)]; if ( - plan.mode !== "targets" || - plan.targets.length === 0 || - plan.targets.length > MAX_CHANGED_NODE_TEST_TARGETS || - plan.targets.some( + targets.length > MAX_CHANGED_NODE_TEST_TARGETS || + targets.some( (target) => /^test\/vitest\/vitest\.full-.*\.config\.ts$/u.test(target) || splitNodeTestConfigs.has(target), @@ -92,7 +147,7 @@ export function createChangedNodeTestShards(changedPaths, options = {}) { } if ( - plan.targets.some( + targets.some( (target) => !isTestFileTarget(target) || findUnmatchedExplicitTestTargets([target], cwd).length > 0, ) @@ -100,7 +155,7 @@ export function createChangedNodeTestShards(changedPaths, options = {}) { return null; } - const targetPlans = plan.targets.map((target) => ({ + const targetPlans = targets.map((target) => ({ plans: buildVitestRunPlans([target], cwd), target, })); @@ -121,37 +176,30 @@ export function createChangedNodeTestShards(changedPaths, options = {}) { return null; } - const nonDistTargets = targetPlans - .filter(({ plans }) => plans.some(({ config }) => !DIST_DEPENDENT_CONFIGS.has(config))) - .map(({ target }) => target); - const distTargets = targetPlans - .filter(({ plans }) => plans.some(({ config }) => DIST_DEPENDENT_CONFIGS.has(config))) - .map(({ target }) => target); - - return [ - ...(nonDistTargets.length > 0 - ? [ - { - checkName: "checks-node-changed", - configs: [], - requiresDist: false, - runner: DEFAULT_NODE_TEST_RUNNER, - shardName: "changed", - targets: nonDistTargets, - }, - ] - : []), - ...(distTargets.length > 0 - ? [ - { - checkName: "checks-node-changed-dist", - configs: ["test/vitest/vitest.boundary.config.ts"], - requiresDist: true, - runner: DEFAULT_NODE_TEST_RUNNER, - shardName: "changed-dist", - targets: distTargets, - }, - ] - : []), + // Boundary-config targets run as regular nondist targets: the boundary + // suite scans the checked-out tree and never consumes the built dist. + const orderedTargets = targetPlans.map(({ target }) => target); + const targetChunks = []; + for ( + let offset = 0; + offset < orderedTargets.length; + offset += CHANGED_NODE_TEST_TARGETS_PER_JOB + ) { + targetChunks.push(orderedTargets.slice(offset, offset + CHANGED_NODE_TEST_TARGETS_PER_JOB)); + } + const shards = [ + ...targetChunks.map((chunk, index) => { + const suffix = targetChunks.length === 1 ? "" : `-${index + 1}`; + return { + checkName: `checks-node-changed${suffix}`, + configs: [], + requiresDist: false, + runner: DEFAULT_NODE_TEST_RUNNER, + shardName: `changed${suffix}`, + targets: chunk, + }; + }), + ...(hasBuildArtifactAffectingChange(changedPaths) ? [] : [createBoundaryShard()]), ]; + return shards.length > 0 ? shards : null; } diff --git a/scripts/lib/ci-node-test-plan.mjs b/scripts/lib/ci-node-test-plan.mjs index 8ac4b692282f..a357b52edc48 100644 --- a/scripts/lib/ci-node-test-plan.mjs +++ b/scripts/lib/ci-node-test-plan.mjs @@ -28,11 +28,107 @@ const GATEWAY_STARTUP_HEALTH_RUNTIME_ENV = { const MAX_BUNDLED_NODE_TEST_PATTERNS = 64; // PR-only bundles trade a little serial work for fewer ephemeral runner registrations. // Keep runner classes and subprocess isolation intact while bounding each combined job. -const COMPACT_NODE_TEST_JOB_WEIGHT = 256; +const COMPACT_NODE_TEST_JOB_SECONDS = 260; const COMPACT_NODE_TEST_JOB_GROUPS = 10; -const COMPACT_TOOLING_NODE_TEST_GROUPS = 3; -const COMPACT_WHOLE_NODE_TEST_JOB_GROUPS = 8; +const COMPACT_TOOLING_NODE_TEST_GROUPS = 4; const COMPACT_WHOLE_NODE_TEST_TIMEOUT_MINUTES = 120; +const AUTO_REPLY_COMMANDS_STRIPES = 3; +// Advisory runtime estimates (seconds) per split shard, measured from a +// Blacksmith compact PR run under two-plans-per-job concurrency (run +// 29395969440). Packing only: a stale entry skews job balance but never +// correctness. Unknown shards fall back to a per-file estimate. +const COMPACT_GROUP_SECONDS_HINTS = new Map([ + ["agentic-agents-core-auth", 28], + ["agentic-agents-core-isolated", 12], + ["agentic-agents-core-models", 48], + ["agentic-agents-core-runner-cli", 120], + ["agentic-agents-core-runner-commands", 38], + ["agentic-agents-core-runner-embedded", 27], + ["agentic-agents-core-runner-sessions", 120], + ["agentic-agents-core-runtime", 81], + ["agentic-agents-core-subagents", 30], + ["agentic-agents-core-tools", 41], + ["agentic-agents-embedded", 69], + ["agentic-agents-support", 104], + ["agentic-agents-tools", 43], + ["agentic-cli", 70], + ["agentic-command-support", 30], + ["agentic-commands-agent-channel", 43], + ["agentic-commands-doctor", 26], + ["agentic-commands-doctor-auth", 8], + ["agentic-commands-doctor-config-state", 58], + ["agentic-commands-doctor-gateway", 6], + ["agentic-commands-doctor-plugins-tools", 14], + ["agentic-commands-doctor-sessions-cron", 22], + ["agentic-commands-models", 17], + ["agentic-commands-onboard-config", 25], + ["agentic-commands-status-tools", 19], + ["agentic-control-plane-agent-chat", 74], + ["agentic-control-plane-auth-node", 105], + ["agentic-control-plane-http-models", 34], + ["agentic-control-plane-http-plugin-ws", 28], + ["agentic-control-plane-runtime-config", 32], + ["agentic-control-plane-runtime-cron", 22], + ["agentic-control-plane-runtime-server", 40], + ["agentic-control-plane-runtime-state", 29], + ["agentic-control-plane-runtime-ui-tools", 23], + ["agentic-control-plane-startup-core", 85], + ["agentic-control-plane-startup-health-runtime", 20], + ["agentic-control-plane-startup-restart-close", 19], + ["agentic-gateway-core", 112], + ["agentic-gateway-methods", 90], + ["agentic-plugin-sdk", 52], + ["auto-reply-core-top-level", 26], + ["auto-reply-reply-agent-runner", 33], + ["auto-reply-reply-commands-1", 179], + ["auto-reply-reply-commands-2", 39], + ["auto-reply-reply-commands-3", 28], + ["auto-reply-reply-dispatch", 41], + ["auto-reply-reply-session", 38], + ["auto-reply-reply-state-routing", 22], + ["core-runtime-cron-core", 29], + ["core-runtime-cron-isolated-agent", 55], + ["core-runtime-cron-service", 13], + ["core-runtime-hooks", 11], + ["core-runtime-infra-approval-exec", 35], + ["core-runtime-infra-channel-plugin", 20], + ["core-runtime-infra-heartbeat-runner", 44], + ["core-runtime-infra-net-install", 15], + ["core-runtime-infra-outbound-actions", 15], + ["core-runtime-infra-outbound-core", 37], + ["core-runtime-infra-process", 75], + ["core-runtime-infra-provider-push", 21], + ["core-runtime-infra-storage-state", 41], + ["core-runtime-infra-system-runtime", 29], + ["core-runtime-media-ui", 123], + ["core-runtime-secrets", 30], + ["core-runtime-shared", 49], + // PTY timing tests inflate badly next to co-runners; keep this group in a + // lightly packed bin so its lane stays close to solo runtime. + ["core-runtime-tui-pty", 200], + ["core-tooling-1", 88], + ["core-tooling-2", 84], + ["core-tooling-3", 100], + ["core-tooling-4", 79], + ["core-tooling-docker", 6], + ["core-tooling-isolated", 50], + ["core-unit-fast", 157], + ["core-unit-src-security", 149], + ["core-unit-support", 28], +]); +const DEFAULT_WHOLE_GROUP_SECONDS = 25; +const DEFAULT_SECONDS_PER_TEST_FILE = 0.5; + +function estimateCompactGroupSeconds(group) { + const hint = COMPACT_GROUP_SECONDS_HINTS.get(group.shard_name); + if (hint !== undefined) { + return hint; + } + if (Array.isArray(group.includePatterns)) { + return Math.max(3, Math.round(group.includePatterns.length * DEFAULT_SECONDS_PER_TEST_FILE)); + } + return DEFAULT_WHOLE_GROUP_SECONDS; +} const TOOLING_CONFIG = "test/vitest/vitest.tooling.config.ts"; const TOOLING_DOCKER_TEST_FILE = "test/scripts/docker-build-helper.test.ts"; const TOOLING_ISOLATED_CONFIG = "test/vitest/vitest.tooling-isolated.config.ts"; @@ -40,7 +136,9 @@ const TOOLING_ISOLATED_CONFIG = "test/vitest/vitest.tooling-isolated.config.ts"; // shards first so short alphabetical groups cannot leave them on the tail. const FULL_NODE_TEST_ADMISSION_PRIORITY = new Map([ ["core-tooling", 0], - ["auto-reply-reply-commands", 1], + ["auto-reply-reply-commands-1", 1], + ["auto-reply-reply-commands-2", 1], + ["auto-reply-reply-commands-3", 1], ]); // Commands and cron run non-isolated, so keep their split shards as separate // processes. Combining their include lists can retain test state across groups. @@ -52,7 +150,10 @@ const KEEP_LARGE_NODE_TEST_RUNNER = new Set([ "agentic-agents-core-subagents", "agentic-agents-embedded", "agentic-agents-support", - "agentic-agents-core-runner", + "agentic-agents-core-runner-cli", + "agentic-agents-core-runner-commands", + "agentic-agents-core-runner-embedded", + "agentic-agents-core-runner-sessions", "agentic-agents-core-tools", "agentic-control-plane-startup-core", "agentic-gateway-core", @@ -103,12 +204,28 @@ function createAutoReplyReplySplitShards() { } return Object.entries(groups) - .map(([groupName, includePatterns]) => ({ - configs: ["test/vitest/vitest.auto-reply-reply.config.ts"], - includePatterns, - requiresDist: false, - shardName: groupName, - })) + .flatMap(([groupName, includePatterns]) => { + // The commands bucket alone serializes ~3 minutes; stripe it so packing + // can spread that runtime across jobs. + if (groupName === "auto-reply-reply-commands") { + return createStripedBatches(includePatterns, AUTO_REPLY_COMMANDS_STRIPES).map( + (batch, index) => ({ + configs: ["test/vitest/vitest.auto-reply-reply.config.ts"], + includePatterns: batch, + requiresDist: false, + shardName: `${groupName}-${index + 1}`, + }), + ); + } + return [ + { + configs: ["test/vitest/vitest.auto-reply-reply.config.ts"], + includePatterns, + requiresDist: false, + shardName: groupName, + }, + ]; + }) .filter((shard) => shard.includePatterns.length > 0); } @@ -267,15 +384,23 @@ function resolveAgentCoreShardName(file) { ) { return "agentic-agents-core-subagents"; } + // The former single "core-runner" bucket serialized ~3 minutes of tests in + // one group; keep these three slices separate so packing can balance them. + if (name.startsWith("embedded-agent-runner")) { + return "agentic-agents-core-runner-embedded"; + } if ( - name.startsWith("embedded-agent-runner") || - name.startsWith("cli-runner") || name.startsWith("agent-command") || name.startsWith("command") || - name.includes("compaction") || - name.includes("session") + name.includes("compaction") ) { - return "agentic-agents-core-runner"; + return "agentic-agents-core-runner-commands"; + } + if (name.startsWith("cli-runner")) { + return "agentic-agents-core-runner-cli"; + } + if (name.includes("session")) { + return "agentic-agents-core-runner-sessions"; } return "agentic-agents-core-runtime"; } @@ -297,7 +422,10 @@ function createAgentCoreSplitShards() { "agentic-agents-core-models", "agentic-agents-core-tools", "agentic-agents-core-subagents", - "agentic-agents-core-runner", + "agentic-agents-core-runner-cli", + "agentic-agents-core-runner-commands", + "agentic-agents-core-runner-embedded", + "agentic-agents-core-runner-sessions", "agentic-agents-core-runtime", ] .map((shardName) => ({ @@ -1196,62 +1324,31 @@ function createCompactNodeTestShardBundles(options = {}) { const compactJobs = []; for (const groups of groupsByRunner.values()) { + // First-fit decreasing on estimated serial seconds keeps every job near + // the same runtime; the old per-file weights let one 3-minute group land + // next to nine trivial ones and own the PR wall clock. const bins = []; const sortedGroups = groups.toSorted( (a, b) => - (b.includePatterns?.length ?? 1) - (a.includePatterns?.length ?? 1) || + estimateCompactGroupSeconds(b) - estimateCompactGroupSeconds(a) || a.shard_name.localeCompare(b.shard_name), ); - for (const group of sortedGroups.filter((candidate) => candidate.includePatterns)) { - const weight = group.includePatterns.length; + for (const group of sortedGroups) { + const weight = estimateCompactGroupSeconds(group); const bin = bins.find( (candidate) => candidate.groups.length < COMPACT_NODE_TEST_JOB_GROUPS && - candidate.weight + weight <= COMPACT_NODE_TEST_JOB_WEIGHT, + candidate.weight + weight <= COMPACT_NODE_TEST_JOB_SECONDS, ); if (bin) { bin.groups.push(group); bin.weight += weight; + bin.hasWholeConfigGroup ||= !group.includePatterns; } else { - bins.push({ groups: [group], weight }); + bins.push({ groups: [group], hasWholeConfigGroup: !group.includePatterns, weight }); } } - const wholeGroups = sortedGroups.filter((candidate) => !candidate.includePatterns); - const wholeJobCount = Math.ceil(wholeGroups.length / COMPACT_WHOLE_NODE_TEST_JOB_GROUPS); - // A lone whole-config job serializes every fixed suite and owns PR wall time. - // Fold it into same-runner jobs when caps allow, retaining the whole-config timeout. - const canSpreadWholeGroups = - wholeJobCount === 1 && - bins.length > 1 && - bins.every( - (bin) => - bin.groups.length + Math.ceil(wholeGroups.length / bins.length) <= - COMPACT_NODE_TEST_JOB_GROUPS, - ); - const wholeGroupBatches = canSpreadWholeGroups - ? [] - : createStripedBatches(wholeGroups, wholeJobCount); - if (canSpreadWholeGroups) { - for (const [index, group] of wholeGroups.entries()) { - const bin = bins[index % bins.length]; - bin.groups.push(group); - bin.timeoutMinutes = COMPACT_WHOLE_NODE_TEST_TIMEOUT_MINUTES; - } - } - for (const [index, groupBatch] of wholeGroupBatches.entries()) { - const runnerClass = groupBatch[0].runner.includes("-8vcpu-") ? "large" : "small"; - const distSuffix = groupBatch[0].requiresDist ? "-dist" : ""; - compactJobs.push({ - checkName: `checks-node-compact-${runnerClass}${distSuffix}-whole-${index + 1}`, - groups: groupBatch, - requiresDist: groupBatch[0].requiresDist, - runner: groupBatch[0].runner, - shardName: `compact-${runnerClass}${distSuffix}-whole-${index + 1}`, - timeoutMinutes: COMPACT_WHOLE_NODE_TEST_TIMEOUT_MINUTES, - }); - } - for (const [index, bin] of bins.entries()) { const runnerClass = bin.groups[0].runner.includes("-8vcpu-") ? "large" : "small"; const distSuffix = bin.groups[0].requiresDist ? "-dist" : ""; @@ -1260,8 +1357,11 @@ function createCompactNodeTestShardBundles(options = {}) { groups: bin.groups, requiresDist: bin.groups[0].requiresDist, runner: bin.groups[0].runner, - shardName: `compact-${runnerClass}-${index + 1}`, - ...(bin.timeoutMinutes ? { timeoutMinutes: bin.timeoutMinutes } : {}), + shardName: `compact-${runnerClass}${distSuffix}-${index + 1}`, + // Whole-config groups run entire suites; keep their generous timeout. + ...(bin.hasWholeConfigGroup + ? { timeoutMinutes: COMPACT_WHOLE_NODE_TEST_TIMEOUT_MINUTES } + : {}), }); } } diff --git a/test/scripts/ci-changed-node-test-plan.test.ts b/test/scripts/ci-changed-node-test-plan.test.ts index 5f4ee27c76bc..14830f67e1f9 100644 --- a/test/scripts/ci-changed-node-test-plan.test.ts +++ b/test/scripts/ci-changed-node-test-plan.test.ts @@ -2,7 +2,12 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; -import { createChangedNodeTestShards } from "../../scripts/lib/ci-changed-node-test-plan.mjs"; +import { + createChangedNodeTestShards, + hasBuildArtifactAffectingChange, + hasQaSmokeAffectingChange, +} from "../../scripts/lib/ci-changed-node-test-plan.mjs"; +import { listGitTrackedFiles } from "../../src/test-utils/repo-files.js"; describe("CI changed Node test plan", () => { it("routes a focused source change into one targeted job", () => { @@ -22,22 +27,77 @@ describe("CI changed Node test plan", () => { ]); }); - it("routes built-artifact boundary tests through the dist gate", () => { + it("keeps boundary coverage on test-only diffs without the build-artifacts lane", () => { + // Test-only diffs skip build-artifacts (which hosts the full boundary + // gate), so the plan carries its own nondist boundary shard instead. expect(createChangedNodeTestShards(["test/extension-import-boundaries.test.ts"])).toEqual([ { - checkName: "checks-node-changed-dist", - configs: ["test/vitest/vitest.boundary.config.ts"], - requiresDist: true, + checkName: "checks-node-changed", + configs: [], + requiresDist: false, runner: "blacksmith-8vcpu-ubuntu-2404", - shardName: "changed-dist", + shardName: "changed", targets: ["test/extension-import-boundaries.test.ts"], }, + { + checkName: "checks-node-changed-boundary", + configs: ["test/vitest/vitest.boundary.config.ts"], + requiresDist: false, + runner: "blacksmith-8vcpu-ubuntu-2404", + shardName: "changed-boundary", + }, ]); }); - it("fails safe to the full plan for broad or deleted changes", () => { + it("classifies build-artifact and QA smoke impact by changed surface", () => { + expect(hasBuildArtifactAffectingChange(["src/agents/foo.test.ts", "test/helpers/x.ts"])).toBe( + false, + ); + expect(hasBuildArtifactAffectingChange(["src/agents/foo.ts"])).toBe(true); + expect(hasQaSmokeAffectingChange(["extensions/qa-lab/src/ci-smoke-plan.ts"])).toBe(true); + expect(hasQaSmokeAffectingChange(["ui/src/app.ts"])).toBe(true); + expect(hasQaSmokeAffectingChange(["src/infra/retry.ts"])).toBe(false); + }); + + it("fails safe to the full plan for broad changes", () => { expect(createChangedNodeTestShards(["package.json"])).toBeNull(); - expect(createChangedNodeTestShards(["src/removed-module.ts"])).toBeNull(); + }); + + it("fails safe whenever a diff deletes source files", () => { + expect(createChangedNodeTestShards(["src/infra/format-time/deleted-helper.ts"])).toBeNull(); + expect( + createChangedNodeTestShards([ + "src/infra/format-time/deleted-helper.ts", + "src/agents/live-model-filter.ts", + ]), + ).toBeNull(); + }); + + it("keeps targeting when a diff only deletes test files alongside live source", () => { + const shards = createChangedNodeTestShards([ + "src/agents/deleted-obsolete.test.ts", + "src/agents/live-model-filter.ts", + ]); + expect(shards).not.toBeNull(); + const targets = shards?.flatMap((shard) => shard.targets ?? []) ?? []; + expect(targets).toContain("src/agents/live-model-filter.test.ts"); + }); + + it("runs only the boundary shard when a diff deletes test files", () => { + const cwd = mkdtempSync(path.join(tmpdir(), "openclaw-ci-deleted-test-")); + try { + expect(createChangedNodeTestShards(["src/gone.test.ts"], { cwd })).toEqual([ + { + checkName: "checks-node-changed-boundary", + configs: ["test/vitest/vitest.boundary.config.ts"], + requiresDist: false, + runner: "blacksmith-8vcpu-ubuntu-2404", + shardName: "changed-boundary", + }, + ]); + } finally { + rmSync(cwd, { force: true, recursive: true }); + } }); it("fails safe when an unresolved path is mixed with a precise source change", () => { @@ -97,16 +157,22 @@ describe("CI changed Node test plan", () => { ).toBeNull(); }); - it("fails safe before serial target processes can dominate job time", () => { - const cwd = mkdtempSync(path.join(tmpdir(), "openclaw-ci-many-targets-")); - try { - writeFileSync(path.join(cwd, "value.ts"), "export const value = 1;\n"); - for (let index = 0; index < 21; index += 1) { - writeFileSync(path.join(cwd, `consumer-${index}.test.ts`), 'import "./value.js";\n'); - } - expect(createChangedNodeTestShards(["value.ts"], { cwd })).toBeNull(); - } finally { - rmSync(cwd, { force: true, recursive: true }); - } + it("chunks many targets into bounded parallel jobs", () => { + // A wide test-file diff exercises the multi-chunk path against the real + // tree; the cron suite has well over one chunk's worth of test files. + const changedTests = listGitTrackedFiles({ pathspecs: "src/cron" }) + ?.filter((file) => file.endsWith(".test.ts") && !/\.(?:e2e|live)\.test\.ts$/u.test(file)) + .slice(0, 15); + expect(changedTests?.length).toBe(15); + const shards = createChangedNodeTestShards(changedTests ?? []); + expect(shards).not.toBeNull(); + const targetShards = shards?.filter((shard) => shard.targets) ?? []; + expect(targetShards.length).toBeGreaterThan(1); + expect( + targetShards.every((shard, index) => shard.checkName === `checks-node-changed-${index + 1}`), + ).toBe(true); + expect(targetShards.every((shard) => (shard.targets?.length ?? 0) <= 12)).toBe(true); + const targets = targetShards.flatMap((shard) => shard.targets ?? []); + expect(new Set(targets).size).toBe(targets.length); }); }); diff --git a/test/scripts/ci-node-test-plan.test.ts b/test/scripts/ci-node-test-plan.test.ts index c9fbb7019e33..aeb13ca349ee 100644 --- a/test/scripts/ci-node-test-plan.test.ts +++ b/test/scripts/ci-node-test-plan.test.ts @@ -145,9 +145,11 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { ).toBe(true); expect(bundled.every((shard) => shard.runner?.startsWith("blacksmith-"))).toBe(true); expect(bundled).toEqual(createNodeTestShardBundles({ includeReleaseOnlyPluginShards: false })); - expect(bundled.slice(0, 2).map((shard) => shard.shardName)).toEqual([ + expect(bundled.slice(0, 4).map((shard) => shard.shardName)).toEqual([ "core-tooling", - "auto-reply-reply-commands", + "auto-reply-reply-commands-1", + "auto-reply-reply-commands-2", + "auto-reply-reply-commands-3", ]); expect(bundled.find((shard) => shard.shardName === "core-unit-fast")?.runner).toBe( DEFAULT_NODE_TEST_RUNNER, @@ -184,15 +186,23 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { compact: true, }); - expect(compact).toHaveLength(15); - expect(compact.filter((shard) => !shard.requiresDist)).toHaveLength(14); + expect(compact.length).toBeGreaterThanOrEqual(12); + expect(compact.length).toBeLessThanOrEqual(24); expect(compact.every((shard) => Array.isArray(shard.groups))).toBe(true); + expect(compact.every((shard) => shard.groups.length <= 10)).toBe(true); expect(compact.some((shard) => shard.requiresDist)).toBe(true); expect( compact.every((shard) => shard.groups.every((group) => group.requiresDist === shard.requiresDist), ), ).toBe(true); + // Runtime-balanced packing must keep the two heaviest measured groups in + // different jobs; regressing to per-file weights recombines them. + const jobOf = (name: string) => + compact.findIndex((shard) => shard.groups.some((group) => group.shard_name === name)); + expect(jobOf("agentic-agents-core-runner-embedded")).toBeGreaterThanOrEqual(0); + expect(jobOf("core-unit-fast")).toBeGreaterThanOrEqual(0); + expect(jobOf("agentic-agents-core-runner-embedded")).not.toBe(jobOf("core-unit-fast")); expect( compact .flatMap((shard) => @@ -233,27 +243,14 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { .filter((shard) => shard.groups.some((group) => !group.includePatterns)) .every((shard) => shard.timeoutMinutes === 120), ).toBe(true); - const largeJobs = compact.filter((shard) => - shard.checkName.startsWith("checks-node-compact-large-"), - ); + // Whole-config groups now pack into the same runtime-balanced bins as + // include-pattern groups; the separate "-whole-" job class is gone. + expect(compact.some((shard) => shard.checkName.includes("-whole-"))).toBe(false); expect( - largeJobs.map((shard) => shard.groups.filter((group) => !group.includePatterns).length), - ).toEqual([2, 2, 2]); - expect( - compact.some((shard) => shard.checkName.startsWith("checks-node-compact-large-whole-")), - ).toBe(false); - const smallWholeJobs = compact.filter((shard) => - shard.checkName.startsWith("checks-node-compact-small-whole-"), - ); - const wholeGroupCounts = smallWholeJobs.map((shard) => shard.groups.length); - expect(Math.max(...wholeGroupCounts) - Math.min(...wholeGroupCounts)).toBeLessThanOrEqual(1); - expect( - smallWholeJobs.some((shard) => - shard.groups.some((group) => group.shard_name === "core-tooling"), - ), + compact.some((shard) => shard.groups.some((group) => group.shard_name === "core-tooling")), ).toBe(false); expect( - smallWholeJobs + compact .flatMap((shard) => shard.groups) .find((group) => group.shard_name === "core-tooling-isolated"), ).toEqual( @@ -262,7 +259,7 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { }), ); expect( - smallWholeJobs + compact .flatMap((shard) => shard.groups) .find((group) => group.shard_name === "core-tooling-docker"), ).toEqual( @@ -274,7 +271,7 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { .flatMap((shard) => shard.groups) .filter((group) => /^core-tooling-\d+$/u.test(group.shard_name)); const toolingFiles = toolingGroups.flatMap((group) => group.includePatterns ?? []); - expect(toolingGroups).toHaveLength(3); + expect(toolingGroups).toHaveLength(4); expect( toolingGroups.every((group) => group.configs[0] === "test/vitest/vitest.tooling.config.ts"), ).toBe(true); @@ -859,17 +856,41 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { shardName: "agentic-agents-core-subagents", }, { - checkName: "checks-node-agentic-agents-core-runner", + checkName: "checks-node-agentic-agents-core-runner-cli", configs: ["test/vitest/vitest.agents-core.config.ts"], includePatterns: agentShards[4]?.includePatterns, requiresDist: false, runner: DEFAULT_NODE_TEST_RUNNER, - shardName: "agentic-agents-core-runner", + shardName: "agentic-agents-core-runner-cli", + }, + { + checkName: "checks-node-agentic-agents-core-runner-commands", + configs: ["test/vitest/vitest.agents-core.config.ts"], + includePatterns: agentShards[5]?.includePatterns, + requiresDist: false, + runner: DEFAULT_NODE_TEST_RUNNER, + shardName: "agentic-agents-core-runner-commands", + }, + { + checkName: "checks-node-agentic-agents-core-runner-embedded", + configs: ["test/vitest/vitest.agents-core.config.ts"], + includePatterns: agentShards[6]?.includePatterns, + requiresDist: false, + runner: DEFAULT_NODE_TEST_RUNNER, + shardName: "agentic-agents-core-runner-embedded", + }, + { + checkName: "checks-node-agentic-agents-core-runner-sessions", + configs: ["test/vitest/vitest.agents-core.config.ts"], + includePatterns: agentShards[7]?.includePatterns, + requiresDist: false, + runner: DEFAULT_NODE_TEST_RUNNER, + shardName: "agentic-agents-core-runner-sessions", }, { checkName: "checks-node-agentic-agents-core-runtime", configs: ["test/vitest/vitest.agents-core.config.ts"], - includePatterns: agentShards[5]?.includePatterns, + includePatterns: agentShards[8]?.includePatterns, requiresDist: false, runner: DEFAULT_NODE_TEST_RUNNER, shardName: "agentic-agents-core-runtime", @@ -877,7 +898,7 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { { checkName: "checks-node-agentic-agents-core-isolated", configs: ["test/vitest/vitest.agents-core-isolated.config.ts"], - includePatterns: agentShards[6]?.includePatterns, + includePatterns: agentShards[9]?.includePatterns, requiresDist: false, runner: DEFAULT_NODE_TEST_RUNNER, shardName: "agentic-agents-core-isolated", @@ -1011,10 +1032,22 @@ describe("scripts/lib/ci-node-test-plan.mjs", () => { shardName: "auto-reply-reply-agent-runner", }, { - checkName: "checks-node-auto-reply-reply-commands", + checkName: "checks-node-auto-reply-reply-commands-1", configs: ["test/vitest/vitest.auto-reply-reply.config.ts"], requiresDist: false, - shardName: "auto-reply-reply-commands", + shardName: "auto-reply-reply-commands-1", + }, + { + checkName: "checks-node-auto-reply-reply-commands-2", + configs: ["test/vitest/vitest.auto-reply-reply.config.ts"], + requiresDist: false, + shardName: "auto-reply-reply-commands-2", + }, + { + checkName: "checks-node-auto-reply-reply-commands-3", + configs: ["test/vitest/vitest.auto-reply-reply.config.ts"], + requiresDist: false, + shardName: "auto-reply-reply-commands-3", }, { checkName: "checks-node-auto-reply-reply-dispatch", diff --git a/test/scripts/ci-run-node-test-shard.test.ts b/test/scripts/ci-run-node-test-shard.test.ts new file mode 100644 index 000000000000..28a6a16ed69c --- /dev/null +++ b/test/scripts/ci-run-node-test-shard.test.ts @@ -0,0 +1,176 @@ +// Covers the CI node test shard runner: plan resolution from job env and +// bounded-concurrency execution with per-child Vitest cache isolation. +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + buildChildEnv, + resolveShardPlans, + runShardPlans, +} from "../../scripts/ci-run-node-test-shard.mjs"; + +const scratchDirs: string[] = []; + +function makeScratchDir(): string { + const dir = mkdtempSync(path.join(tmpdir(), "openclaw-shard-test-")); + scratchDirs.push(dir); + return dir; +} + +afterEach(() => { + for (const dir of scratchDirs.splice(0)) { + rmSync(dir, { force: true, recursive: true }); + } +}); + +describe("scripts/ci-run-node-test-shard.mjs", () => { + it("prefers explicit targets and keeps one target per child", () => { + const plans = resolveShardPlans({ + OPENCLAW_NODE_TEST_TARGETS_JSON: JSON.stringify(["a.test.ts", "b.test.ts"]), + OPENCLAW_NODE_TEST_GROUPS_JSON: JSON.stringify([{ configs: ["c.config.ts"] }]), + }); + expect(plans).toEqual([ + { kind: "target", name: "a.test.ts", target: "a.test.ts" }, + { kind: "target", name: "b.test.ts", target: "b.test.ts" }, + ]); + }); + + it("falls back from groups to the single-shard matrix envelope", () => { + const groupPlans = resolveShardPlans({ + OPENCLAW_NODE_TEST_GROUPS_JSON: JSON.stringify([ + { configs: ["one.config.ts"], shard_name: "one" }, + { configs: ["two.config.ts"], shard_name: "two" }, + ]), + }); + expect(groupPlans.map((plan) => plan.name)).toEqual(["one", "two"]); + + const singlePlans = resolveShardPlans({ + OPENCLAW_NODE_TEST_CONFIGS_JSON: JSON.stringify(["solo.config.ts"]), + OPENCLAW_VITEST_SHARD_NAME: "solo", + }); + expect(singlePlans).toHaveLength(1); + expect(singlePlans[0]).toMatchObject({ kind: "group", name: "solo" }); + }); + + it("builds child env with per-plan cache isolation, includes, and env overlays", () => { + const scratchDir = makeScratchDir(); + const entry = { + kind: "group" as const, + name: "g", + plan: { + configs: ["cfg.ts"], + env: { EXTRA: "yes", IGNORED: 42 }, + includePatterns: ["src/a.test.ts"], + shard_name: "g", + }, + }; + const childEnv = buildChildEnv( + entry, + { BASE: "1", OPENCLAW_VITEST_INCLUDE_FILE: "stale.json" }, + scratchDir, + 3, + ); + expect(childEnv.BASE).toBe("1"); + expect(childEnv.EXTRA).toBe("yes"); + expect(childEnv.IGNORED).toBeUndefined(); + expect(childEnv.OPENCLAW_VITEST_SHARD_NAME).toBe("g"); + expect(childEnv.OPENCLAW_TEST_PROJECTS_PARALLEL).toBe("1"); + expect(childEnv.OPENCLAW_TEST_HEAVY_CHECK_LOCK_HELD).toBe("1"); + expect(childEnv.OPENCLAW_VITEST_FS_MODULE_CACHE_PATH).toBe( + path.join(scratchDir, "vitest-cache-3"), + ); + expect(childEnv.OPENCLAW_VITEST_INCLUDE_FILE).toBe( + path.join(scratchDir, "node-test-include-3.json"), + ); + expect(JSON.parse(readFileSync(childEnv.OPENCLAW_VITEST_INCLUDE_FILE ?? "", "utf8"))).toEqual([ + "src/a.test.ts", + ]); + + const bare = buildChildEnv( + { kind: "group" as const, name: "bare", plan: { configs: ["cfg.ts"] } }, + { OPENCLAW_VITEST_INCLUDE_FILE: "stale.json" }, + scratchDir, + 0, + ); + expect(bare.OPENCLAW_VITEST_INCLUDE_FILE).toBeUndefined(); + }); + + it("runs plans with bounded concurrency and distinct cache paths", async () => { + const scratchDir = makeScratchDir(); + const seen: Array<{ args: string[]; cache: string | undefined; label: string }> = []; + let active = 0; + let peakActive = 0; + const exitCode = await runShardPlans( + resolveShardPlans({ + OPENCLAW_NODE_TEST_GROUPS_JSON: JSON.stringify([ + { configs: ["a.config.ts"], shard_name: "a" }, + { configs: ["b.config.ts"], shard_name: "b" }, + { configs: ["c.config.ts"], shard_name: "c" }, + ]), + }), + { + concurrency: 2, + env: {}, + runChild: async ( + args: string[], + childEnv: Record, + label: string, + ) => { + active += 1; + peakActive = Math.max(peakActive, active); + await new Promise((resolve) => { + setTimeout(resolve, 10); + }); + seen.push({ args, cache: childEnv.OPENCLAW_VITEST_FS_MODULE_CACHE_PATH, label }); + active -= 1; + return 0; + }, + scratchDir, + }, + ); + expect(exitCode).toBe(0); + expect(peakActive).toBeLessThanOrEqual(2); + expect(seen.map((run) => run.label).toSorted()).toEqual(["a", "b", "c"]); + expect(new Set(seen.map((run) => run.cache)).size).toBe(3); + }); + + it("stops scheduling new plans after a failure and reports the first failing code", async () => { + const scratchDir = makeScratchDir(); + const started: string[] = []; + const exitCode = await runShardPlans( + resolveShardPlans({ + OPENCLAW_NODE_TEST_GROUPS_JSON: JSON.stringify([ + { configs: ["a.config.ts"], shard_name: "a" }, + { configs: ["b.config.ts"], shard_name: "b" }, + { configs: ["c.config.ts"], shard_name: "c" }, + { configs: ["d.config.ts"], shard_name: "d" }, + ]), + }), + { + concurrency: 1, + env: {}, + runChild: async ( + _args: string[], + _env: Record, + label: string, + ) => { + started.push(label); + return label === "b" ? 7 : 0; + }, + scratchDir, + }, + ); + expect(exitCode).toBe(7); + expect(started).toEqual(["a", "b"]); + }); + + it("fails plans that carry no configs", async () => { + const scratchDir = makeScratchDir(); + const exitCode = await runShardPlans( + [{ kind: "group" as const, name: "broken", plan: { configs: [] } }], + { concurrency: 1, env: {}, runChild: async () => 0, scratchDir }, + ); + expect(exitCode).toBe(1); + }); +}); diff --git a/test/scripts/ci-workflow-guards.test.ts b/test/scripts/ci-workflow-guards.test.ts index ecdcaa1dd285..ffc31920063e 100644 --- a/test/scripts/ci-workflow-guards.test.ts +++ b/test/scripts/ci-workflow-guards.test.ts @@ -2641,13 +2641,12 @@ describe("ci workflow guards", () => { expect(nodeTestJob["timeout-minutes"]).toBe("${{ matrix.timeout_minutes || 60 }}"); expect(runStep.env.OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS).toBe("300000"); expect(runStep.env.OPENCLAW_VITEST_NO_OUTPUT_RETRY).toBe("1"); - expect(runStep.env.OPENCLAW_TEST_PROJECTS_PARALLEL).toBe("2"); expect(runStep.env.OPENCLAW_NODE_TEST_ENV_JSON).toBe("${{ toJson(matrix.env) }}"); expect(runStep.env.OPENCLAW_NODE_TEST_TARGETS_JSON).toBe("${{ toJson(matrix.targets) }}"); - expect(runStep.run).toContain("env: JSON.parse(process.env.OPENCLAW_NODE_TEST_ENV_JSON"); - expect(runStep.run).toContain('["exec", "node", "scripts/test-projects.mjs", target]'); - expect(runStep.run).toContain('if (plan.env && typeof plan.env === "object"'); - expect(runStep.run).toContain("childEnv[key] = value"); + // Shard execution policy lives in the unit-tested wrapper script, not in + // inline workflow JavaScript. + expect(runStep.run).toBe("node scripts/ci-run-node-test-shard.mjs"); + expect(existsSync("scripts/ci-run-node-test-shard.mjs")).toBe(true); }); it("keeps the CI timing summary parked for timing optimization work", () => {