fix(e2e): refuse a bad flag at the runner's entry instead of running everything, through node:util parseArgs

test/e2e/run.test.ts

@@ -0,0 +1,115 @@
+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 <key,key|all>, --scenario <name>, --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}`],
+  // 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.
+  [
+    "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=<key> 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,
+);

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 <key,key|all>, --scenario <name>, --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<typeof parseArgs<{ tokens: true; strict: false }>>["tokens"][number],
+  { kind: "option" }
+>;
+
 /** The run's result table and counts, grown one scenario at a time. */
 interface Tally {
   table: string[];
@@ -29,22 +49,71 @@ 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;
+  }
+  // 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") {
+    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 +238,12 @@ function addCorpusWitness(tally: Tally): void {
 }
 
 async function main(): Promise<number> {
-  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);