From 5addb147529d0e122f4831edf82439d1647749b1 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:45:09 -0400 Subject: [PATCH 1/2] fix(e2e): refuse a bad runner flag at the entry and read every form parseArgs does The curated runner's parseFlags walked argv by hand and ignored whatever it did not recognize: a typo'd flag, a stray word, an inline `--sections=labels` or `--scenario=name`, a flag with no value, or an empty `--sections ""` ran the whole corpus for minutes and exited green, and `--` was skipped so the flags after it were read. node:util's parseArgs, which test/e2e/fuzz.ts already uses, reads the three flags in every form it defines, and the runner now refuses the rest at the entry with exit 2, naming the argument and the usage line, as the fuzzer does. Answers that change, each an input a developer can type and none the repository itself passes: - `--sections=labels` and `--scenario=name` are read as the flag's value; the old parser ignored both and ran everything. - `--sections labels --sections all` ends with no filter: `all` clears the list, so the last flag wins whichever word it carries; the old parser kept `labels` because `all` only ever meant "do not set". - A `--` and the arguments after it are refused as unexpected; the old parser skipped the `--` and read the flags after it. bun itself drops a `--` that directly follows the script path, so `bun test/e2e/run.ts -- --sections labels` reaches the script without it and filters as before, while a `--` after a flag reaches the script and is refused. - An unknown flag, a stray word, a flag with no value, `--print-stdout=yes`, and an empty or all-commas `--sections` are refused; the old parser ran everything, or filtered on an empty key and found nothing. Differential check, the old parser against the new: every invocation the repository makes (`bun run test:e2e` from package.json, checks.yml, and CONTRIBUTING; the `--scenario ` replay lines the runner writes; the header's `--sections a,b`, `--sections all`, `--print-stdout`, and their pairings) parses identically. Of 6000 generated argv shapes over the flags, their inline forms, `all`, `--`, stray words, misspellings, and empty values, 149 parse identically and 5851 differ only as listed above: 5770 by a refusal where the old parser accepted silently, 81 by an inline value or a later `all` now honored. The repeated `--sections` forms the old parser got right (`all` first, or two lists) still parse the same. The new test/e2e/run.test.ts spawns the runner on each refused shape and on the two honored shapes a filter nothing matches exposes through its message, so neither can regress to a silent full run. --- test/e2e/run.test.ts | 103 +++++++++++++++++++++++++++++++++++++++++++ test/e2e/run.ts | 100 +++++++++++++++++++++++++++++++++++------ 2 files changed, 189 insertions(+), 14 deletions(-) create mode 100644 test/e2e/run.test.ts diff --git a/test/e2e/run.test.ts b/test/e2e/run.test.ts new file mode 100644 index 00000000..86a8b131 --- /dev/null +++ b/test/e2e/run.test.ts @@ -0,0 +1,103 @@ +import { expect, test } from "bun:test"; +import { spawn } from "node:child_process"; +import { join } from "node:path"; +import { ROOT } from "../root.js"; +import { withTempDir } from "../temp-dir.js"; + +const RUN = join(ROOT, "test", "e2e", "run.ts"); +const USAGE = "the flags are --sections , --scenario , --print-stdout"; + +/** A runner that does not refuse at the entry is into the corpus with a node child under it, so the bound kills the + * process group, not one pid, and the scenario directories it leaves land in the case's own TMPDIR. */ +function run( + args: string[], +): Promise<{ exitCode: number | string | null; stdout: string; stderr: string }> { + return withTempDir( + "run-flags-", + (tmp) => + new Promise((resolve) => { + const child = spawn(process.execPath, [RUN, ...args], { + cwd: ROOT, + env: { ...process.env, TMPDIR: tmp }, + stdio: ["ignore", "pipe", "pipe"], + detached: true, + }); + let stdout = ""; + let stderr = ""; + child.stdout.setEncoding("utf8").on("data", (chunk: string) => { + stdout += chunk; + }); + child.stderr.setEncoding("utf8").on("data", (chunk: string) => { + stderr += chunk; + }); + const bound = setTimeout(() => { + if (child.pid !== undefined) { + process.kill(-child.pid, "SIGKILL"); + } + }, 10_000); + child.on("close", (code, signal) => { + clearTimeout(bound); + resolve({ exitCode: code ?? signal, stdout, stderr }); + }); + }), + ); +} + +// The hand-rolled parser these replaced ignored every argument it did not recognize, so a typo'd flag, an inline +// `--sections=labels`, or a stray word ran the whole corpus for minutes and exited green. Each case is an input +// the shell cannot refuse for the script. +test.each<[label: string, args: string[], error: string]>([ + ["--sections with no value", ["--sections"], `--sections needs a value; ${USAGE}`], + ["--scenario with no value", ["--scenario"], `--scenario needs a value; ${USAGE}`], + [ + "--sections with an empty list", + ["--sections", " , "], + `--sections got an empty list; name at least one section, or all; ${USAGE}`, + ], + ["a misspelled flag", ["--section", "labels"], `unknown flag --section; ${USAGE}`], + ["a stray argument", ["labels"], `unexpected argument "labels"; ${USAGE}`], + // bun drops a "--" that directly follows the script path, so the row puts a flag ahead of it. + [ + "a -- and the flags after it", + ["--print-stdout", "--", "--sections", "labels"], + `unexpected argument "--"; ${USAGE}`, + ], + [ + "--print-stdout with a value", + ["--print-stdout=yes"], + `--print-stdout takes no value; ${USAGE}`, + ], +])( + "%s is refused at the entry, naming the flag and the fix", + async (_label, args, error) => { + expect(await run(args)).toEqual({ exitCode: 2, stdout: "", stderr: `e2e: ${error}\n` }); + }, + 20_000, +); + +// A filter nothing matches exits 1 before any scenario runs, and its message echoes the filters as parsed, so the +// two parsings the old code got wrong (an inline value, a later `all`) are pinned without running the corpus. +test.each<[label: string, args: string[], filters: string]>([ + [ + "--sections= is read as the flag's value", + ["--sections=no-such-section"], + '--sections "no-such-section"', + ], + [ + "a later --sections all clears the list", + ["--scenario", "no-such-scenario", "--sections", "labels", "--sections", "all"], + '--scenario "no-such-scenario"', + ], +])( + "%s", + async (_label, args, filters) => { + const result = await run(args); + expect([result.exitCode, result.stdout]).toEqual([1, ""]); + expect(result.stderr).toMatch( + new RegExp( + `^none of the \\d+ scenario\\(s\\) under .* match ${filters.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}; check`, + ), + ); + }, + 20_000, +); diff --git a/test/e2e/run.ts b/test/e2e/run.ts index 01fff3a1..657bdf64 100644 --- a/test/e2e/run.ts +++ b/test/e2e/run.ts @@ -9,18 +9,38 @@ * runner registers no mask, so the echo redacts on its own); a failure's * artifact directory holds the whole of it anyway, a pass otherwise leaves no * trace of it + * + * An unknown flag, a stray argument, a flag without its value, or an empty section list is refused at + * the entry with exit 2, before any scenario runs. */ +import { parseArgs } from "node:util"; +import { quote } from "../../src/text.js"; import { corpusUnwitnessedExemptEndpoints } from "./runner/apply-idempotence-proof.js"; import { indentedStdout, runScenario, type ScenarioReport } from "./runner/runner.js"; import { loadScenarios, type Scenario, scenarioRoots } from "./scenario.js"; +const FLAGS = { + sections: { type: "string" }, + scenario: { type: "string" }, + "print-stdout": { type: "boolean" }, +} as const; +const USAGE = "the flags are --sections , --scenario , --print-stdout"; + interface Flags { sections?: string[]; scenario?: string; printStdout?: boolean; } +type ParsedFlags = { ok: true; flags: Flags } | { ok: false; error: string }; + +/** parseArgs's option token, which node:util declares but does not export. */ +type OptionToken = Extract< + ReturnType>["tokens"][number], + { kind: "option" } +>; + /** The run's result table and counts, grown one scenario at a time. */ interface Tally { table: string[]; @@ -29,22 +49,69 @@ interface Tally { failed: number; } -function parseFlags(argv: string[]): Flags { +function refuse(error: string): ParsedFlags { + return { ok: false, error: `e2e: ${error}; ${USAGE}` }; +} + +/** `all` clears an earlier list, so the last `--sections` wins whichever word it carries. */ +function applySections(flags: Flags, value: string): string | undefined { + if (value === "all") { + flags.sections = undefined; + return undefined; + } + const keys = value + .split(",") + .map((key) => key.trim()) + .filter((key) => key !== ""); + if (keys.length === 0) { + return "--sections got an empty list; name at least one section, or all"; + } + flags.sections = keys; + return undefined; +} + +/** One flag applied to `flags`, or the refusal it earns. */ +function applyFlag(flags: Flags, token: OptionToken): string | undefined { + if (!Object.hasOwn(FLAGS, token.name)) { + return `unknown flag ${token.rawName}`; + } + if (token.name === "print-stdout") { + if (token.value !== undefined) { + return "--print-stdout takes no value"; + } + flags.printStdout = true; + return undefined; + } + if (token.value === undefined) { + return `${token.rawName} needs a value`; + } + if (token.name === "scenario") { + flags.scenario = token.value; + return undefined; + } + return applySections(flags, token.value); +} + +function parseFlags(argv: readonly string[]): ParsedFlags { + const { tokens } = parseArgs({ + args: [...argv], + options: FLAGS, + strict: false, + allowPositionals: true, + tokens: true, + }); const flags: Flags = {}; - for (let i = 0; i < argv.length; i++) { - const arg = argv[i]; - if (arg === "--sections") { - const value = argv[++i] ?? ""; - if (value && value !== "all") { - flags.sections = value.split(",").map((s) => s.trim()); - } - } else if (arg === "--scenario") { - flags.scenario = argv[++i]; - } else if (arg === "--print-stdout") { - flags.printStdout = true; + for (const token of tokens) { + if (token.kind !== "option") { + const argument = token.kind === "positional" ? token.value : "--"; + return refuse(`unexpected argument ${quote(argument)}`); + } + const problem = applyFlag(flags, token); + if (problem !== undefined) { + return refuse(problem); } } - return flags; + return { ok: true, flags }; } /** @@ -169,7 +236,12 @@ function addCorpusWitness(tally: Tally): void { } async function main(): Promise { - const flags = parseFlags(process.argv.slice(2)); + const parsed = parseFlags(process.argv.slice(2)); + if (!parsed.ok) { + console.error(parsed.error); + return 2; + } + const { flags } = parsed; const roots = scenarioRoots(); const all = loadScenarios(roots); const scenarios = selectScenarios(all, flags); From b5675b2b9d3642d1ac2e2d3f7d6e8ad9fb35e45a Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Mon, 26 Oct 2026 16:56:18 -0400 Subject: [PATCH 2/2] fix(e2e): refuse a flag or "--" standing where a runner flag's value belongs parseArgs hands a string flag the next word whatever it is, so `--sections -- --sections all` took `--` as the sections value, the later `all` cleared the filter, and the whole corpus ran instead of exit 2. A non-inline value that starts with `-` is now refused as the value missing (`--sections needs a value; ...`), before `--sections` or `--scenario` applies; an inline `--scenario=-name` still passes. One answer changes with it: `--sections --print-stdout` is refused at the entry, where the hand-rolled loop and the first commit both took the flag as the value and the run then reported that no scenario matched it. Two regression rows pin both shapes. --- test/e2e/run.test.ts | 12 ++++++++++++ test/e2e/run.ts | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/test/e2e/run.test.ts b/test/e2e/run.test.ts index 86a8b131..c8c8efd5 100644 --- a/test/e2e/run.test.ts +++ b/test/e2e/run.test.ts @@ -55,6 +55,18 @@ test.each<[label: string, args: string[], error: string]>([ `--sections got an empty list; name at least one section, or all; ${USAGE}`, ], ["a misspelled flag", ["--section", "labels"], `unknown flag --section; ${USAGE}`], + // parseArgs hands a string flag the next word whatever it is, so a "--" or a flag standing where the value + // belongs would otherwise become the value: here the later "all" would clear the filter and run the corpus. + [ + "--sections swallowing a --", + ["--sections", "--", "--sections", "all"], + `--sections needs a value; ${USAGE}`, + ], + [ + "--sections swallowing the next flag", + ["--sections", "--print-stdout"], + `--sections needs a value; ${USAGE}`, + ], ["a stray argument", ["labels"], `unexpected argument "labels"; ${USAGE}`], // bun drops a "--" that directly follows the script path, so the row puts a flag ahead of it. [ diff --git a/test/e2e/run.ts b/test/e2e/run.ts index 657bdf64..85c51370 100644 --- a/test/e2e/run.ts +++ b/test/e2e/run.ts @@ -82,7 +82,9 @@ function applyFlag(flags: Flags, token: OptionToken): string | undefined { flags.printStdout = true; return undefined; } - if (token.value === undefined) { + // parseArgs hands a string flag the next word whatever it is, so a flag or "--" standing where the value + // belongs is the value missing, not a value; an inline "--scenario=-name" keeps its dash. + if (token.value === undefined || (!token.inlineValue && token.value.startsWith("-"))) { return `${token.rawName} needs a value`; } if (token.name === "scenario") {