fix(scripts): reject loose test perf budgets

2026-05-31 09:28:30 +00:00 · 2026-05-29 05:44:51 +02:00
parent 18ef59bb33
commit 9ec4e94c48
2 changed files with 146 additions and 52 deletions
--- 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();
 }
--- a/test/scripts/test-perf-budget.test.ts
+++ 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",
+    );
+  });
+});