From a06daab97ec7b64b46de010fadf1251c9747b8ff Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 30 May 2026 10:07:07 +0200 Subject: [PATCH] fix(test): guard build runner help --- scripts/build-all.mjs | 124 +++++++++++++++++-------- scripts/test-projects.test-support.mjs | 1 + test/scripts/build-all.test.ts | 45 +++++++++ test/scripts/test-projects.test.ts | 10 ++ 4 files changed, 142 insertions(+), 38 deletions(-) diff --git a/scripts/build-all.mjs b/scripts/build-all.mjs index 8a2b657e8ef7..72d9b845a2f5 100644 --- a/scripts/build-all.mjs +++ b/scripts/build-all.mjs @@ -173,6 +173,44 @@ export const BUILD_ALL_PROFILE_STEP_ENV = { }, }; +export function buildAllUsage() { + return [ + "Usage: node scripts/build-all.mjs [profile]", + "", + "Builds OpenClaw artifacts for the selected profile.", + "", + "Profiles:", + ...Object.keys(BUILD_ALL_PROFILES).map((profile) => ` ${profile}`), + "", + "Options:", + " -h, --help Show this help.", + ].join("\n"); +} + +export function parseBuildAllArgs(argv) { + const args = { + help: false, + profile: "full", + }; + let sawProfile = false; + for (const arg of argv) { + if (arg === "--help" || arg === "-h") { + args.help = true; + } else if (arg.startsWith("-")) { + throw new Error(`unknown argument: ${arg}\n\n${buildAllUsage()}`); + } else if (sawProfile) { + throw new Error(`unexpected argument: ${arg}\n\n${buildAllUsage()}`); + } else { + args.profile = arg; + sawProfile = true; + } + } + if (!args.help && !BUILD_ALL_PROFILES[args.profile]) { + throw new Error(`Unknown build profile: ${args.profile}\n\n${buildAllUsage()}`); + } + return args; +} + export function resolveBuildAllSteps(profile = "full") { const labels = BUILD_ALL_PROFILES[profile]; if (!labels) { @@ -465,44 +503,54 @@ function isMainModule() { } if (isMainModule()) { - const profile = process.argv[2] ?? "full"; - const timings = []; - let exitCode = 0; - for (const step of resolveBuildAllSteps(profile)) { - const startedAt = performance.now(); - const cacheState = resolveBuildAllStepCacheState(step); - if (process.env.OPENCLAW_BUILD_CACHE !== "0" && cacheState.fresh) { - restoreBuildAllStepCacheOutputs(cacheState); - const durationMs = performance.now() - startedAt; - timings.push({ label: step.label, status: "cached", durationMs }); - console.error(`[build-all] ${step.label} (cached) ${formatBuildAllDuration(durationMs)}`); - continue; - } - console.error(`[build-all] ${step.label}`); - const invocation = resolveBuildAllStep(step); - const result = spawnSync(invocation.command, invocation.args, invocation.options); - const durationMs = performance.now() - startedAt; - if (typeof result.status === "number") { - if (result.status !== 0) { - timings.push({ label: step.label, status: "failed", durationMs }); - console.error( - `[build-all] ${step.label} failed after ${formatBuildAllDuration(durationMs)}`, - ); - exitCode = result.status; - break; - } - writeBuildAllStepCacheStamp(step, resolveBuildAllStepCacheState(step)); - timings.push({ label: step.label, status: "ran", durationMs }); - console.error(`[build-all] ${step.label} done in ${formatBuildAllDuration(durationMs)}`); - continue; - } - timings.push({ label: step.label, status: "failed", durationMs }); - console.error(`[build-all] ${step.label} failed after ${formatBuildAllDuration(durationMs)}`); - exitCode = 1; - break; + let args; + try { + args = parseBuildAllArgs(process.argv.slice(2)); + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exit(2); } - console.error(formatBuildAllTimingSummary(timings)); - if (exitCode !== 0) { - process.exit(exitCode); + if (args?.help) { + console.log(buildAllUsage()); + } else { + const timings = []; + let exitCode = 0; + for (const step of resolveBuildAllSteps(args.profile)) { + const startedAt = performance.now(); + const cacheState = resolveBuildAllStepCacheState(step); + if (process.env.OPENCLAW_BUILD_CACHE !== "0" && cacheState.fresh) { + restoreBuildAllStepCacheOutputs(cacheState); + const durationMs = performance.now() - startedAt; + timings.push({ label: step.label, status: "cached", durationMs }); + console.error(`[build-all] ${step.label} (cached) ${formatBuildAllDuration(durationMs)}`); + continue; + } + console.error(`[build-all] ${step.label}`); + const invocation = resolveBuildAllStep(step); + const result = spawnSync(invocation.command, invocation.args, invocation.options); + const durationMs = performance.now() - startedAt; + if (typeof result.status === "number") { + if (result.status !== 0) { + timings.push({ label: step.label, status: "failed", durationMs }); + console.error( + `[build-all] ${step.label} failed after ${formatBuildAllDuration(durationMs)}`, + ); + exitCode = result.status; + break; + } + writeBuildAllStepCacheStamp(step, resolveBuildAllStepCacheState(step)); + timings.push({ label: step.label, status: "ran", durationMs }); + console.error(`[build-all] ${step.label} done in ${formatBuildAllDuration(durationMs)}`); + continue; + } + timings.push({ label: step.label, status: "failed", durationMs }); + console.error(`[build-all] ${step.label} failed after ${formatBuildAllDuration(durationMs)}`); + exitCode = 1; + break; + } + console.error(formatBuildAllTimingSummary(timings)); + if (exitCode !== 0) { + process.exit(exitCode); + } } } diff --git a/scripts/test-projects.test-support.mjs b/scripts/test-projects.test-support.mjs index eb137976e11f..de4a4298ee49 100644 --- a/scripts/test-projects.test-support.mjs +++ b/scripts/test-projects.test-support.mjs @@ -365,6 +365,7 @@ const PRECISE_SOURCE_TEST_TARGETS = new Map([ ]); const BROAD_ONLY_TEST_HELPERS = new Set(["test/helpers/poll.ts"]); const TOOLING_SOURCE_TEST_TARGETS = new Map([ + ["scripts/build-all.mjs", ["test/scripts/build-all.test.ts"]], ["scripts/github/barnacle-auto-response.mjs", ["test/scripts/barnacle-auto-response.test.ts"]], ["scripts/changed-lanes.mjs", ["test/scripts/changed-lanes.test.ts"]], ["scripts/check.mjs", ["test/scripts/check.test.ts"]], diff --git a/test/scripts/build-all.test.ts b/test/scripts/build-all.test.ts index e820b4b3a1ad..e9e9d81dc068 100644 --- a/test/scripts/build-all.test.ts +++ b/test/scripts/build-all.test.ts @@ -1,3 +1,4 @@ +import { spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -6,8 +7,10 @@ import { BUILD_ALL_PROFILES, BUILD_ALL_PROFILE_STEP_ENV, BUILD_ALL_STEPS, + buildAllUsage, formatBuildAllDuration, formatBuildAllTimingSummary, + parseBuildAllArgs, resolveBuildAllStepCacheState, resolveBuildAllStep, resolveBuildAllSteps, @@ -170,6 +173,48 @@ describe("resolveBuildAllStep", () => { }); describe("resolveBuildAllSteps", () => { + it("parses build-all CLI args before any build work", () => { + expect(parseBuildAllArgs([])).toEqual({ help: false, profile: "full" }); + expect(parseBuildAllArgs(["cliStartup"])).toEqual({ help: false, profile: "cliStartup" }); + expect(parseBuildAllArgs(["cliStartup", "--help"])).toEqual({ + help: true, + profile: "cliStartup", + }); + expect(() => parseBuildAllArgs(["cliStartup", "--bogus"])).toThrow( + "unknown argument: --bogus", + ); + expect(() => parseBuildAllArgs(["wat"])).toThrow("Unknown build profile: wat"); + }); + + it("prints CLI help without starting build steps", () => { + for (const args of [["--help"], ["cliStartup", "--help"]]) { + const result = spawnSync(process.execPath, ["scripts/build-all.mjs", ...args], { + cwd: process.cwd(), + encoding: "utf8", + }); + + expect(result.status).toBe(0); + expect(result.stderr).toBe(""); + expect(result.stdout).toContain("Usage: node scripts/build-all.mjs [profile]"); + expect(result.stdout).toContain("cliStartup"); + expect(result.stdout).not.toContain("[build-all]"); + } + }); + + it("rejects unknown CLI args without starting build steps", () => { + const result = spawnSync(process.execPath, ["scripts/build-all.mjs", "cliStartup", "--bogus"], { + cwd: process.cwd(), + encoding: "utf8", + }); + + expect(result.status).toBe(2); + expect(result.stdout).toBe(""); + expect(result.stderr).toContain("unknown argument: --bogus"); + expect(result.stderr).toContain(buildAllUsage()); + expect(result.stderr).not.toContain("[build-all]"); + expect(result.stderr).not.toContain("at "); + }); + it("keeps the full profile aligned with the declared steps", () => { expect(resolveBuildAllSteps("full")).toEqual(BUILD_ALL_STEPS); expect(BUILD_ALL_PROFILES.full).toEqual(BUILD_ALL_STEPS.map((step) => step.label)); diff --git a/test/scripts/test-projects.test.ts b/test/scripts/test-projects.test.ts index 74fa3d06d34c..d89b54dfd1da 100644 --- a/test/scripts/test-projects.test.ts +++ b/test/scripts/test-projects.test.ts @@ -307,6 +307,13 @@ describe("scripts/test-projects changed-target routing", () => { }); }); + it("keeps build runner edits on build runner tests", () => { + expect(resolveChangedTestTargetPlan(["scripts/build-all.mjs"])).toEqual({ + mode: "targets", + targets: ["test/scripts/build-all.test.ts"], + }); + }); + it("keeps force-test runner edits on its safe CLI tests", () => { expect(resolveChangedTestTargetPlan(["scripts/test-force.ts"])).toEqual({ mode: "targets", @@ -331,6 +338,7 @@ describe("scripts/test-projects changed-target routing", () => { it("routes explicit tooling implementation files to owner tests", () => { expect( findUnmatchedExplicitTestTargets([ + "scripts/build-all.mjs", "scripts/check.mjs", "scripts/check-dynamic-import-warts.mjs", "scripts/run-oxlint-shards.mjs", @@ -342,6 +350,7 @@ describe("scripts/test-projects changed-target routing", () => { expect( buildVitestRunPlans([ + "scripts/build-all.mjs", "scripts/check.mjs", "scripts/check-dynamic-import-warts.mjs", "scripts/run-oxlint-shards.mjs", @@ -364,6 +373,7 @@ describe("scripts/test-projects changed-target routing", () => { config: "test/vitest/vitest.tooling.config.ts", forwardedArgs: [], includePatterns: [ + "test/scripts/build-all.test.ts", "test/scripts/check-dynamic-import-warts.test.ts", "test/scripts/run-oxlint.test.ts", "test/scripts/tsdown-build.test.ts",