fix(bench): reject invalid routing benchmark options

Author: chrisbbreuer

bench/routing/run.test.ts

@@ -0,0 +1,56 @@
+import { describe, expect, it } from 'bun:test'
+import { parseArgs } from './run'
+import { DEFAULT_TARGETS } from './targets'
+
+describe('routing benchmark options', () => {
+  it('preserves the default matrix and accepts fractional timing windows', () => {
+    expect(parseArgs([])).toMatchObject({
+      targets: DEFAULT_TARGETS.map(target => target.id),
+      connections: 50,
+      warmupSeconds: 5,
+      durationSeconds: 30,
+      runs: 3,
+      db: true,
+    })
+    expect(parseArgs(['-c', '2', '-d', '0.5', '--warmup', '0.25', '--runs', '1'])).toMatchObject({
+      connections: 2, durationSeconds: 0.5, warmupSeconds: 0.25, runs: 1,
+    })
+    expect(parseArgs(['--warmup', '0']).warmupSeconds).toBe(0)
+  })
+
+  it.each(['--connections', '--runs'])('requires a positive safe integer for %s', (flag) => {
+    for (const value of ['0', '-1', '1.5', 'NaN', 'Infinity', '9007199254740992', ''])
+      expect(() => parseArgs([flag, value]), `${flag} ${value}`).toThrow()
+  })
+
+  it.each(['0', '-1', 'NaN', 'Infinity', ''])('rejects invalid measured duration %s', (value) => {
+    expect(() => parseArgs(['--duration', value])).toThrow()
+  })
+
+  it.each(['-1', 'NaN', 'Infinity', ''])('rejects invalid warmup %s', (value) => {
+    expect(() => parseArgs(['--warmup', value])).toThrow()
+  })
+
+  it.each(['missing', 'stacks,missing', '', 'stacks,'])('rejects unknown or empty target selections: %s', (value) => {
+    expect(() => parseArgs(['--targets', value])).toThrow()
+  })
+
+  it.each(['missing', 'static-json,missing', '', 'static-json,'])('rejects unknown or empty scenario selections: %s', (value) => {
+    expect(() => parseArgs(['--scenarios', value])).toThrow()
+  })
+
+  it('rejects a matrix emptied by --no-db', () => {
+    expect(() => parseArgs(['--scenarios', 'db-roundtrip', '--no-db'])).toThrow('No scenarios')
+  })
+
+  it('accepts explicit tuned targets and a mixed matrix with --no-db', () => {
+    expect(parseArgs(['--targets', 'stacks-wal-full,bun-raw', '--scenarios', 'static-json,db-roundtrip', '--no-db'])).toMatchObject({
+      targets: ['stacks-wal-full', 'bun-raw'], scenarios: ['static-json', 'db-roundtrip'], db: false,
+    })
+  })
+
+  it('rejects missing values and unknown flags', () => {
+    expect(() => parseArgs(['--runs'])).toThrow('--runs needs a value')
+    expect(() => parseArgs(['--mystery'])).toThrow('Unknown flag')
+  })
+})

bench/routing/run.ts

@@ -41,7 +41,7 @@ interface Options {
   db: boolean
 }
 
-function parseArgs(argv: string[]): Options {
+export function parseArgs(argv: string[]): Options {
   const opts: Options = {
     targets: DEFAULT_TARGETS.map(t => t.id),
     scenarios: SCENARIOS.map(s => s.id),
@@ -55,7 +55,7 @@ function parseArgs(argv: string[]): Options {
     const arg = argv[i]!
     const next = () => {
       const value = argv[++i]
-      if (value == null) throw new Error(`${arg} needs a value`)
+      if (value == null || value.trim() === '') throw new Error(`${arg} needs a value`)
       return value
     }
     switch (arg) {
@@ -75,6 +75,24 @@ function parseArgs(argv: string[]): Options {
         throw new Error(`Unknown flag ${arg}`)
     }
   }
+  if (!Number.isSafeInteger(opts.connections) || opts.connections <= 0)
+    throw new Error('--connections must be a positive safe integer')
+  if (!Number.isSafeInteger(opts.runs) || opts.runs <= 0)
+    throw new Error('--runs must be a positive safe integer')
+  if (!Number.isFinite(opts.durationSeconds) || opts.durationSeconds <= 0)
+    throw new Error('--duration must be a positive finite number')
+  if (!Number.isFinite(opts.warmupSeconds) || opts.warmupSeconds < 0)
+    throw new Error('--warmup must be a non-negative finite number')
+
+  const unknownTargets = opts.targets.filter(id => !TARGETS.some(target => target.id === id))
+  if (unknownTargets.length > 0)
+    throw new Error(`Unknown target(s): ${unknownTargets.map(id => JSON.stringify(id)).join(', ')}`)
+  const unknownScenarios = opts.scenarios.filter(id => !SCENARIOS.some(scenario => scenario.id === id))
+  if (unknownScenarios.length > 0)
+    throw new Error(`Unknown scenario(s): ${unknownScenarios.map(id => JSON.stringify(id)).join(', ')}`)
+  if (!SCENARIOS.some(scenario => opts.scenarios.includes(scenario.id) && (opts.db || !scenario.requiresDb)))
+    throw new Error('No scenarios remain after applying --no-db')
+
   return opts
 }
 
@@ -83,10 +101,10 @@ const HELP = `bun bench/routing/run.ts [flags]
   --targets      comma-separated target ids  (default: ${DEFAULT_TARGETS.map(t => t.id).join(', ')})
   --scenarios    comma-separated scenario ids (${SCENARIOS.map(s => s.id).join(', ')})
   --driver       oha | bombardier | autocannon | builtin  (default: first available)
-  --connections  concurrent connections (default 50)
-  --warmup       seconds discarded before measuring (default 5)
-  --duration     seconds measured (default 30)
-  --runs         repeats per scenario, median reported (default 3)
+  --connections  concurrent connections, positive integer (default 50)
+  --warmup       non-negative seconds discarded before measuring (default 5)
+  --duration     positive seconds measured (default 30)
+  --runs         positive integer repeats per scenario, median reported (default 3)
   --no-db        skip the SQLite fixture and the db-roundtrip scenario
 
 Available targets: ${TARGETS.map(t => t.id).join(', ')}`
@@ -208,4 +226,5 @@ async function main(): Promise<void> {
   console.log(report)
 }
 
-await main()
+if (import.meta.main)
+  await main()