From e2f2deae78c1d8a7f25571accb45bc9efe83e38f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 23 Apr 2026 19:40:16 +0100 Subject: [PATCH] fix: fail fast on silent changed-test hangs --- scripts/check-changed.mjs | 31 +++++++++++++- scripts/test-projects.mjs | 3 +- scripts/test-projects.test-support.mjs | 6 +++ test/scripts/changed-lanes.test.ts | 58 +++++++++++++++++++++----- test/scripts/test-projects.test.ts | 10 +++++ 5 files changed, 94 insertions(+), 14 deletions(-) diff --git a/scripts/check-changed.mjs b/scripts/check-changed.mjs index d855babe5b8..230ea9ec8dd 100644 --- a/scripts/check-changed.mjs +++ b/scripts/check-changed.mjs @@ -10,6 +10,20 @@ import { printTimingSummary } from "./lib/check-timing-summary.mjs"; import { runManagedCommand } from "./lib/managed-child-process.mjs"; import { resolveChangedTestTargetPlan } from "./test-projects.test-support.mjs"; +export const CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS = "60000"; +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 function createChangedCheckVitestEnv(baseEnv = process.env) { + return { + ...baseEnv, + [VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY]: + baseEnv[VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY]?.trim() || + CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS, + [VITEST_NO_OUTPUT_RETRY_ENV_KEY]: baseEnv[VITEST_NO_OUTPUT_RETRY_ENV_KEY]?.trim() || "0", + }; +} + export function createChangedCheckPlan(result, options = {}) { const commands = []; const add = (name, args) => { @@ -138,7 +152,10 @@ export async function runChangedCheck(result, options = {}) { } if (plan.runFullTests) { - const status = await runPnpm({ name: "tests all", args: ["test"] }, timings); + const status = await runPnpm( + { name: "tests all", args: ["test"], env: createChangedCheckVitestEnv() }, + timings, + ); if (status !== 0) { printSummary(timings, options); return status; @@ -151,6 +168,7 @@ export async function runChangedCheck(result, options = {}) { { name: options.explicitPaths ? "tests all" : "tests changed broad", args: testArgs, + env: createChangedCheckVitestEnv(), }, timings, ); @@ -163,6 +181,7 @@ export async function runChangedCheck(result, options = {}) { { name: "tests changed", args: ["test", ...plan.testTargets], + env: createChangedCheckVitestEnv(), }, timings, ); @@ -173,7 +192,14 @@ export async function runChangedCheck(result, options = {}) { } if (plan.runExtensionTests) { - const status = await runPnpm({ name: "tests extensions", args: ["test:extensions"] }, timings); + const status = await runPnpm( + { + name: "tests extensions", + args: ["test:extensions"], + env: createChangedCheckVitestEnv(), + }, + timings, + ); if (status !== 0) { printSummary(timings, options); return status; @@ -217,6 +243,7 @@ async function runCommand(command, timings) { status = await runManagedCommand({ bin: command.bin, args: command.args, + env: command.env, }); } catch (error) { console.error(error); diff --git a/scripts/test-projects.mjs b/scripts/test-projects.mjs index 459166cd8de..bfa58a763a7 100644 --- a/scripts/test-projects.mjs +++ b/scripts/test-projects.mjs @@ -20,6 +20,7 @@ import { resolveParallelFullSuiteConcurrency, resolveChangedTargetArgs, shouldAcquireLocalHeavyCheckLock, + shouldRetryVitestNoOutputTimeout, writeVitestIncludeFile, } from "./test-projects.test-support.mjs"; @@ -236,7 +237,7 @@ async function runLoggedVitestSpec(spec) { console.error(`[test] starting ${spec.config}`); const startedAt = performance.now(); let result = await runVitestSpec(spec); - if (result.noOutputTimedOut && !spec.watchMode) { + if (result.noOutputTimedOut && !spec.watchMode && shouldRetryVitestNoOutputTimeout(spec.env)) { console.error(`[test] retrying ${spec.config} after no-output timeout`); result = await runVitestSpec(spec); } diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 3449ed27c77..6227b5aa898 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -243,6 +243,7 @@ const GENERATED_CHANGED_TEST_TARGETS = new Set([ "src/canvas-host/a2ui/a2ui.bundle.js", ]); 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"; const VITEST_CONFIG_TARGET_KIND_BY_PATH = new Map( Object.entries(VITEST_CONFIG_BY_KIND).map(([kind, config]) => [config, kind]), @@ -1098,6 +1099,11 @@ export function applyDefaultVitestNoOutputTimeout(specs, params = {}) { }); } +export function shouldRetryVitestNoOutputTimeout(env = process.env) { + const value = env[VITEST_NO_OUTPUT_RETRY_ENV_KEY]?.trim().toLowerCase(); + return !["0", "false", "no", "off"].includes(value ?? ""); +} + export function createVitestRunSpecs(args, params = {}) { const cwd = params.cwd ?? process.cwd(); const baseEnv = params.baseEnv ?? process.env; diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index af2eb9aac8b..9c68bce392d 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -3,21 +3,41 @@ import { mkdirSync, writeFileSync } from "node:fs"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { detectChangedLanes } from "../../scripts/changed-lanes.mjs"; -import { createChangedCheckPlan } from "../../scripts/check-changed.mjs"; +import { + CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS, + createChangedCheckPlan, + createChangedCheckVitestEnv, +} from "../../scripts/check-changed.mjs"; import { cleanupTempDirs, makeTempRepoRoot } from "../helpers/temp-repo.js"; const tempDirs: string[] = []; const repoRoot = process.cwd(); +const nestedGitEnvKeys = [ + "GIT_ALTERNATE_OBJECT_DIRECTORIES", + "GIT_DIR", + "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", + "GIT_QUARANTINE_PATH", + "GIT_WORK_TREE", +] as const; + +function createNestedGitEnv(): NodeJS.ProcessEnv { + const env = { + ...process.env, + GIT_CONFIG_NOSYSTEM: "1", + GIT_TERMINAL_PROMPT: "0", + }; + for (const key of nestedGitEnvKeys) { + delete env[key]; + } + return env; +} const git = (cwd: string, args: string[]) => execFileSync("git", args, { cwd, encoding: "utf8", - env: { - ...process.env, - GIT_CONFIG_NOSYSTEM: "1", - GIT_TERMINAL_PROMPT: "0", - }, + env: createNestedGitEnv(), }).trim(); afterEach(() => { @@ -50,11 +70,7 @@ describe("scripts/changed-lanes", () => { { cwd: dir, encoding: "utf8", - env: { - ...process.env, - GIT_CONFIG_NOSYSTEM: "1", - GIT_TERMINAL_PROMPT: "0", - }, + env: createNestedGitEnv(), }, ); @@ -238,6 +254,7 @@ describe("scripts/changed-lanes", () => { [path.join(repoRoot, "scripts", "check-release-metadata-only.mjs"), "--staged"], { cwd: dir, + env: createNestedGitEnv(), stdio: "pipe", }, ), @@ -255,6 +272,7 @@ describe("scripts/changed-lanes", () => { [path.join(repoRoot, "scripts", "check-release-metadata-only.mjs"), "--staged"], { cwd: dir, + env: createNestedGitEnv(), stdio: "pipe", }, ), @@ -344,4 +362,22 @@ describe("scripts/changed-lanes", () => { expect(plan.runChangedTestsBroad).toBe(false); expect(plan.runFullTests).toBe(false); }); + + it("sets a fail-fast Vitest watchdog for changed checks", () => { + expect(createChangedCheckVitestEnv({ PATH: "/usr/bin" })).toMatchObject({ + PATH: "/usr/bin", + OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS: CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS, + OPENCLAW_VITEST_NO_OUTPUT_RETRY: "0", + }); + + expect( + createChangedCheckVitestEnv({ + OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS: "45000", + OPENCLAW_VITEST_NO_OUTPUT_RETRY: "1", + }), + ).toMatchObject({ + OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS: "45000", + OPENCLAW_VITEST_NO_OUTPUT_RETRY: "1", + }); + }); }); diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 60b904ce486..0637b4476b1 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -12,6 +12,7 @@ import { resolveChangedTestTargetPlan, resolveChangedTargetArgs, resolveParallelFullSuiteConcurrency, + shouldRetryVitestNoOutputTimeout, } from "../../scripts/test-projects.test-support.mjs"; describe("scripts/test-projects changed-target routing", () => { @@ -873,6 +874,15 @@ describe("scripts/test-projects Vitest stall watchdog", () => { expect(specs[0]?.env.OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS).toBeUndefined(); expect(specs[1]?.env.OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS).toBe("0"); }); + + it("allows changed checks to disable automatic silent-run retries", () => { + expect(shouldRetryVitestNoOutputTimeout({})).toBe(true); + expect(shouldRetryVitestNoOutputTimeout({ OPENCLAW_VITEST_NO_OUTPUT_RETRY: "1" })).toBe(true); + expect(shouldRetryVitestNoOutputTimeout({ OPENCLAW_VITEST_NO_OUTPUT_RETRY: "0" })).toBe(false); + expect(shouldRetryVitestNoOutputTimeout({ OPENCLAW_VITEST_NO_OUTPUT_RETRY: "false" })).toBe( + false, + ); + }); }); describe("scripts/test-projects Vitest cache isolation", () => {