From 97e79bb5f666a86d8df38f84411dba69791c4ba7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 20 Apr 2026 17:41:21 +0100 Subject: [PATCH] test: balance extension shard scheduling --- AGENTS.md | 1 + docs/reference/test.md | 1 + scripts/test-projects.mjs | 134 +++++++++++++++++++++++-- scripts/test-projects.test-support.mjs | 30 +++++- test/scripts/test-projects.test.ts | 15 +++ 5 files changed, 169 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f6a03f44a04..c4e606dae9c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,7 @@ Scoped guides: - Full tests: `pnpm test` - Changed tests only: `pnpm test:changed` - Extension tests: `pnpm test:extensions` or `pnpm test extensions` = all extension shards; `pnpm test extensions/` = one extension lane. Heavy channels/OpenAI have dedicated shards. +- Shard timing artifact: `.artifacts/vitest-shard-timings.json`; auto-used for balanced shard ordering. Disable with `OPENCLAW_TEST_PROJECTS_TIMINGS=0`. - Targeted tests: `pnpm test [vitest args...]`; do not call raw `vitest`. - Coverage: `pnpm test:coverage` - Format check/fix: `pnpm format:check` / `pnpm format` diff --git a/docs/reference/test.md b/docs/reference/test.md index 8df68c4109e..2752ddf0b4d 100644 --- a/docs/reference/test.md +++ b/docs/reference/test.md @@ -16,6 +16,7 @@ title: "Tests" - `pnpm changed:lanes`: shows the architectural lanes triggered by the diff against `origin/main`. - `pnpm check:changed`: runs the smart changed gate for the diff against `origin/main`. It runs core work with core test lanes, extension work with extension test lanes, test-only work with test typecheck/tests only, and expands public Plugin SDK or plugin-contract changes to extension validation. - `pnpm test`: routes explicit file/directory targets through scoped Vitest lanes. Untargeted runs use fixed shard groups and expand to leaf configs for local parallel execution; the extension group always expands to the per-extension shard configs instead of one giant root-project process. +- Full and extension shard runs update local timing data in `.artifacts/vitest-shard-timings.json`; later runs use those timings to balance slow and fast shards. Set `OPENCLAW_TEST_PROJECTS_TIMINGS=0` to ignore the local timing artifact. - Selected `plugin-sdk` and `commands` test files now route through dedicated light lanes that keep only `test/setup.ts`, leaving runtime-heavy cases on their existing lanes. - Selected `plugin-sdk` and `commands` helper source files also map `pnpm test:changed` to explicit sibling tests in those light lanes, so small helper edits avoid rerunning the heavy runtime-backed suites. - `auto-reply` now also splits into three dedicated configs (`core`, `top-level`, `reply`) so the reply harness does not dominate the lighter top-level status/token/helper tests. diff --git a/scripts/test-projects.mjs b/scripts/test-projects.mjs index 00864fccfea..c9893d4d000 100644 --- a/scripts/test-projects.mjs +++ b/scripts/test-projects.mjs @@ -1,4 +1,6 @@ import fs from "node:fs"; +import path from "node:path"; +import { performance } from "node:perf_hooks"; import { acquireLocalHeavyCheckLockSync } from "./lib/local-heavy-check-runtime.mjs"; import { isCiLikeEnv, resolveLocalFullSuiteProfile } from "./lib/vitest-local-scheduling.mjs"; import { @@ -81,6 +83,8 @@ const FULL_SUITE_CONFIG_WEIGHT = new Map([ ["test/vitest/vitest.extension-memory.config.ts", 6], ["test/vitest/vitest.extension-msteams.config.ts", 4], ]); +const TIMINGS_FILE_ENV_KEY = "OPENCLAW_TEST_PROJECTS_TIMINGS_PATH"; +const TIMINGS_DISABLE_ENV_KEY = "OPENCLAW_TEST_PROJECTS_TIMINGS"; const releaseLockOnce = () => { if (lockReleased) { return; @@ -89,6 +93,81 @@ const releaseLockOnce = () => { releaseLock(); }; +function shouldUseShardTimings(env = process.env) { + return env[TIMINGS_DISABLE_ENV_KEY] !== "0"; +} + +function resolveShardTimingsPath(cwd = process.cwd(), env = process.env) { + return env[TIMINGS_FILE_ENV_KEY] || path.join(cwd, ".artifacts", "vitest-shard-timings.json"); +} + +function readShardTimings(cwd = process.cwd(), env = process.env) { + if (!shouldUseShardTimings(env)) { + return new Map(); + } + try { + const raw = fs.readFileSync(resolveShardTimingsPath(cwd, env), "utf8"); + const parsed = JSON.parse(raw); + const configs = parsed && typeof parsed === "object" ? parsed.configs : null; + if (!configs || typeof configs !== "object") { + return new Map(); + } + return new Map( + Object.entries(configs) + .map(([config, value]) => { + const durationMs = Number(value?.averageMs ?? value?.durationMs); + return Number.isFinite(durationMs) && durationMs > 0 ? [config, durationMs] : null; + }) + .filter(Boolean), + ); + } catch { + return new Map(); + } +} + +function writeShardTimings(samples, cwd = process.cwd(), env = process.env) { + if (!shouldUseShardTimings(env) || samples.length === 0) { + return; + } + + const outputPath = resolveShardTimingsPath(cwd, env); + let current = { version: 1, configs: {} }; + try { + current = JSON.parse(fs.readFileSync(outputPath, "utf8")); + } catch { + // First run, or a corrupt local artifact. Rewrite below. + } + + const configs = + current && typeof current === "object" && current.configs && typeof current.configs === "object" + ? { ...current.configs } + : {}; + const updatedAt = new Date().toISOString(); + for (const sample of samples) { + if (!sample.config || !Number.isFinite(sample.durationMs) || sample.durationMs <= 0) { + continue; + } + const previous = configs[sample.config]; + const previousAverage = Number(previous?.averageMs ?? previous?.durationMs); + const sampleCount = Math.max(0, Number(previous?.sampleCount) || 0) + 1; + const averageMs = + Number.isFinite(previousAverage) && previousAverage > 0 + ? Math.round(previousAverage * 0.7 + sample.durationMs * 0.3) + : Math.round(sample.durationMs); + configs[sample.config] = { + averageMs, + lastMs: Math.round(sample.durationMs), + sampleCount, + updatedAt, + }; + } + + fs.mkdirSync(path.dirname(outputPath), { recursive: true }); + const tempPath = `${outputPath}.${process.pid}.tmp`; + fs.writeFileSync(tempPath, `${JSON.stringify({ version: 1, configs }, null, 2)}\n`, "utf8"); + fs.renameSync(tempPath, outputPath); +} + function cleanupVitestRunSpec(spec) { if (!spec.includeFilePath) { return; @@ -145,25 +224,52 @@ function applyDefaultParallelVitestWorkerBudget(specs, env) { async function runLoggedVitestSpec(spec) { console.error(`[test] starting ${spec.config}`); + const startedAt = performance.now(); const result = await runVitestSpec(spec); + const durationMs = performance.now() - startedAt; if (result.signal) { console.error(`[test] ${spec.config} exited by signal ${result.signal}`); releaseLockOnce(); process.kill(process.pid, result.signal); return null; } - return result; + return { + ...result, + timing: + !spec.watchMode && spec.includePatterns === null ? { config: spec.config, durationMs } : null, + }; } -function orderFullSuiteSpecsForParallelRun(specs) { - return specs.toSorted((a, b) => { +function resolveConfigSortWeight(config, shardTimings) { + return shardTimings.get(config) ?? (FULL_SUITE_CONFIG_WEIGHT.get(config) ?? 0) * 1000; +} + +function interleaveSlowAndFastSpecs(sortedSpecs) { + const ordered = []; + let slowIndex = 0; + let fastIndex = sortedSpecs.length - 1; + while (slowIndex <= fastIndex) { + ordered.push(sortedSpecs[slowIndex]); + slowIndex += 1; + if (slowIndex <= fastIndex) { + ordered.push(sortedSpecs[fastIndex]); + fastIndex -= 1; + } + } + return ordered; +} + +function orderFullSuiteSpecsForParallelRun(specs, shardTimings = new Map()) { + const sortedSpecs = specs.toSorted((a, b) => { const weightDelta = - (FULL_SUITE_CONFIG_WEIGHT.get(b.config) ?? 0) - (FULL_SUITE_CONFIG_WEIGHT.get(a.config) ?? 0); + resolveConfigSortWeight(b.config, shardTimings) - + resolveConfigSortWeight(a.config, shardTimings); if (weightDelta !== 0) { return weightDelta; } return a.config.localeCompare(b.config); }); + return shardTimings.size > 0 ? interleaveSlowAndFastSpecs(sortedSpecs) : sortedSpecs; } function isFullExtensionsProjectRun(specs) { @@ -182,6 +288,7 @@ function isFullExtensionsProjectRun(specs) { async function runVitestSpecsParallel(specs, concurrency) { let nextIndex = 0; let exitCode = 0; + const timings = []; const runWorker = async () => { for (;;) { @@ -198,11 +305,14 @@ async function runVitestSpecsParallel(specs, concurrency) { if (result.code !== 0) { exitCode = exitCode || result.code; } + if (result.timing) { + timings.push(result.timing); + } } }; await Promise.all(Array.from({ length: concurrency }, () => runWorker())); - return exitCode; + return { exitCode, timings }; } async function main() { @@ -257,8 +367,9 @@ async function main() { const concurrency = resolveParallelFullSuiteConcurrency(runSpecs.length, process.env); if (concurrency > 1) { const localFullSuiteProfile = resolveLocalFullSuiteProfile(process.env); + const shardTimings = readShardTimings(process.cwd(), process.env); const parallelSpecs = applyDefaultParallelVitestWorkerBudget( - applyParallelVitestCachePaths(orderFullSuiteSpecsForParallelRun(runSpecs), { + applyParallelVitestCachePaths(orderFullSuiteSpecsForParallelRun(runSpecs, shardTimings), { cwd: process.cwd(), env: process.env, }), @@ -277,7 +388,11 @@ async function main() { console.error( `[test] running ${parallelSpecs.length} Vitest shards with parallelism ${concurrency}`, ); - const parallelExitCode = await runVitestSpecsParallel(parallelSpecs, concurrency); + const { exitCode: parallelExitCode, timings } = await runVitestSpecsParallel( + parallelSpecs, + concurrency, + ); + writeShardTimings(timings, process.cwd(), process.env); console.error( `[test] completed ${parallelSpecs.length} Vitest shards; Vitest summaries above are per-shard, not aggregate totals.`, ); @@ -290,6 +405,7 @@ async function main() { } let exitCode = 0; + const timings = []; for (const spec of runSpecs) { const result = await runLoggedVitestSpec(spec); if (!result) { @@ -302,7 +418,11 @@ async function main() { process.exit(result.code); } } + if (result.timing) { + timings.push(result.timing); + } } + writeShardTimings(timings, process.cwd(), process.env); releaseLockOnce(); if (exitCode !== 0) { diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 7173a2aa9fd..4c6e97828e1 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -166,11 +166,14 @@ const BROAD_CHANGED_RERUN_PATTERNS = [ /^pnpm-lock\.yaml$/u, /^test\/setup(?:\.shared|\.extensions|-openclaw-runtime)?\.ts$/u, /^vitest(?:\..+)?\.(?:config\.ts|paths\.mjs)$/u, - /^test\/vitest\/vitest(?:\..+)?\.(?:config\.ts|paths\.mjs)$/u, + /^test\/vitest\/vitest\.(?:config|shared\.config|scoped-config|performance-config)\.ts$/u, /^test\/helpers\//u, /^scripts\/run-vitest\.mjs$/u, /^scripts\/test-projects(?:\.test-support)?\.mjs$/u, ]; +const VITEST_CONFIG_TARGET_KIND_BY_PATH = new Map( + Object.entries(VITEST_CONFIG_BY_KIND).map(([kind, config]) => [config, kind]), +); function normalizePathPattern(value) { return value.replaceAll("\\", "/"); @@ -227,6 +230,14 @@ function toScopedIncludePattern(arg, cwd) { return `${relative.replace(/\/+$/u, "")}/**/*.test.ts`; } +function resolveVitestConfigTargetKind(relative) { + return VITEST_CONFIG_TARGET_KIND_BY_PATH.get(relative) ?? null; +} + +function isVitestConfigTargetForKind(kind, targetArg, cwd) { + return resolveVitestConfigTargetKind(toRepoRelativeTarget(targetArg, cwd)) === kind; +} + function listChangedPathsFromGit(baseRef, cwd) { return execFileSync("git", ["diff", "--name-only", `${baseRef}...HEAD`], { cwd, @@ -317,6 +328,10 @@ export function resolveChangedTargetArgs( function classifyTarget(arg, cwd) { const relative = toRepoRelativeTarget(arg, cwd); + const configTargetKind = resolveVitestConfigTargetKind(relative); + if (configTargetKind) { + return configTargetKind; + } if (resolveUnitFastTestIncludePattern(relative)) { return "unitFast"; } @@ -667,12 +682,17 @@ export function buildVitestRunPlans( kind === "e2e" || (kind === "default" && grouped.every((targetArg) => isFileLikeTarget(toRepoRelativeTarget(targetArg, cwd)))); + const useWholeConfigTarget = grouped.some((targetArg) => + isVitestConfigTargetForKind(kind, targetArg, cwd), + ); const includePatterns = useCliTargetArgs ? null - : grouped.flatMap((targetArg) => { - const lightLanePatterns = resolveLightLaneIncludePatterns(kind, targetArg, cwd); - return lightLanePatterns ?? [toScopedIncludePattern(targetArg, cwd)]; - }); + : useWholeConfigTarget + ? null + : grouped.flatMap((targetArg) => { + const lightLanePatterns = resolveLightLaneIncludePatterns(kind, targetArg, cwd); + return lightLanePatterns ?? [toScopedIncludePattern(targetArg, cwd)]; + }); const scopedTargetArgs = useCliTargetArgs ? grouped : []; plans.push({ config, diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index d529780b5d5..2c0fef40c48 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -29,6 +29,21 @@ describe("scripts/test-projects changed-target routing", () => { ).toBeNull(); }); + it("routes changed extension vitest configs to their own shard", () => { + expect( + buildVitestRunPlans(["--changed", "origin/main"], process.cwd(), () => [ + "test/vitest/vitest.extension-discord.config.ts", + ]), + ).toEqual([ + { + config: "test/vitest/vitest.extension-discord.config.ts", + forwardedArgs: [], + includePatterns: null, + watchMode: false, + }, + ]); + }); + it("keeps the broad changed run for shared test helpers", () => { expect( resolveChangedTargetArgs(["--changed", "origin/main"], process.cwd(), () => [