From bff340c1ca65306aea97d61c8f47b00de97b2ace Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Fri, 13 Mar 2026 18:36:38 -0500 Subject: [PATCH] test: preserve wrapper behavior for targeted runs FIX OOM issues(#45518) * test: preserve wrapper behavior for targeted runs * test: tighten targeted wrapper routing --- AGENTS.md | 1 + scripts/test-parallel.mjs | 344 ++++++++++++++++++++++++++++++++------ 2 files changed, 295 insertions(+), 50 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5112a8241df..32e706997cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -132,6 +132,7 @@ - Framework: Vitest with V8 coverage thresholds (70% lines/branches/functions/statements). - Naming: match source names with `*.test.ts`; e2e in `*.e2e.test.ts`. - Run `pnpm test` (or `pnpm test:coverage`) before pushing when you touch logic. +- For targeted/local debugging, keep using the wrapper: `pnpm test -- [vitest args...]` (for example `pnpm test -- src/commands/onboard-search.test.ts -t "shows registered plugin providers"`); do not default to raw `pnpm vitest run ...` because it bypasses wrapper config/profile/pool routing. - Do not set test workers above 16; tried already. - If local Vitest runs cause memory pressure (common on non-Mac-Studio hosts), use `OPENCLAW_TEST_PROFILE=low OPENCLAW_TEST_SERIAL_GATEWAY=1 pnpm test` for land/gate runs. - Live tests (real keys): `CLAWDBOT_LIVE_TEST=1 pnpm test:live` (OpenClaw-only) or `LIVE=1 pnpm test:live` (includes provider live tests). Docker: `pnpm test:docker:live-models`, `pnpm test:docker:live-gateway`. Onboarding Docker E2E: `pnpm test:docker:onboard`. diff --git a/scripts/test-parallel.mjs b/scripts/test-parallel.mjs index 1716f724bff..021ff1f905e 100644 --- a/scripts/test-parallel.mjs +++ b/scripts/test-parallel.mjs @@ -1,6 +1,7 @@ import { spawn } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; +import path from "node:path"; // On Windows, `.cmd` launchers can fail with `spawn EINVAL` when invoked without a shell // (especially under GitHub Actions + Git Bash). Use `shell: true` and let the shell resolve pnpm. @@ -205,6 +206,45 @@ const shardIndexOverride = (() => { const parsed = Number.parseInt(process.env.OPENCLAW_TEST_SHARD_INDEX ?? "", 10); return Number.isFinite(parsed) && parsed > 0 ? parsed : null; })(); +const OPTION_TAKES_VALUE = new Set([ + "-t", + "-c", + "-r", + "--testNamePattern", + "--config", + "--root", + "--dir", + "--reporter", + "--outputFile", + "--pool", + "--execArgv", + "--vmMemoryLimit", + "--maxWorkers", + "--environment", + "--shard", + "--changed", + "--sequence", + "--inspect", + "--inspectBrk", + "--testTimeout", + "--hookTimeout", + "--bail", + "--retry", + "--diff", + "--exclude", + "--project", + "--slowTestThreshold", + "--teardownTimeout", + "--attachmentsDir", + "--mode", + "--api", + "--browser", + "--maxConcurrency", + "--mergeReports", + "--configLoader", + "--experimental", +]); +const SINGLE_RUN_ONLY_FLAGS = new Set(["--coverage", "--outputFile", "--mergeReports"]); if (shardIndexOverride !== null && shardCount <= 1) { console.error( @@ -229,6 +269,219 @@ const silentArgs = const rawPassthroughArgs = process.argv.slice(2); const passthroughArgs = rawPassthroughArgs[0] === "--" ? rawPassthroughArgs.slice(1) : rawPassthroughArgs; +const parsePassthroughArgs = (args) => { + const fileFilters = []; + const optionArgs = []; + let consumeNextAsOptionValue = false; + + for (const arg of args) { + if (consumeNextAsOptionValue) { + optionArgs.push(arg); + consumeNextAsOptionValue = false; + continue; + } + if (arg === "--") { + optionArgs.push(arg); + continue; + } + if (arg.startsWith("-")) { + optionArgs.push(arg); + consumeNextAsOptionValue = !arg.includes("=") && OPTION_TAKES_VALUE.has(arg); + continue; + } + fileFilters.push(arg); + } + + return { fileFilters, optionArgs }; +}; +const { fileFilters: passthroughFileFilters, optionArgs: passthroughOptionArgs } = + parsePassthroughArgs(passthroughArgs); +const passthroughRequiresSingleRun = passthroughOptionArgs.some((arg) => { + if (!arg.startsWith("-")) { + return false; + } + const [flag] = arg.split("=", 1); + return SINGLE_RUN_ONLY_FLAGS.has(flag); +}); +const channelPrefixes = ["src/telegram/", "src/discord/", "src/web/", "src/browser/", "src/line/"]; +const baseConfigPrefixes = ["src/agents/", "src/auto-reply/", "src/commands/", "test/", "ui/"]; +const normalizeRepoPath = (value) => value.split(path.sep).join("/"); +const walkTestFiles = (rootDir) => { + if (!fs.existsSync(rootDir)) { + return []; + } + const entries = fs.readdirSync(rootDir, { withFileTypes: true }); + const files = []; + for (const entry of entries) { + const fullPath = path.join(rootDir, entry.name); + if (entry.isDirectory()) { + files.push(...walkTestFiles(fullPath)); + continue; + } + if (!entry.isFile()) { + continue; + } + if ( + fullPath.endsWith(".test.ts") || + fullPath.endsWith(".live.test.ts") || + fullPath.endsWith(".e2e.test.ts") + ) { + files.push(normalizeRepoPath(fullPath)); + } + } + return files; +}; +const allKnownTestFiles = [ + ...new Set([ + ...walkTestFiles("src"), + ...walkTestFiles("extensions"), + ...walkTestFiles("test"), + ...walkTestFiles(path.join("ui", "src", "ui")), + ]), +]; +const inferTarget = (fileFilter) => { + const isolated = unitIsolatedFiles.includes(fileFilter); + if (fileFilter.endsWith(".live.test.ts")) { + return { owner: "live", isolated }; + } + if (fileFilter.endsWith(".e2e.test.ts")) { + return { owner: "e2e", isolated }; + } + if (fileFilter.startsWith("extensions/")) { + return { owner: "extensions", isolated }; + } + if (fileFilter.startsWith("src/gateway/")) { + return { owner: "gateway", isolated }; + } + if (channelPrefixes.some((prefix) => fileFilter.startsWith(prefix))) { + return { owner: "channels", isolated }; + } + if (baseConfigPrefixes.some((prefix) => fileFilter.startsWith(prefix))) { + return { owner: "base", isolated }; + } + if (fileFilter.startsWith("src/")) { + return { owner: "unit", isolated }; + } + return { owner: "base", isolated }; +}; +const resolveFilterMatches = (fileFilter) => { + const normalizedFilter = normalizeRepoPath(fileFilter); + if (fs.existsSync(fileFilter)) { + const stats = fs.statSync(fileFilter); + if (stats.isFile()) { + return [normalizedFilter]; + } + if (stats.isDirectory()) { + const prefix = normalizedFilter.endsWith("/") ? normalizedFilter : `${normalizedFilter}/`; + return allKnownTestFiles.filter((file) => file.startsWith(prefix)); + } + } + if (/[*?[\]{}]/.test(normalizedFilter)) { + return allKnownTestFiles.filter((file) => path.matchesGlob(file, normalizedFilter)); + } + return allKnownTestFiles.filter((file) => file.includes(normalizedFilter)); +}; +const createTargetedEntry = (owner, isolated, filters) => { + const name = isolated ? `${owner}-isolated` : owner; + const forceForks = isolated; + if (owner === "unit") { + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.unit.config.ts", + `--pool=${forceForks ? "forks" : useVmForks ? "vmForks" : "forks"}`, + ...(disableIsolation ? ["--isolate=false"] : []), + ...filters, + ], + }; + } + if (owner === "extensions") { + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.extensions.config.ts", + ...(forceForks ? ["--pool=forks"] : useVmForks ? ["--pool=vmForks"] : []), + ...filters, + ], + }; + } + if (owner === "gateway") { + return { + name, + args: ["vitest", "run", "--config", "vitest.gateway.config.ts", "--pool=forks", ...filters], + }; + } + if (owner === "channels") { + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.channels.config.ts", + ...(forceForks ? ["--pool=forks"] : []), + ...filters, + ], + }; + } + if (owner === "live") { + return { + name, + args: ["vitest", "run", "--config", "vitest.live.config.ts", ...filters], + }; + } + if (owner === "e2e") { + return { + name, + args: ["vitest", "run", "--config", "vitest.e2e.config.ts", ...filters], + }; + } + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.config.ts", + ...(forceForks ? ["--pool=forks"] : []), + ...filters, + ], + }; +}; +const targetedEntries = (() => { + if (passthroughFileFilters.length === 0) { + return []; + } + const groups = passthroughFileFilters.reduce((acc, fileFilter) => { + const matchedFiles = resolveFilterMatches(fileFilter); + if (matchedFiles.length === 0) { + const target = inferTarget(normalizeRepoPath(fileFilter)); + const key = `${target.owner}:${target.isolated ? "isolated" : "default"}`; + const files = acc.get(key) ?? []; + files.push(normalizeRepoPath(fileFilter)); + acc.set(key, files); + return acc; + } + for (const matchedFile of matchedFiles) { + const target = inferTarget(matchedFile); + const key = `${target.owner}:${target.isolated ? "isolated" : "default"}`; + const files = acc.get(key) ?? []; + files.push(matchedFile); + acc.set(key, files); + } + return acc; + }, new Map()); + return Array.from(groups, ([key, filters]) => { + const [owner, mode] = key.split(":"); + return createTargetedEntry(owner, mode === "isolated", [...new Set(filters)]); + }); +})(); const topLevelParallelEnabled = testProfile !== "low" && testProfile !== "serial"; const overrideWorkers = Number.parseInt(process.env.OPENCLAW_TEST_WORKERS ?? "", 10); const resolvedOverride = @@ -311,7 +564,7 @@ const maxWorkersForRun = (name) => { if (isCI && isMacOS) { return 1; } - if (name === "unit-isolated") { + if (name === "unit-isolated" || name.endsWith("-isolated")) { return defaultWorkerBudget.unitIsolated; } if (name === "extensions") { @@ -397,16 +650,16 @@ const runOnce = (entry, extraArgs = []) => }); }); -const run = async (entry) => { +const run = async (entry, extraArgs = []) => { if (shardCount <= 1) { - return runOnce(entry); + return runOnce(entry, extraArgs); } if (shardIndexOverride !== null) { - return runOnce(entry, ["--shard", `${shardIndexOverride}/${shardCount}`]); + return runOnce(entry, ["--shard", `${shardIndexOverride}/${shardCount}`, ...extraArgs]); } for (let shardIndex = 1; shardIndex <= shardCount; shardIndex += 1) { // eslint-disable-next-line no-await-in-loop - const code = await runOnce(entry, ["--shard", `${shardIndex}/${shardCount}`]); + const code = await runOnce(entry, ["--shard", `${shardIndex}/${shardCount}`, ...extraArgs]); if (code !== 0) { return code; } @@ -414,15 +667,15 @@ const run = async (entry) => { return 0; }; -const runEntries = async (entries) => { +const runEntries = async (entries, extraArgs = []) => { if (topLevelParallelEnabled) { - const codes = await Promise.all(entries.map(run)); + const codes = await Promise.all(entries.map((entry) => run(entry, extraArgs))); return codes.find((code) => code !== 0); } for (const entry of entries) { // eslint-disable-next-line no-await-in-loop - const code = await run(entry); + const code = await run(entry, extraArgs); if (code !== 0) { return code; } @@ -440,57 +693,48 @@ const shutdown = (signal) => { process.on("SIGINT", () => shutdown("SIGINT")); process.on("SIGTERM", () => shutdown("SIGTERM")); -if (passthroughArgs.length > 0) { - const maxWorkers = maxWorkersForRun("unit"); - const args = maxWorkers - ? [ - "vitest", - "run", - "--maxWorkers", - String(maxWorkers), - ...silentArgs, - ...windowsCiArgs, - ...passthroughArgs, - ] - : ["vitest", "run", ...silentArgs, ...windowsCiArgs, ...passthroughArgs]; - const nodeOptions = process.env.NODE_OPTIONS ?? ""; - const nextNodeOptions = WARNING_SUPPRESSION_FLAGS.reduce( - (acc, flag) => (acc.includes(flag) ? acc : `${acc} ${flag}`.trim()), - nodeOptions, - ); - const code = await new Promise((resolve) => { - let child; - try { - child = spawn(pnpm, args, { - stdio: "inherit", - env: { ...process.env, NODE_OPTIONS: nextNodeOptions }, - shell: isWindows, - }); - } catch (err) { - console.error(`[test-parallel] spawn failed: ${String(err)}`); - resolve(1); - return; +if (targetedEntries.length > 0) { + if (passthroughRequiresSingleRun && targetedEntries.length > 1) { + console.error( + "[test-parallel] The provided Vitest args require a single run, but the selected test filters span multiple wrapper configs. Run one target/config at a time.", + ); + process.exit(2); + } + const targetedParallelRuns = keepGatewaySerial + ? targetedEntries.filter((entry) => entry.name !== "gateway") + : targetedEntries; + const targetedSerialRuns = keepGatewaySerial + ? targetedEntries.filter((entry) => entry.name === "gateway") + : []; + const failedTargetedParallel = await runEntries(targetedParallelRuns, passthroughOptionArgs); + if (failedTargetedParallel !== undefined) { + process.exit(failedTargetedParallel); + } + for (const entry of targetedSerialRuns) { + // eslint-disable-next-line no-await-in-loop + const code = await run(entry, passthroughOptionArgs); + if (code !== 0) { + process.exit(code); } - children.add(child); - child.on("error", (err) => { - console.error(`[test-parallel] child error: ${String(err)}`); - }); - child.on("exit", (exitCode, signal) => { - children.delete(child); - resolve(exitCode ?? (signal ? 1 : 0)); - }); - }); - process.exit(Number(code) || 0); + } + process.exit(0); } -const failedParallel = await runEntries(parallelRuns); +if (passthroughRequiresSingleRun && passthroughOptionArgs.length > 0) { + console.error( + "[test-parallel] The provided Vitest args require a single run. Use the dedicated npm script for that workflow (for example `pnpm test:coverage`) or target a single test file/filter.", + ); + process.exit(2); +} + +const failedParallel = await runEntries(parallelRuns, passthroughOptionArgs); if (failedParallel !== undefined) { process.exit(failedParallel); } for (const entry of serialRuns) { // eslint-disable-next-line no-await-in-loop - const code = await run(entry); + const code = await run(entry, passthroughOptionArgs); if (code !== 0) { process.exit(code); }