From f81696886af2ec928ad3cdd1b48b3ee51a3180eb Mon Sep 17 00:00:00 2001 From: Miguel de Benito Delgado Date: Thu, 17 Sep 2026 21:37:25 +0200 Subject: [PATCH] :sparkles: Add feature-flag A/B harness to renderer benchmarks Compare every on/off combination of requested Cargo features over the Boolean-lattice edges, in Gray-code build order, with a matrix index of configs, edge comparisons and provenance. Single runs record explicit feature sets as schemaVersion 2 with resolved defaults and build environments. Edge comparisons stay diagnostic; edges clean apart from the intended toggle are marked controlled. Validate with 68 focused tests, CLI fault checks, dry runs, and container smoke matrices for branch-b pan cases. AI-assisted-by: muse-spark-1.3-contributor-free --- .../resources/wasm-playground/BENCHMARKS.md | 47 +- frontend/scripts/renderer-benchmarks.js | 454 ++++++++++++-- .../scripts/renderer-benchmarks/compare.js | 27 +- .../renderer-benchmarks/tests/compare.test.js | 54 +- .../renderer-benchmarks/tests/runner.test.js | 560 ++++++++++++++++++ 5 files changed, 1092 insertions(+), 50 deletions(-) diff --git a/frontend/resources/wasm-playground/BENCHMARKS.md b/frontend/resources/wasm-playground/BENCHMARKS.md index e8cc9abfa5..9d3ce5bce9 100644 --- a/frontend/resources/wasm-playground/BENCHMARKS.md +++ b/frontend/resources/wasm-playground/BENCHMARKS.md @@ -116,8 +116,51 @@ these commands separately from scored runs. The runner uses the manifest's default Cargo features and records their local feature expansion. It rejects diagnostic defaults and disables the optional -function-name profiling switch for scored builds. Feature selection and A/B -orchestration are separate follow-up work. +function-name profiling switch for scored builds. + +## Feature A/B comparisons + +`run --features a,b` builds and runs one configuration with explicit Cargo +features and records it. `ab --features a,b` builds and runs every on/off +configuration of the requested features (2^N for N features, default cap 3, +`--max-features` 1..6), then compares the Boolean-lattice edges: pairs that +differ in exactly one feature. Build order follows a binary-reflected Gray +code so consecutive builds differ by one feature bit; this only minimizes +rebuild wall time. Build durations are provenance +(`metadata.build.durationMs`), never metrics, but non-adjacent edges are +measured further apart in wall time, so repeated whole runs confirm findings +that matter. + +```sh +pnpm run benchmark:renderer run --features branch-b --filter rects/default/pan --output /tmp/renderer-one.json +pnpm run benchmark:renderer ab --features branch-b --filter rects/default/pan --output /tmp/renderer-ab-branch-b +pnpm run benchmark:renderer ab --features branch-b --dry-run +``` + +`ab` requires empty manifest defaults and aborts when a requested feature +overlaps the enabled defaults or names a `stats*`/`profile*` diagnostic; +`run --features` records defaults and enforces only the overlap abort. +`--dry-run` prints the plan and builds nothing. Configurations beyond 8, or an +empty `--filter`, require `--confirm`. Each configuration launches its own +browser; the printed plan states this cost. `--output` for `ab` is a directory +root that must not exist yet; `configs/.json`, +`comparisons/__.json`, and `matrix.json` land beneath it. + +Run JSON is `schemaVersion` 2: `metadata.build.features` is the explicit +sorted set (the A/B identity axis), `defaultFeatures` the resolved manifest +defaults, plus the runner-controlled `env` map and the allowlisted +`ambientEnv` (`RUSTFLAGS`, `RUSTC_WRAPPER`, `CARGO_TARGET_DIR`, `CARGO_HOME`, +`CC`, `CFLAGS`, `CXXFLAGS`, `LDFLAGS`, `PENPOT_WASM_FUNCTION_NAMES`, +`CARGO_PROFILE_*`, `EMSDK*`). Comparison of the six build fields is always +diagnostic; the expected `features` mismatch on every edge is tagged, not +suppressed. An edge is marked `controlled: true` when the intended toggle is +the only mismatch: the comparison measures exactly what the matrix was built +for, and the diagnostic banner reads as single-run evidence, not as a +verdict. Edges with other mismatches (environment, seeds, manifest defaults +shifting mid-matrix) keep the strong diagnostic-only reading and print a +terminal warning. `branch-a` has no observable effect because the scenes never +call `set_modifiers`. Exit 0 means every edge has a comparison, never a +performance verdict. ## Current validation limits diff --git a/frontend/scripts/renderer-benchmarks.js b/frontend/scripts/renderer-benchmarks.js index 8afce7b4df..336015e91b 100644 --- a/frontend/scripts/renderer-benchmarks.js +++ b/frontend/scripts/renderer-benchmarks.js @@ -18,6 +18,7 @@ import { import { compareRuns, formatComparison, + tagExpectedFeatureDiff, } from "./renderer-benchmarks/compare.js"; const frontend = fileURLToPath(new URL("../", import.meta.url)); @@ -25,6 +26,20 @@ const root = path.resolve(frontend, ".."); const LIMITS = "Renderer submission timings only: not GPU completion, displayed pixels, whole-editor responsiveness, or visual correctness."; +// Runner-controlled build overrides shared by run and ab. VERSION stays out: +// every run stamps its own runId as VERSION in the child environment. +export const BUILD_ENV = { + PENPOT_WASM_PREPARED: "1", + PENPOT_WASM_FUNCTION_NAMES: "0", + BUILD_MODE: "release", + NODE_ENV: "production", + RENDER_TARGET: "frontend", + BUILD_NAME: "render-wasm-benchmark", + CARGO_BUILD_TARGET: "wasm32-unknown-emscripten", + COREPACK_ENABLE_NETWORK: "0", + CARGO_NET_OFFLINE: "true", +}; + export function readOptions(argv) { const { values, positionals } = parseArgs({ args: argv, @@ -40,10 +55,14 @@ export function readOptions(argv) { warmups: { type: "string", default: "3" }, repetitions: { type: "string", default: "10" }, "timeout-ms": { type: "string", default: "30000" }, + features: { type: "string", default: "" }, + "max-features": { type: "string", default: "3" }, + confirm: { type: "boolean", default: false }, + "dry-run": { type: "boolean", default: false }, }, }); const command = positionals[0] ?? "run"; - if (!["run", "compare"].includes(command)) + if (!["run", "compare", "ab"].includes(command)) throw new Error(`Unknown command: ${command}`); const number = (name, minimum, maximum) => { const value = Number(values[name]); @@ -53,11 +72,35 @@ export function readOptions(argv) { }; if ( !values.help && - (command === "run" ? positionals.length > 1 : positionals.length !== 3) + (command === "compare" ? positionals.length !== 3 : positionals.length > 1) ) throw new Error( - "Use run [options] or compare baseline.json candidate.json [options]", + "Use run [options], ab --features a,b [options], or compare baseline.json candidate.json [options]", ); + const featureList = values.features + .split(",") + .map((token) => token.trim()) + .filter(Boolean); + for (const token of featureList) { + if (!/^[a-z0-9-]+$/.test(token)) + throw new Error(`Invalid --features token: ${token}`); + } + const features = [...new Set(featureList)].sort(); + const maxFeatures = number("max-features", 1, 6); + if (command !== "ab" && values["dry-run"]) + throw new Error("--dry-run is only valid for ab"); + if (command === "ab" && !values.help) { + if (features.some((name) => /^(stats|profile)(-|$)/.test(name))) + throw new Error( + "ab rejects renderer diagnostic features; record them with run --features instead", + ); + if (features.length < 1) + throw new Error("ab requires --features with at least one feature"); + if (features.length > maxFeatures) + throw new Error( + `ab requests ${features.length} features but --max-features is ${maxFeatures}`, + ); + } const baseUrl = new URL(values["base-url"]); if ( !["http:", "https:"].includes(baseUrl.protocol) || @@ -80,9 +123,43 @@ export function readOptions(argv) { warmups: number("warmups", 0, 1000), repetitions: number("repetitions", 1, 10000), timeoutMs: number("timeout-ms", 100, 600000), + features, + maxFeatures, + confirm: values.confirm, + dryRun: values["dry-run"], }; } +// All on/off configurations of sorted unique feature names in binary-reflected +// Gray order, plus the Boolean-lattice edges (pairs differing in exactly one +// feature). features[0] is the most significant bit. Pure: no I/O. +export function planFeatureMatrix(features) { + const configurations = []; + for (let index = 0; index < 2 ** features.length; index++) { + const gray = index ^ (index >> 1); + const slug = features + .map((_, bit) => (gray & (1 << (features.length - 1 - bit)) ? "1" : "0")) + .join(""); + configurations.push({ + slug, + features: features.filter((_, bit) => slug[bit] === "1"), + }); + } + const edges = []; + configurations.forEach((config) => { + features.forEach((name, bit) => { + if (config.slug[bit] === "0") { + edges.push({ + from: config.slug, + to: config.slug.slice(0, bit) + "1" + config.slug.slice(bit + 1), + toggledFeature: name, + }); + } + }); + }); + return { configurations, edges }; +} + export async function bounded(operation, milliseconds, signal) { let timer; let onAbort; @@ -152,6 +229,81 @@ export function defaultFeatures(definitions) { return [...enabled].sort(); } +// Reads the renderer package's feature definitions from the Cargo manifest +// without requiring unused targets' dependencies to be cached. +export function readRendererManifest() { + const manifest = JSON.parse( + execFileSync( + "cargo", + [ + "metadata", + "--offline", + "--locked", + "--no-deps", + "--format-version", + "1", + ], + { cwd: path.join(root, "render-wasm"), encoding: "utf8" }, + ), + ); + const renderer = manifest.packages.find( + (item) => item.manifest_path === path.join(root, "render-wasm/Cargo.toml"), + ); + if (!renderer) + throw new Error("Renderer package missing from Cargo metadata"); + return renderer.features; +} + +// Validates requested explicit features against the manifest definitions. +// Returns the resolved manifest defaults (synthetic "default" node included). +// Pure: no I/O. +export function validateFeatureSelection(definitions, features, command) { + const resolved = defaultFeatures(definitions); + const enabled = resolved.filter((name) => name !== "default"); + for (const name of features) { + if (!Object.hasOwn(definitions, name)) + throw new Error(`Unknown Cargo feature: ${name}`); + } + const overlap = features.filter((name) => enabled.includes(name)); + if (overlap.length) + throw new Error( + `Requested features already enabled by default: ${overlap.join(",")}`, + ); + if (command === "ab" && enabled.length) + throw new Error( + `ab requires an empty default feature set; found: ${enabled.join(",")}`, + ); + return resolved; +} + +const AMBIENT_ENV_EXACT = new Set([ + "RUSTFLAGS", + "RUSTC_WRAPPER", + "CARGO_TARGET_DIR", + "CARGO_HOME", + "CC", + "CFLAGS", + "CXXFLAGS", + "LDFLAGS", + "PENPOT_WASM_FUNCTION_NAMES", +]); + +// Records the ambient build environment allowlist. Never the full process +// environment. Pure: no I/O. +export function recordAmbientEnv(env = process.env) { + const recorded = {}; + for (const [key, value] of Object.entries(env)) { + if ( + AMBIENT_ENV_EXACT.has(key) || + key.startsWith("CARGO_PROFILE_") || + key.startsWith("EMSDK") + ) { + recorded[key] = String(value); + } + } + return recorded; +} + async function preflight(options) { await requireFile( "/opt/emsdk/emsdk_env.sh", @@ -224,7 +376,7 @@ export async function run(options) { const git = (args) => execFileSync("git", args, { cwd: root, encoding: "utf8" }).trim(); const result = { - schemaVersion: 1, + schemaVersion: 2, runId, timestamp: new Date().toISOString(), status: "running", @@ -238,7 +390,7 @@ export async function run(options) { build: { mode: "release", featurePolicy: "manifest-defaults", - features: null, + features: [], target: "frontend", prepared: true, }, @@ -322,46 +474,33 @@ export async function run(options) { ), ); result.metadata.environment.playwright = packageJson.version; - const manifest = JSON.parse( - execFileSync( - "cargo", - [ - "metadata", - "--offline", - "--locked", - "--no-deps", - "--format-version", - "1", - ], - { cwd: path.join(root, "render-wasm"), encoding: "utf8" }, - ), + const requested = [...(options.features ?? [])].sort(); + const resolved = validateFeatureSelection( + readRendererManifest(), + requested, + options.command, ); - const renderer = manifest.packages.find( - (item) => - item.manifest_path === path.join(root, "render-wasm/Cargo.toml"), - ); - if (!renderer) - throw new Error("Renderer package missing from Cargo metadata"); - result.metadata.build.features = defaultFeatures(renderer.features); + result.metadata.build.features = requested; + result.metadata.build.defaultFeatures = resolved; + if (requested.length) result.metadata.build.featurePolicy = "explicit"; + result.metadata.build.env = { ...BUILD_ENV }; + result.metadata.build.ambientEnv = recordAmbientEnv(); // Disallow package-manager network fallback in a prepared run. const buildStart = performance.now(); - await execute("./build", ["frontend", "--offline", "--locked"], { - cwd: path.join(root, "render-wasm"), - signal: abort.signal, - env: { - ...process.env, - PENPOT_WASM_PREPARED: "1", - PENPOT_WASM_FUNCTION_NAMES: "0", - BUILD_MODE: "release", - NODE_ENV: "production", - RENDER_TARGET: "frontend", - BUILD_NAME: "render-wasm-benchmark", - CARGO_BUILD_TARGET: "wasm32-unknown-emscripten", - COREPACK_ENABLE_NETWORK: "0", - CARGO_NET_OFFLINE: "true", - VERSION: runId, + await execute( + "./build", + [ + "frontend", + "--offline", + "--locked", + ...(requested.length ? ["--features", requested.join(",")] : []), + ], + { + cwd: path.join(root, "render-wasm"), + signal: abort.signal, + env: { ...process.env, ...BUILD_ENV, VERSION: runId }, }, - }); + ); result.metadata.build.durationMs = performance.now() - buildStart; const { copyWasmPlayground } = await import("./_helpers.js"); const previousCwd = process.cwd(); @@ -529,11 +668,241 @@ export async function run(options) { return result.status === "complete" ? 0 : 1; } +// Classifies one configuration output for the feature-matrix loop: whether the +// matrix continues with the next configuration or stops and marks the rest +// unattempted. Takes the parsed config JSON, or null when the file is missing +// or unparseable. Pure: no I/O. +export function classifyConfigResult(result) { + if (result === null || result === undefined) + return { + outcome: "stop", + status: "unattempted", + error: "configuration output is missing or invalid", + }; + if (result.status === "interrupted") + return { outcome: "stop", status: "interrupted" }; + if (typeof result.metadata?.build?.durationMs !== "number") + return { + outcome: "stop", + status: "failed", + error: "build or preflight failed before any suite ran", + }; + const attempts = (result.cases ?? []).reduce( + (sum, benchmarkCase) => sum + (benchmarkCase.attempts?.length ?? 0), + 0, + ); + if (attempts === 0) + return { + outcome: "stop", + status: "failed", + error: "suite recorded no attempts", + }; + return { + outcome: "continue", + status: result.status === "complete" ? "complete" : "failed", + }; +} + +// Refuses an existing feature-matrix output root: it must be absent, or an +// empty directory. Never deletes or overwrites previous matrix output. +async function ensureFreshAbOutputRoot(outputRoot) { + let stat; + try { + stat = await fs.stat(outputRoot); + } catch (error) { + if (error.code === "ENOENT") return; + throw error; + } + if (!stat.isDirectory()) + throw new Error(`ab output root already exists: ${outputRoot}`); + if ((await fs.readdir(outputRoot)).length) + throw new Error( + `ab output root already exists and is not empty: ${outputRoot}`, + ); +} + +// Builds and runs every on/off configuration of the requested features, then +// compares the Boolean-lattice edges. Each configuration runs in Gray order +// with its own build and browser. Diagnostic tooling only: exit 0 reports +// that every edge has a comparison, never a performance verdict. An +// interruption between configurations or edges still writes matrix.json and +// exits 1. Callers may inject an AbortSignal; otherwise ab watches SIGINT and +// SIGTERM itself. +export async function ab( + options, + { + runFn = run, + compareFn = compareRuns, + manifestFn = readRendererManifest, + signal = null, + } = {}, +) { + const abId = randomUUID(); + const outputRoot = options.output + ? path.resolve(options.output) + : path.join( + os.tmpdir(), + `renderer-ab-${new Date().toISOString().replaceAll(":", "-")}-${abId.slice(0, 8)}`, + ); + await ensureFreshAbOutputRoot(outputRoot); + const definitions = await manifestFn(); + validateFeatureSelection(definitions, options.features, "ab"); + const { configurations, edges } = planFeatureMatrix(options.features); + const cases = collectCases(scenarios, options.seed, options.filter); + const attemptsPerCase = options.warmups + options.repetitions; + const plan = [ + `Feature matrix: ${options.features.join(",")}`, + ...configurations.map( + (config) => ` ${config.slug}: [${config.features.join(",")}]`, + ), + ...edges.map( + (edge) => ` ${edge.from}__${edge.to}: ${edge.toggledFeature}`, + ), + `${cases.length} cases, ${attemptsPerCase} attempts per case, one browser per configuration.`, + ].join("\n"); + console.log(plan); + if (options.dryRun) return 0; + if ((configurations.length > 8 || options.filter === "") && !options.confirm) + throw new Error( + "ab plans more work than confirmed; rerun with --confirm to proceed", + ); + let watched = signal; + let releaseSignal = null; + if (watched === null) { + const abort = new AbortController(); + watched = abort.signal; + const interrupt = () => abort.abort(new Error("Benchmark interrupted")); + process.once("SIGINT", interrupt); + process.once("SIGTERM", interrupt); + releaseSignal = () => { + process.removeListener("SIGINT", interrupt); + process.removeListener("SIGTERM", interrupt); + }; + } + await fs.mkdir(path.join(outputRoot, "configs"), { recursive: true }); + await fs.mkdir(path.join(outputRoot, "comparisons"), { recursive: true }); + const parsed = new Map(); + const configurationRecords = []; + let stopped = false; + const unattemptedRecord = (config) => ({ + slug: config.slug, + features: config.features, + status: "unattempted", + file: path.join(outputRoot, "configs", `${config.slug}.json`), + runId: null, + runStatus: "unattempted", + }); + for (const config of configurations) { + if (watched.aborted) { + configurationRecords.push({ + ...unattemptedRecord(config), + error: "Benchmark interrupted", + }); + continue; + } + if (stopped) { + configurationRecords.push(unattemptedRecord(config)); + continue; + } + const configPath = path.join(outputRoot, "configs", `${config.slug}.json`); + await runFn({ + ...options, + command: "ab", + features: config.features, + output: configPath, + }); + let result = null; + try { + result = JSON.parse(await fs.readFile(configPath, "utf8")); + } catch { + result = null; + } + const verdict = classifyConfigResult(result); + if (result !== null) parsed.set(config.slug, result); + configurationRecords.push({ + slug: config.slug, + features: config.features, + status: verdict.status, + file: configPath, + runId: result?.runId ?? null, + runStatus: result?.status ?? "unattempted", + ...(verdict.error ? { error: verdict.error } : {}), + }); + console.log(`${config.slug}: ${verdict.status}`); + if (verdict.outcome === "stop") stopped = true; + } + const comparisonRecords = []; + for (const edge of edges) { + if (watched.aborted) { + comparisonRecords.push({ ...edge, status: "not-attempted", file: null }); + continue; + } + const from = parsed.get(edge.from); + const to = parsed.get(edge.to); + if (!from || !to) { + comparisonRecords.push({ ...edge, status: "not-attempted", file: null }); + continue; + } + const comparison = tagExpectedFeatureDiff( + await compareFn(from, to, { diagnostic: true }), + edge.toggledFeature, + ); + // An edge is controlled when the intended feature toggle is the only + // mismatch. Anything else (environment, seeds, manifest defaults shifting + // mid-matrix) keeps the strong diagnostic-only reading. + const controlled = + comparison.mismatches?.every((entry) => entry.expected === true) ?? false; + const comparisonPath = path.join( + outputRoot, + "comparisons", + `${edge.from}__${edge.to}.json`, + ); + await save(comparisonPath, { ...comparison, edge, controlled }); + console.log(`${edge.from}__${edge.to}: ${comparisonPath}`); + if (!controlled) + console.warn( + `${edge.from}__${edge.to}: unexpected mismatches beyond the intended feature toggle`, + ); + comparisonRecords.push({ + ...edge, + status: "compared", + file: comparisonPath, + compatible: comparison.compatible, + diagnostic: comparison.diagnostic, + controlled, + }); + } + const status = + configurationRecords.every((record) => + ["complete", "failed"].includes(record.status), + ) && comparisonRecords.every((record) => record.status === "compared") + ? "complete" + : "failed"; + await save(path.join(outputRoot, "matrix.json"), { + schemaVersion: 1, + abId, + timestamp: new Date().toISOString(), + features: options.features, + maxFeatures: options.maxFeatures, + filter: options.filter, + outputRoot, + configurations: configurationRecords, + comparisons: comparisonRecords, + status, + warnings: [LIMITS], + }); + console.log(`Matrix: ${path.join(outputRoot, "matrix.json")}`); + // ab runs once per process; release before the single return. An unexpected + // throw past this point exits through main's handler with the listeners. + releaseSignal?.(); + return status === "complete" ? 0 : 1; +} + export async function main(argv = process.argv.slice(2)) { const options = readOptions(argv); if (options.help) { console.log( - "Renderer benchmarks\n node scripts/renderer-benchmarks.js run [--filter ID] [--seed N] [--warmups N] [--repetitions N] [--timeout-ms N] [--headed] [--base-url URL] [--output FILE]\n node scripts/renderer-benchmarks.js compare BASELINE CANDIDATE [--diagnostic] [--output FILE]\nRun requires a prepared development container and an existing static server on port 3000. Defaults: 3 warmups, 10 measured attempts, 30s timeout; provisional pending calibration.\n" + + "Renderer benchmarks\n node scripts/renderer-benchmarks.js run [--filter ID] [--seed N] [--warmups N] [--repetitions N] [--timeout-ms N] [--headed] [--base-url URL] [--features a,b] [--output FILE]\n node scripts/renderer-benchmarks.js ab --features a,b [--max-features N] [--confirm] [--dry-run] [--filter ID] [--seed N] [--warmups N] [--repetitions N] [--timeout-ms N] [--headed] [--base-url URL] [--output DIR]\n node scripts/renderer-benchmarks.js compare BASELINE CANDIDATE [--diagnostic] [--output FILE]\nRun requires a prepared development container and an existing static server on port 3000. Defaults: 3 warmups, 10 measured attempts, 30s timeout; provisional pending calibration.\n" + LIMITS, ); return 0; @@ -552,6 +921,7 @@ export async function main(argv = process.argv.slice(2)) { if (options.output) await save(path.resolve(options.output), comparison); return 0; } + if (options.command === "ab") return ab(options); return run(options); } diff --git a/frontend/scripts/renderer-benchmarks/compare.js b/frontend/scripts/renderer-benchmarks/compare.js index 53dbb98a08..0741a7ce9f 100644 --- a/frontend/scripts/renderer-benchmarks/compare.js +++ b/frontend/scripts/renderer-benchmarks/compare.js @@ -16,8 +16,8 @@ function requireValid(condition, message) { export function validateResult(result) { requireValid( - record(result) && result.schemaVersion === 1, - "expected schemaVersion 1", + record(result) && result.schemaVersion === 2, + "expected schemaVersion 2", ); requireValid(record(result.metadata), "metadata is required"); for (const field of ["environment", "configuration"]) @@ -135,6 +135,20 @@ export class CompatibilityError extends Error { } } +// Tags the expected per-edge feature difference of a feature-matrix comparison +// without suppressing it: the `metadata.build.features` mismatch stays listed +// but carries the toggling feature. Mutates the comparison's mismatch entries +// in place; no I/O. +export function tagExpectedFeatureDiff(comparison, toggledFeature) { + for (const entry of comparison.mismatches) { + if (entry.path === "metadata.build.features") { + entry.expected = true; + entry.toggledFeature = toggledFeature; + } + } + return comparison; +} + export function compareRuns( baseline, candidate, @@ -145,7 +159,14 @@ export function compareRuns( const mismatches = []; // Compare renderer revisions, but never silently mix debug/profiling builds // with ordinary release measurements. - for (const field of ["mode", "features", "target"]) { + for (const field of [ + "mode", + "features", + "target", + "defaultFeatures", + "env", + "ambientEnv", + ]) { differences( baseline.metadata.build?.[field], candidate.metadata.build?.[field], diff --git a/frontend/scripts/renderer-benchmarks/tests/compare.test.js b/frontend/scripts/renderer-benchmarks/tests/compare.test.js index fe6784dcf5..063275b0b3 100644 --- a/frontend/scripts/renderer-benchmarks/tests/compare.test.js +++ b/frontend/scripts/renderer-benchmarks/tests/compare.test.js @@ -1,14 +1,23 @@ import assert from "node:assert/strict"; import test from "node:test"; import { compareRuns, formatComparison } from "../compare.js"; +import { tagExpectedFeatureDiff } from "../compare.js"; function run(value = 10) { return { - schemaVersion: 1, + schemaVersion: 2, status: "complete", metadata: { git: { sha: "before", dirty: false }, - build: { mode: "release", features: [] }, + build: { + mode: "release", + featurePolicy: "manifest-defaults", + features: [], + defaultFeatures: ["default"], + target: "frontend", + env: { BUILD_MODE: "release" }, + ambientEnv: { CARGO_HOME: "/home/penpot/.cargo" }, + }, environment: { chromium: "123", headless: true }, configuration: { viewport: { width: 1920, height: 1080 }, dpr: 2 }, }, @@ -167,7 +176,8 @@ test("offline comparison rejects malformed results without diagnostic bypass", ( for (const invalid of [ null, {}, - { schemaVersion: 2 }, + { schemaVersion: 1 }, + { schemaVersion: 3 }, missingEnvironment, duplicateCase, invalidMetric, @@ -190,3 +200,41 @@ test("text comparisons display changes, intervals and incompatibilities", () => assert.match(report, /change 2\.000 \(20\.000%\)/); assert.match(report, /change interval: \[2\.000, 2\.000\]/); }); + +for (const [name, change] of [ + ["default features", (r) => (r.metadata.build.defaultFeatures = [])], + ["build env", (r) => (r.metadata.build.env.BUILD_MODE = "debug")], + ["ambient env", (r) => (r.metadata.build.ambientEnv.CARGO_HOME = "/other")], +]) { + test(`comparison refuses changed ${name} unless diagnostic override is explicit`, () => { + const candidate = run(); + change(candidate); + assert.throws(() => compareRuns(run(), candidate), { + name: "CompatibilityError", + }); + const result = compareRuns(run(), candidate, { diagnostic: true }); + assert.equal(result.compatible, false); + assert.ok(result.mismatches.length > 0); + }); +} + +test("ab edge tags the expected feature mismatch without suppressing it", () => { + const candidate = run(); + candidate.metadata.build.features = ["branch-b"]; + candidate.metadata.build.featurePolicy = "explicit"; + assert.throws(() => compareRuns(run(), candidate), { + name: "CompatibilityError", + }); + const compared = compareRuns(run(), candidate, { diagnostic: true }); + assert.equal(compared.compatible, false); + tagExpectedFeatureDiff(compared, "branch-b"); + const tagged = compared.mismatches.filter((entry) => entry.expected); + assert.equal(tagged.length, 1); + assert.deepEqual(tagged[0], { + path: "metadata.build.features", + baseline: [], + candidate: ["branch-b"], + expected: true, + toggledFeature: "branch-b", + }); +}); diff --git a/frontend/scripts/renderer-benchmarks/tests/runner.test.js b/frontend/scripts/renderer-benchmarks/tests/runner.test.js index 7e35e5ecf4..cdc8293d88 100644 --- a/frontend/scripts/renderer-benchmarks/tests/runner.test.js +++ b/frontend/scripts/renderer-benchmarks/tests/runner.test.js @@ -1,5 +1,8 @@ import test from "node:test"; import assert from "node:assert/strict"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { readOptions, bounded, @@ -7,6 +10,12 @@ import { recordAttempt, finishCases, defaultFeatures, + planFeatureMatrix, + validateFeatureSelection, + recordAmbientEnv, + BUILD_ENV, + classifyConfigResult, + ab, } from "../../renderer-benchmarks.js"; test("build provenance expands the renderer's declared default features", () => { @@ -46,6 +55,557 @@ test("CLI validates bounded numbers, commands and URLs before execution", () => ); }); +test("feature matrix plans Gray-ordered configurations and lattice edges", () => { + assert.deepEqual(planFeatureMatrix(["branch-b"]), { + configurations: [ + { slug: "0", features: [] }, + { slug: "1", features: ["branch-b"] }, + ], + edges: [{ from: "0", to: "1", toggledFeature: "branch-b" }], + }); + const two = planFeatureMatrix(["a", "b"]); + assert.deepEqual( + two.configurations.map((c) => c.slug), + ["00", "01", "11", "10"], + ); + assert.deepEqual( + two.configurations.map((c) => c.features), + [[], ["b"], ["a", "b"], ["a"]], + ); + assert.deepEqual( + two.edges.map((e) => `${e.from}__${e.to}:${e.toggledFeature}`).sort(), + ["00__01:b", "00__10:a", "01__11:a", "10__11:b"], + ); + const three = planFeatureMatrix(["a", "b", "c"]); + assert.equal(three.configurations.length, 8); + assert.equal(three.edges.length, 12); + assert.equal(new Set(three.edges.map((e) => `${e.from}__${e.to}`)).size, 12); + for (const config of three.configurations) { + assert.equal( + three.edges.filter((e) => e.from === config.slug || e.to === config.slug) + .length, + 3, + ); + } +}); + +test("ab help needs no features and dry run is ab-only", async () => { + assert.equal(readOptions(["ab", "--help"]).command, "ab"); + assert.equal(await main(["ab", "--help"]), 0); + for (const args of [ + ["run", "--dry-run"], + ["compare", "a.json", "b.json", "--dry-run"], + ]) { + assert.throws(() => readOptions(args), /dry-run/); + } +}); + +test("CLI parses feature selection for run and ab", () => { + assert.deepEqual(readOptions(["run"]).features, []); + assert.deepEqual(readOptions(["run", "--features", "b,a"]).features, [ + "a", + "b", + ]); + assert.deepEqual(readOptions(["run", "--features", "a,a,, b "]).features, [ + "a", + "b", + ]); + assert.deepEqual(readOptions(["run", "--features", "profile"]).features, [ + "profile", + ]); + const ab = readOptions(["ab", "--features", "branch-b"]); + assert.deepEqual(ab.features, ["branch-b"]); + assert.equal(ab.maxFeatures, 3); + assert.equal(ab.confirm, false); + assert.equal(ab.dryRun, false); + assert.equal( + readOptions(["ab", "--features", "a", "--confirm", "--dry-run"]).confirm, + true, + ); + for (const args of [ + ["ab"], + ["ab", "--features", ""], + ["ab", "--features", "a", "--max-features", "0"], + ["ab", "--features", "a", "--max-features", "7"], + ["ab", "--features", "Bad_Name"], + ["ab", "--features", "profile"], + ["ab", "--features", "stats-foo"], + ["ab", "--features", "a", "extra.json"], + ["run", "--features", "a", "extra.json"], + ]) { + assert.throws(() => readOptions(args)); + } +}); + +test("feature selection validates against the renderer manifest", () => { + const definitions = { default: [], "branch-a": [], "branch-b": [] }; + assert.deepEqual(validateFeatureSelection(definitions, ["branch-b"], "ab"), [ + "default", + ]); + assert.deepEqual(validateFeatureSelection(definitions, [], "run"), [ + "default", + ]); + assert.throws( + () => validateFeatureSelection({ default: ["branch-b"] }, [], "ab"), + /empty default feature set/, + ); + assert.deepEqual( + validateFeatureSelection({ default: ["branch-b"] }, [], "run"), + ["branch-b", "default"], + ); + assert.throws( + () => validateFeatureSelection(definitions, ["nope"], "run"), + /Unknown Cargo feature/, + ); + for (const command of ["run", "ab"]) { + assert.throws( + () => + validateFeatureSelection( + { default: ["shared"], shared: [] }, + ["shared"], + command, + ), + /already enabled by default/, + ); + } +}); + +test("build environment recording excludes volatile values", () => { + assert.equal("VERSION" in BUILD_ENV, false); + assert.equal(BUILD_ENV.PENPOT_WASM_FUNCTION_NAMES, "0"); + assert.deepEqual( + recordAmbientEnv({ + RUSTFLAGS: "--deny warnings", + CARGO_HOME: "/home/penpot/.cargo", + CARGO_PROFILE_RELEASE_DEBUG: "true", + EMSDK: "/opt/emsdk", + EMSDK_CLANG: "clang", + PATH: "/usr/bin", + VERSION: "volatile", + PENPOT_WASM_PREPARED: "1", + }), + { + RUSTFLAGS: "--deny warnings", + CARGO_HOME: "/home/penpot/.cargo", + CARGO_PROFILE_RELEASE_DEBUG: "true", + EMSDK: "/opt/emsdk", + EMSDK_CLANG: "clang", + }, + ); +}); + +test("classifier separates continuation from matrix stops", () => { + const complete = { + status: "complete", + metadata: { build: { durationMs: 5 } }, + cases: [{ attempts: [{}] }], + }; + assert.deepEqual(classifyConfigResult(complete), { + outcome: "continue", + status: "complete", + }); + assert.deepEqual(classifyConfigResult({ ...complete, status: "failed" }), { + outcome: "continue", + status: "failed", + }); + assert.deepEqual(classifyConfigResult(null), { + outcome: "stop", + status: "unattempted", + error: "configuration output is missing or invalid", + }); + assert.deepEqual(classifyConfigResult({ status: "interrupted" }), { + outcome: "stop", + status: "interrupted", + }); + assert.deepEqual( + classifyConfigResult({ + status: "failed", + metadata: { build: {} }, + cases: [], + }), + { + outcome: "stop", + status: "failed", + error: "build or preflight failed before any suite ran", + }, + ); + assert.deepEqual( + classifyConfigResult({ + status: "failed", + metadata: { build: { durationMs: 5 } }, + cases: [], + }), + { + outcome: "stop", + status: "failed", + error: "suite recorded no attempts", + }, + ); +}); + +const abDefinitions = { default: [], "branch-a": [], "branch-b": [] }; + +async function abTempRoot() { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "renderer-ab-test-")); + return path.join(tmp, "matrix"); +} + +function abOptions(argv) { + return readOptions(argv); +} + +function stubManifest(definitions = abDefinitions) { + return async () => definitions; +} + +function stubRun(failSlugs = new Set()) { + const calls = []; + const runFn = async (options) => { + calls.push(options); + const slug = path.basename(options.output, ".json"); + const failed = failSlugs.has(slug); + const result = failed + ? { + schemaVersion: 2, + runId: `run-${slug}`, + status: "failed", + error: "./build failed (1)", + metadata: { build: { mode: "release", features: [] } }, + cases: [], + } + : { + schemaVersion: 2, + runId: `run-${slug}`, + status: "complete", + metadata: { + build: { + mode: "release", + features: options.features, + durationMs: 5, + }, + }, + cases: [ + { + id: "rects/default/pan", + status: "complete", + attempts: [{ index: 0, warmup: false, status: "ok" }], + }, + ], + }; + await fs.mkdir(path.dirname(options.output), { recursive: true }); + await fs.writeFile(options.output, JSON.stringify(result)); + return failed ? 1 : 0; + }; + return { calls, runFn }; +} + +function stubCompare(extra = []) { + const calls = []; + const compareFn = async (baseline, candidate, { diagnostic } = {}) => { + calls.push({ baseline, candidate, diagnostic }); + return { + schemaVersion: 1, + compatible: false, + diagnostic: diagnostic ?? false, + mismatches: [ + { + path: "metadata.build.features", + baseline: baseline.metadata.build.features, + candidate: candidate.metadata.build.features, + }, + ...extra, + ], + warnings: [], + cases: [], + }; + }; + return { calls, compareFn }; +} + +test("ab refuses an existing output root before any preflight", async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "renderer-ab-test-")); + const nonEmpty = path.join(tmp, "full"); + await fs.mkdir(nonEmpty); + await fs.writeFile(path.join(nonEmpty, "x"), "x"); + const filePath = path.join(tmp, "file.json"); + await fs.writeFile(filePath, "{}"); + const mustNotRun = () => { + throw new Error("must not be called"); + }; + for (const output of [nonEmpty, filePath]) { + await assert.rejects( + ab(abOptions(["ab", "--features", "branch-b", "--output", output]), { + runFn: mustNotRun, + compareFn: mustNotRun, + manifestFn: mustNotRun, + }), + /already exists|non-empty/, + ); + } + await fs.rm(tmp, { recursive: true, force: true }); +}); + +test("ab dry run prints the plan without building", async () => { + const output = await abTempRoot(); + const mustNotRun = () => { + throw new Error("must not be called"); + }; + const code = await ab( + abOptions([ + "ab", + "--features", + "branch-b", + "--filter", + "rects/default/pan", + "--dry-run", + "--output", + output, + ]), + { runFn: mustNotRun, compareFn: mustNotRun, manifestFn: stubManifest() }, + ); + assert.equal(code, 0); + await assert.rejects(fs.access(output), /ENOENT/); + await fs.rm(path.dirname(output), { recursive: true, force: true }); +}); + +test("ab confirm gate blocks wide matrices and empty filters", async () => { + const runFn = () => { + throw new Error("must not be called"); + }; + const wide = await abTempRoot(); + const narrow = await abTempRoot(); + await assert.rejects( + ab( + abOptions([ + "ab", + "--features", + "f1,f2,f3,f4", + "--max-features", + "4", + "--filter", + "rects", + "--output", + wide, + ]), + { + runFn, + manifestFn: stubManifest({ + default: [], + f1: [], + f2: [], + f3: [], + f4: [], + }), + }, + ), + /confirm/, + ); + await assert.rejects( + ab(abOptions(["ab", "--features", "branch-b", "--output", narrow]), { + runFn, + manifestFn: stubManifest(), + }), + /confirm/, + ); + await fs.rm(path.dirname(wide), { recursive: true, force: true }); + await fs.rm(path.dirname(narrow), { recursive: true, force: true }); +}); + +test("ab runs the matrix and writes configs, comparisons and index", async () => { + const output = await abTempRoot(); + const { calls: runCalls, runFn } = stubRun(); + const { calls: compareCalls, compareFn } = stubCompare(); + const before = process.listenerCount("SIGINT"); + const code = await ab( + abOptions([ + "ab", + "--features", + "branch-b", + "--filter", + "rects/default/pan", + "--output", + output, + ]), + { runFn, compareFn, manifestFn: stubManifest() }, + ); + assert.equal(code, 0); + assert.equal(process.listenerCount("SIGINT"), before); + assert.deepEqual( + runCalls.map((options) => options.features), + [[], ["branch-b"]], + ); + assert.equal(compareCalls.length, 1); + assert.equal(compareCalls[0].diagnostic, true); + const first = JSON.parse( + await fs.readFile(path.join(output, "configs", "0.json"), "utf8"), + ); + assert.equal(first.runId, "run-0"); + const comparison = JSON.parse( + await fs.readFile(path.join(output, "comparisons", "0__1.json"), "utf8"), + ); + assert.deepEqual(comparison.edge, { + from: "0", + to: "1", + toggledFeature: "branch-b", + }); + assert.deepEqual( + comparison.mismatches.filter((entry) => entry.expected), + [ + { + path: "metadata.build.features", + baseline: [], + candidate: ["branch-b"], + expected: true, + toggledFeature: "branch-b", + }, + ], + ); + const matrix = JSON.parse( + await fs.readFile(path.join(output, "matrix.json"), "utf8"), + ); + assert.equal(matrix.schemaVersion, 1); + assert.deepEqual(matrix.features, ["branch-b"]); + assert.deepEqual( + matrix.configurations.map((c) => c.status), + ["complete", "complete"], + ); + assert.deepEqual( + matrix.comparisons.map((c) => [c.from, c.to, c.status]), + [["0", "1", "compared"]], + ); + assert.equal(matrix.status, "complete"); + assert.equal(comparison.controlled, true); + assert.equal(matrix.comparisons[0].controlled, true); + await fs.rm(path.dirname(output), { recursive: true, force: true }); +}); + +test("ab marks edges with unexpected diffs uncontrolled", async () => { + const output = await abTempRoot(); + const { runFn } = stubRun(); + const { compareFn } = stubCompare([ + { path: "metadata.build.env", baseline: {}, candidate: {} }, + ]); + const code = await ab( + abOptions([ + "ab", + "--features", + "branch-b", + "--filter", + "rects/default/pan", + "--output", + output, + ]), + { runFn, compareFn, manifestFn: stubManifest() }, + ); + assert.equal(code, 0); + const comparison = JSON.parse( + await fs.readFile(path.join(output, "comparisons", "0__1.json"), "utf8"), + ); + assert.equal(comparison.controlled, false); + const matrix = JSON.parse( + await fs.readFile(path.join(output, "matrix.json"), "utf8"), + ); + assert.equal(matrix.comparisons[0].controlled, false); + await fs.rm(path.dirname(output), { recursive: true, force: true }); +}); + +test("ab interruption between configurations still writes the matrix", async () => { + const output = await abTempRoot(); + const controller = new AbortController(); + const { runFn } = stubRun(); + const interruptAfterFirst = async (options) => { + const code = await runFn(options); + controller.abort(new Error("stopped")); + return code; + }; + const { compareFn } = stubCompare(); + const code = await ab( + abOptions([ + "ab", + "--features", + "a,b", + "--max-features", + "2", + "--filter", + "rects", + "--confirm", + "--output", + output, + ]), + { + runFn: interruptAfterFirst, + compareFn, + manifestFn: stubManifest({ default: [], a: [], b: [] }), + signal: controller.signal, + }, + ); + assert.equal(code, 1); + const matrix = JSON.parse( + await fs.readFile(path.join(output, "matrix.json"), "utf8"), + ); + assert.deepEqual( + matrix.configurations.map((c) => [c.slug, c.status]), + [ + ["00", "complete"], + ["01", "unattempted"], + ["11", "unattempted"], + ["10", "unattempted"], + ], + ); + assert.ok(matrix.comparisons.every((c) => c.status === "not-attempted")); + assert.equal(matrix.status, "failed"); + await fs.rm(path.dirname(output), { recursive: true, force: true }); +}); + +test("ab stops on build failure and marks the rest unattempted", async () => { + const output = await abTempRoot(); + const { runFn } = stubRun(new Set(["01"])); + const { compareFn } = stubCompare(); + const code = await ab( + abOptions([ + "ab", + "--features", + "b,a", + "--max-features", + "2", + "--filter", + "rects", + "--confirm", + "--output", + output, + ]), + { + runFn, + compareFn, + manifestFn: stubManifest({ default: [], a: [], b: [] }), + }, + ); + assert.equal(code, 1); + const matrix = JSON.parse( + await fs.readFile(path.join(output, "matrix.json"), "utf8"), + ); + assert.deepEqual( + matrix.configurations.map((c) => [c.slug, c.status]), + [ + ["00", "complete"], + ["01", "failed"], + ["11", "unattempted"], + ["10", "unattempted"], + ], + ); + assert.deepEqual( + matrix.comparisons.map((c) => [c.from, c.to, c.status]), + [ + ["00", "10", "not-attempted"], + ["00", "01", "compared"], + ["01", "11", "not-attempted"], + ["10", "11", "not-attempted"], + ], + ); + assert.equal(matrix.status, "failed"); + await fs.rm(path.dirname(output), { recursive: true, force: true }); +}); + test("case collector retains a single scene seed across lifecycle and interaction groups", () => { const cases = collectCases(scenarios, 42); assert.equal(cases.length, 21);