fix(test): fail startup bench on bad samples

vincentkoc · vincentkoc · commit 8c6da93fdf7c · 2026-05-27T11:46:02.000+02:00
diff --git a/scripts/bench-cli-startup.ts b/scripts/bench-cli-startup.ts
@@ -2,6 +2,7 @@ import { spawn } from "node:child_process";
 import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import os from "node:os";
 import path from "node:path";
+import { pathToFileURL } from "node:url";
 
 type CommandCase = {
   id: string;
@@ -394,6 +395,17 @@ function parseRepeatableFlag(flag: string): string[] {
 }
 
 function parsePositiveInt(raw: string | undefined, fallback: number): number {
+  if (!raw) {
+    return fallback;
+  }
+  const parsed = Number.parseInt(raw, 10);
+  if (!Number.isFinite(parsed) || parsed < 1) {
+    return fallback;
+  }
+  return parsed;
+}
+
+function parseNonNegativeInt(raw: string | undefined, fallback: number): number {
   if (!raw) {
     return fallback;
   }
@@ -747,6 +759,25 @@ function printDelta(primary: SuiteResult, secondary: SuiteResult): void {
   }
 }
 
+export function collectFailedSamples(result: SuiteResult): string[] {
+  const failures: string[] = [];
+  for (const commandCase of result.cases) {
+    if (commandCase.samples.length === 0) {
+      failures.push(`${result.entry} ${commandCase.id}: no measured samples`);
+      continue;
+    }
+    for (const [sampleIndex, sample] of commandCase.samples.entries()) {
+      const label = `${result.entry} ${commandCase.id} sample ${sampleIndex + 1}`;
+      if (sample.signal !== null) {
+        failures.push(`${label}: exited via signal ${sample.signal}`);
+      } else if (sample.exitCode !== 0) {
+        failures.push(`${label}: exited with code ${String(sample.exitCode)}`);
+      }
+    }
+  }
+  return failures;
+}
+
 async function buildSuiteResult(params: {
   entry: string;
   options: CliOptions;
@@ -796,7 +827,7 @@ function parseOptions(): CliOptions {
     entryPrimary: parseFlagValue("--entry-primary") ?? parseFlagValue("--entry") ?? DEFAULT_ENTRY,
     entrySecondary: parseFlagValue("--entry-secondary"),
     runs: parsePositiveInt(parseFlagValue("--runs"), DEFAULT_RUNS),
-    warmup: parsePositiveInt(parseFlagValue("--warmup"), DEFAULT_WARMUP),
+    warmup: parseNonNegativeInt(parseFlagValue("--warmup"), DEFAULT_WARMUP),
     timeoutMs: parsePositiveInt(parseFlagValue("--timeout-ms"), DEFAULT_TIMEOUT_MS),
     json: hasFlag("--json"),
     output: parseFlagValue("--output"),
@@ -864,6 +895,10 @@ async function main(): Promise<void> {
       primary,
       secondary: secondary ?? null,
     };
+    const failures = [
+      ...collectFailedSamples(primary),
+      ...(secondary ? collectFailedSamples(secondary) : []),
+    ];
 
     if (options.output) {
       mkdirSync(path.dirname(options.output), { recursive: true });
@@ -872,6 +907,12 @@ async function main(): Promise<void> {
 
     if (options.json) {
       console.log(JSON.stringify(report, null, 2));
+      if (failures.length > 0) {
+        process.exitCode = 1;
+        for (const failure of failures) {
+          console.error(`[startup-bench] ${failure}`);
+        }
+      }
       return;
     }
 
@@ -894,9 +935,28 @@ async function main(): Promise<void> {
       printSuite(secondary);
       printDelta(primary, secondary);
     }
+
+    if (failures.length > 0) {
+      process.exitCode = 1;
+      console.error("\nFailed startup benchmark samples:");
+      for (const failure of failures) {
+        console.error(`- ${failure}`);
+      }
+    }
   } finally {
     rmSync(tmpDir, { recursive: true, force: true });
   }
 }
 
-await main();
+export const testing = {
+  collectFailedSamples,
+  parseNonNegativeInt,
+  parsePositiveInt,
+};
+
+if (import.meta.url === pathToFileURL(process.argv[1] ?? "").href) {
+  await main().catch((error: unknown) => {
+    console.error(error instanceof Error ? error.stack : String(error));
+    process.exit(1);
+  });
+}
diff --git a/test/scripts/bench-cli-startup.test.ts b/test/scripts/bench-cli-startup.test.ts
@@ -0,0 +1,73 @@
+import { describe, expect, it } from "vitest";
+import { testing } from "../../scripts/bench-cli-startup.ts";
+
+describe("bench-cli-startup", () => {
+  it("fails reports with no measured samples", () => {
+    expect(
+      testing.collectFailedSamples({
+        entry: "openclaw.mjs",
+        cases: [
+          {
+            id: "version",
+            name: "--version",
+            args: ["--version"],
+            contract: null,
+            samples: [],
+            summary: {
+              sampleCount: 0,
+              durationMs: { avg: 0, p50: 0, p95: 0, min: 0, max: 0 },
+              firstOutputMs: null,
+              maxRssMb: null,
+              exitSummary: "",
+            },
+          },
+        ],
+      }),
+    ).toEqual(["openclaw.mjs version: no measured samples"]);
+  });
+
+  it("fails reports with nonzero or signaled CLI samples", () => {
+    const passingSample = {
+      ms: 10,
+      firstOutputMs: 5,
+      maxRssMb: 50,
+      exitCode: 0,
+      signal: null,
+    };
+
+    expect(
+      testing.collectFailedSamples({
+        entry: "dist/entry.js",
+        cases: [
+          {
+            id: "gatewayStatusJson",
+            name: "gateway status --json",
+            args: ["gateway", "status", "--json"],
+            contract: null,
+            samples: [
+              passingSample,
+              { ...passingSample, exitCode: 1 },
+              { ...passingSample, exitCode: null, signal: "SIGTERM" },
+            ],
+            summary: {
+              sampleCount: 3,
+              durationMs: { avg: 10, p50: 10, p95: 10, min: 10, max: 10 },
+              firstOutputMs: { avg: 5, p50: 5, p95: 5, min: 5, max: 5 },
+              maxRssMb: { avg: 50, p50: 50, p95: 50, min: 50, max: 50 },
+              exitSummary: "code:0x1, code:1x1, signal:SIGTERMx1",
+            },
+          },
+        ],
+      }),
+    ).toEqual([
+      "dist/entry.js gatewayStatusJson sample 2: exited with code 1",
+      "dist/entry.js gatewayStatusJson sample 3: exited via signal SIGTERM",
+    ]);
+  });
+
+  it("does not accept zero measured runs", () => {
+    expect(testing.parsePositiveInt("0", 5)).toBe(5);
+    expect(testing.parsePositiveInt("1", 5)).toBe(1);
+    expect(testing.parseNonNegativeInt("0", 1)).toBe(0);
+  });
+});
