mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 14:16:28 +03:00
Route the last two benchmark scripts through the shared argv parser
Follow-up on review feedback. The previous commit consolidated four of the six Live benchmark parsers and left these two on their own hand-rolled `arg()`, which was the inconsistency the first pass was meant to remove. - benchmark-live-control.mjs and benchmark-live-init.mjs parsed --iterations with Number(), so a non-numeric value became NaN and `index < NaN` ran the benchmark zero times before failing on the metrics file. They also accepted only the space-separated form, so --iterations=20 silently measured the default. Both now use parseArgs + positiveIntFlag, which throws on a value that was clearly meant as a number. - benchmark-live-control.mjs read the metrics file with no handling for the case where the run produced nothing: a missing file surfaced as a raw ENOENT stack and a malformed line as a bare SyntaxError. Report both with a diagnostic naming the file and the env var that populates it. - summarize() now reports a `samples` count and nulls instead of letting percentile() read past an empty array, where the NaN serialized to null and a report of nothing measured looked like a real measurement. Prepared with AI assistance under maintainer direction. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -6,9 +6,12 @@ import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
import { parseArgs, positiveIntFlag } from './lib/cli-args.mjs';
|
||||
|
||||
const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
|
||||
const iterations = Math.max(1, Number(arg('--iterations') || 5));
|
||||
const fixture = arg('--fixture') || 'vite8-react-plain';
|
||||
const args = parseArgs(process.argv.slice(2));
|
||||
const iterations = positiveIntFlag(args.iterations, 5);
|
||||
const fixture = args.fixture ? String(args.fixture) : 'vite8-react-plain';
|
||||
const metricsFile = path.join(os.tmpdir(), 'impeccable-live-control-' + process.pid + '.jsonl');
|
||||
|
||||
try {
|
||||
@@ -26,7 +29,7 @@ try {
|
||||
});
|
||||
}
|
||||
|
||||
const rows = fs.readFileSync(metricsFile, 'utf-8').trim().split('\n').filter(Boolean).map(JSON.parse);
|
||||
const rows = readMetrics(metricsFile);
|
||||
console.log(JSON.stringify({
|
||||
fixture,
|
||||
iterations: rows.length,
|
||||
@@ -39,9 +42,39 @@ try {
|
||||
try { fs.unlinkSync(metricsFile); } catch {}
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the metrics the e2e run appended. Fail loudly rather than reporting a
|
||||
* summary of nothing: an absent file means the run never produced a sample, and
|
||||
* an ENOENT stack or a `{"medianMs": null}` report both read as "measured" when
|
||||
* nothing was measured at all.
|
||||
*/
|
||||
function readMetrics(file) {
|
||||
let raw;
|
||||
try {
|
||||
raw = fs.readFileSync(file, 'utf-8');
|
||||
} catch (error) {
|
||||
if (error.code !== 'ENOENT') throw error;
|
||||
throw new Error(`no metrics were recorded at ${file}. Did the e2e run emit IMPECCABLE_E2E_METRICS_FILE rows?`);
|
||||
}
|
||||
const rows = raw.trim().split('\n').filter(Boolean).map((line, index) => {
|
||||
try {
|
||||
return JSON.parse(line);
|
||||
} catch (error) {
|
||||
throw new Error(`metrics line ${index + 1} is not valid JSON: ${error.message}`);
|
||||
}
|
||||
});
|
||||
if (rows.length === 0) throw new Error(`metrics file ${file} is empty; nothing to summarize`);
|
||||
return rows;
|
||||
}
|
||||
|
||||
function summarize(values) {
|
||||
const sorted = [...values].sort((a, b) => a - b);
|
||||
const sorted = values.filter((value) => Number.isFinite(value)).sort((a, b) => a - b);
|
||||
// Distinguish "every sample was missing this metric" from a real measurement.
|
||||
// percentile() on an empty array reads sorted[-1] and yields NaN, which
|
||||
// JSON.stringify turns into null and silently passes for a result.
|
||||
if (sorted.length === 0) return { samples: 0, medianMs: null, p95Ms: null, minMs: null, maxMs: null };
|
||||
return {
|
||||
samples: sorted.length,
|
||||
medianMs: percentile(sorted, 0.5),
|
||||
p95Ms: percentile(sorted, 0.95),
|
||||
minMs: sorted[0],
|
||||
@@ -55,8 +88,3 @@ function percentile(sorted, p) {
|
||||
const upper = Math.ceil(index);
|
||||
return Math.round((sorted[lower] * (1 - (index - lower)) + sorted[upper] * (index - lower)) * 100) / 100;
|
||||
}
|
||||
|
||||
function arg(name) {
|
||||
const index = process.argv.indexOf(name);
|
||||
return index >= 0 ? process.argv[index + 1] : undefined;
|
||||
}
|
||||
|
||||
@@ -6,11 +6,14 @@ import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
import { parseArgs, positiveIntFlag } from './lib/cli-args.mjs';
|
||||
|
||||
const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
|
||||
const liveScript = path.join(root, 'skill/scripts/live.mjs');
|
||||
const serverScript = path.join(root, 'skill/scripts/live-server.mjs');
|
||||
const iterations = Math.max(1, Number(arg('--iterations') || 10));
|
||||
const fixture = arg('--fixture') || 'vite8-react-plain';
|
||||
const args = parseArgs(process.argv.slice(2));
|
||||
const iterations = positiveIntFlag(args.iterations, 10);
|
||||
const fixture = args.fixture ? String(args.fixture) : 'vite8-react-plain';
|
||||
const fixtureDir = path.join(root, 'tests/framework-fixtures', fixture, 'files');
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-live-init-'));
|
||||
|
||||
@@ -95,8 +98,3 @@ function percentile(sorted, value) {
|
||||
function round(value) {
|
||||
return Math.round(value * 100) / 100;
|
||||
}
|
||||
|
||||
function arg(name) {
|
||||
const index = process.argv.indexOf(name);
|
||||
return index >= 0 ? process.argv[index + 1] : undefined;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user