From 9ec4e94c4860efaa41e2a5c907a06892ea0eb194 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 29 May 2026 05:44:51 +0200 Subject: [PATCH] fix(scripts): reject loose test perf budgets --- scripts/test-perf-budget.mjs | 161 +++++++++++++++++--------- test/scripts/test-perf-budget.test.ts | 37 ++++++ 2 files changed, 146 insertions(+), 52 deletions(-) create mode 100644 test/scripts/test-perf-budget.test.ts diff --git a/scripts/test-perf-budget.mjs b/scripts/test-perf-budget.mjs index efccfc9cfb5..b77342656ee 100644 --- a/scripts/test-perf-budget.mjs +++ b/scripts/test-perf-budget.mjs @@ -1,74 +1,131 @@ -import { floatFlag, parseFlagArgs, readEnvNumber, stringFlag } from "./lib/arg-utils.mjs"; +import { pathToFileURL } from "node:url"; +import { parseFlagArgs, stringFlag } from "./lib/arg-utils.mjs"; import { formatMs } from "./lib/vitest-report-cli-utils.mjs"; import { readJsonFile, runVitestJsonReport } from "./test-report-utils.mjs"; -function parseArgs(argv) { +function parseBudgetNumber(raw, label) { + const value = raw?.trim(); + if (!value) { + return null; + } + const parsed = Number(value); + if (!Number.isFinite(parsed) || parsed < 0) { + throw new Error(`${label} must be a non-negative number`); + } + return parsed; +} + +function readBudgetEnvNumber(name, env) { + return parseBudgetNumber(env[name], name); +} + +function budgetFloatFlag(flag, key) { + return { + consume(argv, index) { + if (argv[index] !== flag) { + return null; + } + return { + nextIndex: index + 1, + apply(target) { + const parsed = parseBudgetNumber(argv[index + 1], flag); + if (parsed === null) { + throw new Error(`${flag} requires a value`); + } + target[key] = parsed; + }, + }; + }, + }; +} + +function parseArgs(argv, env = process.env) { return parseFlagArgs( argv, { config: "test/vitest/vitest.unit.config.ts", - maxWallMs: readEnvNumber("OPENCLAW_TEST_PERF_MAX_WALL_MS"), - baselineWallMs: readEnvNumber("OPENCLAW_TEST_PERF_BASELINE_WALL_MS"), - maxRegressionPct: readEnvNumber("OPENCLAW_TEST_PERF_MAX_REGRESSION_PCT") ?? 10, + maxWallMs: readBudgetEnvNumber("OPENCLAW_TEST_PERF_MAX_WALL_MS", env), + baselineWallMs: readBudgetEnvNumber("OPENCLAW_TEST_PERF_BASELINE_WALL_MS", env), + maxRegressionPct: readBudgetEnvNumber("OPENCLAW_TEST_PERF_MAX_REGRESSION_PCT", env) ?? 10, }, [ stringFlag("--config", "config"), - floatFlag("--max-wall-ms", "maxWallMs"), - floatFlag("--baseline-wall-ms", "baselineWallMs"), - floatFlag("--max-regression-pct", "maxRegressionPct"), + budgetFloatFlag("--max-wall-ms", "maxWallMs"), + budgetFloatFlag("--baseline-wall-ms", "baselineWallMs"), + budgetFloatFlag("--max-regression-pct", "maxRegressionPct"), ], ); } -const opts = parseArgs(process.argv.slice(2)); -const startedAt = process.hrtime.bigint(); -const reportPath = runVitestJsonReport({ - config: opts.config, - prefix: "openclaw-vitest-perf", -}); -const elapsedMs = Number(process.hrtime.bigint() - startedAt) / 1_000_000; - -let totalFileDurationMs = 0; -let fileCount = 0; -try { - const report = readJsonFile(reportPath); - for (const result of report.testResults ?? []) { - if (typeof result.startTime === "number" && typeof result.endTime === "number") { - totalFileDurationMs += Math.max(0, result.endTime - result.startTime); - fileCount += 1; - } +function main() { + let opts; + try { + opts = parseArgs(process.argv.slice(2)); + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exit(1); } -} catch { - // Keep budget checks based on wall time when JSON parsing fails. -} -const allowedByBaseline = - opts.baselineWallMs !== null - ? opts.baselineWallMs * (1 + (opts.maxRegressionPct ?? 0) / 100) - : null; + const startedAt = process.hrtime.bigint(); + const reportPath = runVitestJsonReport({ + config: opts.config, + prefix: "openclaw-vitest-perf", + }); + const elapsedMs = Number(process.hrtime.bigint() - startedAt) / 1_000_000; -let failed = false; -if (opts.maxWallMs !== null && elapsedMs > opts.maxWallMs) { - console.error( - `[test-perf-budget] wall time ${formatMs(elapsedMs)} exceeded max ${formatMs(opts.maxWallMs)}.`, + let totalFileDurationMs = 0; + let fileCount = 0; + try { + const report = readJsonFile(reportPath); + for (const result of report.testResults ?? []) { + if (typeof result.startTime === "number" && typeof result.endTime === "number") { + totalFileDurationMs += Math.max(0, result.endTime - result.startTime); + fileCount += 1; + } + } + } catch { + // Keep budget checks based on wall time when JSON parsing fails. + } + + const allowedByBaseline = + opts.baselineWallMs !== null + ? opts.baselineWallMs * (1 + (opts.maxRegressionPct ?? 0) / 100) + : null; + + let failed = false; + if (opts.maxWallMs !== null && elapsedMs > opts.maxWallMs) { + console.error( + `[test-perf-budget] wall time ${formatMs(elapsedMs)} exceeded max ${formatMs( + opts.maxWallMs, + )}.`, + ); + failed = true; + } + if (allowedByBaseline !== null && elapsedMs > allowedByBaseline) { + console.error( + `[test-perf-budget] wall time ${formatMs(elapsedMs)} exceeded baseline budget ${formatMs( + allowedByBaseline, + )} (baseline ${formatMs(opts.baselineWallMs ?? 0)}, +${String(opts.maxRegressionPct)}%).`, + ); + failed = true; + } + + console.log( + `[test-perf-budget] config=${opts.config} wall=${formatMs(elapsedMs)} file-sum=${formatMs( + totalFileDurationMs, + )} files=${String(fileCount)}`, ); - failed = true; -} -if (allowedByBaseline !== null && elapsedMs > allowedByBaseline) { - console.error( - `[test-perf-budget] wall time ${formatMs(elapsedMs)} exceeded baseline budget ${formatMs( - allowedByBaseline, - )} (baseline ${formatMs(opts.baselineWallMs ?? 0)}, +${String(opts.maxRegressionPct)}%).`, - ); - failed = true; + + if (failed) { + process.exit(1); + } } -console.log( - `[test-perf-budget] config=${opts.config} wall=${formatMs(elapsedMs)} file-sum=${formatMs( - totalFileDurationMs, - )} files=${String(fileCount)}`, -); +export const testing = { + parseArgs, + parseBudgetNumber, +}; -if (failed) { - process.exit(1); +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main(); } diff --git a/test/scripts/test-perf-budget.test.ts b/test/scripts/test-perf-budget.test.ts new file mode 100644 index 00000000000..49d13136624 --- /dev/null +++ b/test/scripts/test-perf-budget.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { testing } from "../../scripts/test-perf-budget.mjs"; + +describe("test perf budget script", () => { + it("parses numeric budget env vars strictly before running Vitest", () => { + expect( + testing.parseArgs([], { + OPENCLAW_TEST_PERF_BASELINE_WALL_MS: "1000", + OPENCLAW_TEST_PERF_MAX_REGRESSION_PCT: "12.5", + OPENCLAW_TEST_PERF_MAX_WALL_MS: "1500", + }), + ).toMatchObject({ + baselineWallMs: 1000, + maxRegressionPct: 12.5, + maxWallMs: 1500, + }); + + expect(() => + testing.parseArgs([], { + OPENCLAW_TEST_PERF_MAX_WALL_MS: "1000ms", + }), + ).toThrow("OPENCLAW_TEST_PERF_MAX_WALL_MS must be a non-negative number"); + }); + + it("rejects malformed CLI budget values before running Vitest", () => { + expect(testing.parseArgs(["--max-wall-ms", "1e3"], {})).toMatchObject({ + maxWallMs: 1000, + }); + + expect(() => testing.parseArgs(["--max-wall-ms", "1e3ms"], {})).toThrow( + "--max-wall-ms must be a non-negative number", + ); + expect(() => testing.parseArgs(["--max-regression-pct"], {})).toThrow( + "--max-regression-pct requires a value", + ); + }); +});