From f8a41e5e9c2a147ab1b12f17e6c3a3c8d55648d9 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 25 Apr 2026 03:43:50 -0700 Subject: [PATCH] fix(test): serialize changed checks locally --- scripts/check-changed.mjs | 172 ++++++++++++++----------- scripts/test-projects.test-support.mjs | 4 + test/scripts/changed-lanes.test.ts | 10 ++ test/scripts/test-projects.test.ts | 18 +++ 4 files changed, 131 insertions(+), 73 deletions(-) diff --git a/scripts/check-changed.mjs b/scripts/check-changed.mjs index 45b0ebf668f..33fa71162f1 100644 --- a/scripts/check-changed.mjs +++ b/scripts/check-changed.mjs @@ -7,7 +7,10 @@ import { } from "./changed-lanes.mjs"; import { booleanFlag, parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs"; import { printTimingSummary } from "./lib/check-timing-summary.mjs"; -import { resolveLocalHeavyCheckEnv } from "./lib/local-heavy-check-runtime.mjs"; +import { + acquireLocalHeavyCheckLockSync, + resolveLocalHeavyCheckEnv, +} from "./lib/local-heavy-check-runtime.mjs"; import { runManagedCommand } from "./lib/managed-child-process.mjs"; import { createSparseTsgoSkipEnv } from "./lib/tsgo-sparse-guard.mjs"; import { isCiLikeEnv } from "./lib/vitest-local-scheduling.mjs"; @@ -17,8 +20,18 @@ export const CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS = "600000"; const VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY = "OPENCLAW_VITEST_NO_OUTPUT_TIMEOUT_MS"; const VITEST_NO_OUTPUT_RETRY_ENV_KEY = "OPENCLAW_VITEST_NO_OUTPUT_RETRY"; -export function createChangedCheckVitestEnv(baseEnv = process.env) { +export function createChangedCheckChildEnv(baseEnv = process.env) { const resolvedBaseEnv = resolveLocalHeavyCheckEnv(baseEnv); + return { + ...resolvedBaseEnv, + OPENCLAW_OXLINT_SKIP_LOCK: "1", + OPENCLAW_TEST_HEAVY_CHECK_LOCK_HELD: "1", + OPENCLAW_TSGO_HEAVY_CHECK_LOCK_HELD: "1", + }; +} + +export function createChangedCheckVitestEnv(baseEnv = process.env) { + const resolvedBaseEnv = createChangedCheckChildEnv(baseEnv); const env = { ...resolvedBaseEnv, [VITEST_NO_OUTPUT_TIMEOUT_ENV_KEY]: @@ -48,7 +61,7 @@ export function createChangedCheckVitestEnv(baseEnv = process.env) { export function createChangedCheckPlan(result, options = {}) { const commands = []; - const baseEnv = resolveLocalHeavyCheckEnv(options.env ?? process.env); + const baseEnv = createChangedCheckChildEnv(options.env ?? process.env); const add = (name, args, env) => { if (!commands.some((command) => command.name === name && sameArgs(command.args, args))) { commands.push({ name, args, ...(env ? { env } : {}) }); @@ -164,79 +177,92 @@ export function createChangedCheckPlan(result, options = {}) { export async function runChangedCheck(result, options = {}) { const baseEnv = resolveLocalHeavyCheckEnv(options.env ?? process.env); - const plan = createChangedCheckPlan(result, { ...options, env: baseEnv }); - printPlan(result, plan, options); + const childEnv = createChangedCheckChildEnv(baseEnv); + const plan = createChangedCheckPlan(result, { ...options, env: childEnv }); + const releaseLock = options.dryRun + ? () => {} + : acquireLocalHeavyCheckLockSync({ + cwd: process.cwd(), + env: baseEnv, + toolName: "check:changed", + }); - if (options.dryRun) { + try { + printPlan(result, plan, options); + + if (options.dryRun) { + return 0; + } + + const timings = []; + for (const command of plan.commands) { + const status = await runPnpm(command, timings); + if (status !== 0) { + printSummary(timings, options); + return status; + } + } + + if (plan.runFullTests) { + const status = await runPnpm( + { name: "tests all", args: ["test"], env: createChangedCheckVitestEnv(childEnv) }, + timings, + ); + if (status !== 0) { + printSummary(timings, options); + return status; + } + } else if (plan.runChangedTestsBroad) { + const testArgs = options.explicitPaths + ? ["test"] + : ["test", "--changed", options.base ?? "origin/main"]; + const status = await runPnpm( + { + name: options.explicitPaths ? "tests all" : "tests changed broad", + args: testArgs, + env: createChangedCheckVitestEnv(childEnv), + }, + timings, + ); + if (status !== 0) { + printSummary(timings, options); + return status; + } + } else if (plan.testTargets.length > 0) { + const status = await runPnpm( + { + name: "tests changed", + args: ["test", ...plan.testTargets], + env: createChangedCheckVitestEnv(childEnv), + }, + timings, + ); + if (status !== 0) { + printSummary(timings, options); + return status; + } + } + + if (plan.runExtensionTests) { + const status = await runPnpm( + { + name: "tests extensions", + args: ["test:extensions"], + env: createChangedCheckVitestEnv(childEnv), + }, + timings, + ); + if (status !== 0) { + printSummary(timings, options); + return status; + } + } + + printSummary(timings, options); return 0; + } finally { + releaseLock(); } - - const timings = []; - for (const command of plan.commands) { - const status = await runPnpm(command, timings); - if (status !== 0) { - printSummary(timings, options); - return status; - } - } - - if (plan.runFullTests) { - const status = await runPnpm( - { name: "tests all", args: ["test"], env: createChangedCheckVitestEnv() }, - timings, - ); - if (status !== 0) { - printSummary(timings, options); - return status; - } - } else if (plan.runChangedTestsBroad) { - const testArgs = options.explicitPaths - ? ["test"] - : ["test", "--changed", options.base ?? "origin/main"]; - const status = await runPnpm( - { - name: options.explicitPaths ? "tests all" : "tests changed broad", - args: testArgs, - env: createChangedCheckVitestEnv(), - }, - timings, - ); - if (status !== 0) { - printSummary(timings, options); - return status; - } - } else if (plan.testTargets.length > 0) { - const status = await runPnpm( - { - name: "tests changed", - args: ["test", ...plan.testTargets], - env: createChangedCheckVitestEnv(), - }, - timings, - ); - if (status !== 0) { - printSummary(timings, options); - return status; - } - } - - if (plan.runExtensionTests) { - const status = await runPnpm( - { - name: "tests extensions", - args: ["test:extensions"], - env: createChangedCheckVitestEnv(), - }, - timings, - ); - if (status !== 0) { - printSummary(timings, options); - return status; - } - } - - printSummary(timings, options); - return 0; } function sameArgs(left, right) { diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index 8647dec29a6..4f3b4faf20f 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -1234,6 +1234,10 @@ function filterPlansForContractIncludeFile(plans, env) { } export function shouldAcquireLocalHeavyCheckLock(runSpecs, env = process.env) { + if (env.OPENCLAW_TEST_HEAVY_CHECK_LOCK_HELD === "1") { + return false; + } + if (env.OPENCLAW_TEST_PROJECTS_FORCE_LOCK === "1") { return true; } diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index 1720ea291ff..6c18d254ad7 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -5,6 +5,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { detectChangedLanes } from "../../scripts/changed-lanes.mjs"; import { CHANGED_CHECK_VITEST_NO_OUTPUT_TIMEOUT_MS, + createChangedCheckChildEnv, createChangedCheckPlan, createChangedCheckVitestEnv, } from "../../scripts/check-changed.mjs"; @@ -112,6 +113,15 @@ describe("scripts/changed-lanes", () => { }); }); + it("marks changed-check children as covered by the parent heavy-check lock", () => { + expect(createChangedCheckChildEnv({ PATH: "/usr/bin" })).toMatchObject({ + OPENCLAW_OXLINT_SKIP_LOCK: "1", + OPENCLAW_TEST_HEAVY_CHECK_LOCK_HELD: "1", + OPENCLAW_TSGO_HEAVY_CHECK_LOCK_HELD: "1", + PATH: "/usr/bin", + }); + }); + it("routes core test-only changes to core test lanes only", () => { const result = detectChangedLanes(["src/shared/string-normalization.test.ts"]); diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index d6fdaa3b3e6..827f46e333b 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -527,6 +527,24 @@ describe("scripts/test-projects local heavy-check lock", () => { ).toBe(true); }); + it("skips the lock when a parent changed gate already holds it", () => { + expect( + shouldAcquireLocalHeavyCheckLock( + [ + { + config: "test/vitest/vitest.unit.config.ts", + includePatterns: ["src/infra/vitest-config.test.ts"], + watchMode: false, + }, + ], + { + ...process.env, + OPENCLAW_TEST_HEAVY_CHECK_LOCK_HELD: "1", + }, + ), + ).toBe(false); + }); + it("allows forcing the lock back on", () => { expect( shouldAcquireLocalHeavyCheckLock(