Skip to content

Commit 9c734b8

Browse files
VivswanVivswan
authored
fix(e2e): refuse a bad flag at the runner's entry instead of running everything, through node:util parseArgs (#567)
1 parent eee29eb commit 9c734b8

2 files changed

Lines changed: 203 additions & 14 deletions

File tree

‎test/e2e/run.test.ts‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { expect, test } from "bun:test";
2+
import { spawn } from "node:child_process";
3+
import { join } from "node:path";
4+
import { ROOT } from "../root.js";
5+
import { withTempDir } from "../temp-dir.js";
6+
7+
const RUN = join(ROOT, "test", "e2e", "run.ts");
8+
const USAGE = "the flags are --sections <key,key|all>, --scenario <name>, --print-stdout";
9+
10+
/** A runner that does not refuse at the entry is into the corpus with a node child under it, so the bound kills the
11+
* process group, not one pid, and the scenario directories it leaves land in the case's own TMPDIR. */
12+
function run(
13+
args: string[],
14+
): Promise<{ exitCode: number | string | null; stdout: string; stderr: string }> {
15+
return withTempDir(
16+
"run-flags-",
17+
(tmp) =>
18+
new Promise((resolve) => {
19+
const child = spawn(process.execPath, [RUN, ...args], {
20+
cwd: ROOT,
21+
env: { ...process.env, TMPDIR: tmp },
22+
stdio: ["ignore", "pipe", "pipe"],
23+
detached: true,
24+
});
25+
let stdout = "";
26+
let stderr = "";
27+
child.stdout.setEncoding("utf8").on("data", (chunk: string) => {
28+
stdout += chunk;
29+
});
30+
child.stderr.setEncoding("utf8").on("data", (chunk: string) => {
31+
stderr += chunk;
32+
});
33+
const bound = setTimeout(() => {
34+
if (child.pid !== undefined) {
35+
process.kill(-child.pid, "SIGKILL");
36+
}
37+
}, 10_000);
38+
child.on("close", (code, signal) => {
39+
clearTimeout(bound);
40+
resolve({ exitCode: code ?? signal, stdout, stderr });
41+
});
42+
}),
43+
);
44+
}
45+
46+
// The hand-rolled parser these replaced ignored every argument it did not recognize, so a typo'd flag, an inline
47+
// `--sections=labels`, or a stray word ran the whole corpus for minutes and exited green. Each case is an input
48+
// the shell cannot refuse for the script.
49+
test.each<[label: string, args: string[], error: string]>([
50+
["--sections with no value", ["--sections"], `--sections needs a value; ${USAGE}`],
51+
["--scenario with no value", ["--scenario"], `--scenario needs a value; ${USAGE}`],
52+
[
53+
"--sections with an empty list",
54+
["--sections", " , "],
55+
`--sections got an empty list; name at least one section, or all; ${USAGE}`,
56+
],
57+
["a misspelled flag", ["--section", "labels"], `unknown flag --section; ${USAGE}`],
58+
// parseArgs hands a string flag the next word whatever it is, so a "--" or a flag standing where the value
59+
// belongs would otherwise become the value: here the later "all" would clear the filter and run the corpus.
60+
[
61+
"--sections swallowing a --",
62+
["--sections", "--", "--sections", "all"],
63+
`--sections needs a value; ${USAGE}`,
64+
],
65+
[
66+
"--sections swallowing the next flag",
67+
["--sections", "--print-stdout"],
68+
`--sections needs a value; ${USAGE}`,
69+
],
70+
["a stray argument", ["labels"], `unexpected argument "labels"; ${USAGE}`],
71+
// bun drops a "--" that directly follows the script path, so the row puts a flag ahead of it.
72+
[
73+
"a -- and the flags after it",
74+
["--print-stdout", "--", "--sections", "labels"],
75+
`unexpected argument "--"; ${USAGE}`,
76+
],
77+
[
78+
"--print-stdout with a value",
79+
["--print-stdout=yes"],
80+
`--print-stdout takes no value; ${USAGE}`,
81+
],
82+
])(
83+
"%s is refused at the entry, naming the flag and the fix",
84+
async (_label, args, error) => {
85+
expect(await run(args)).toEqual({ exitCode: 2, stdout: "", stderr: `e2e: ${error}\n` });
86+
},
87+
20_000,
88+
);
89+
90+
// A filter nothing matches exits 1 before any scenario runs, and its message echoes the filters as parsed, so the
91+
// two parsings the old code got wrong (an inline value, a later `all`) are pinned without running the corpus.
92+
test.each<[label: string, args: string[], filters: string]>([
93+
[
94+
"--sections=<key> is read as the flag's value",
95+
["--sections=no-such-section"],
96+
'--sections "no-such-section"',
97+
],
98+
[
99+
"a later --sections all clears the list",
100+
["--scenario", "no-such-scenario", "--sections", "labels", "--sections", "all"],
101+
'--scenario "no-such-scenario"',
102+
],
103+
])(
104+
"%s",
105+
async (_label, args, filters) => {
106+
const result = await run(args);
107+
expect([result.exitCode, result.stdout]).toEqual([1, ""]);
108+
expect(result.stderr).toMatch(
109+
new RegExp(
110+
`^none of the \\d+ scenario\\(s\\) under .* match ${filters.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}; check`,
111+
),
112+
);
113+
},
114+
20_000,
115+
);

‎test/e2e/run.ts‎

Lines changed: 88 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,38 @@
99
* runner registers no mask, so the echo redacts on its own); a failure's
1010
* artifact directory holds the whole of it anyway, a pass otherwise leaves no
1111
* trace of it
12+
*
13+
* An unknown flag, a stray argument, a flag without its value, or an empty section list is refused at
14+
* the entry with exit 2, before any scenario runs.
1215
*/
1316

17+
import { parseArgs } from "node:util";
18+
import { quote } from "../../src/text.js";
1419
import { corpusUnwitnessedExemptEndpoints } from "./runner/apply-idempotence-proof.js";
1520
import { indentedStdout, runScenario, type ScenarioReport } from "./runner/runner.js";
1621
import { loadScenarios, type Scenario, scenarioRoots } from "./scenario.js";
1722

23+
const FLAGS = {
24+
sections: { type: "string" },
25+
scenario: { type: "string" },
26+
"print-stdout": { type: "boolean" },
27+
} as const;
28+
const USAGE = "the flags are --sections <key,key|all>, --scenario <name>, --print-stdout";
29+
1830
interface Flags {
1931
sections?: string[];
2032
scenario?: string;
2133
printStdout?: boolean;
2234
}
2335

36+
type ParsedFlags = { ok: true; flags: Flags } | { ok: false; error: string };
37+
38+
/** parseArgs's option token, which node:util declares but does not export. */
39+
type OptionToken = Extract<
40+
ReturnType<typeof parseArgs<{ tokens: true; strict: false }>>["tokens"][number],
41+
{ kind: "option" }
42+
>;
43+
2444
/** The run's result table and counts, grown one scenario at a time. */
2545
interface Tally {
2646
table: string[];
@@ -29,22 +49,71 @@ interface Tally {
2949
failed: number;
3050
}
3151

32-
function parseFlags(argv: string[]): Flags {
52+
function refuse(error: string): ParsedFlags {
53+
return { ok: false, error: `e2e: ${error}; ${USAGE}` };
54+
}
55+
56+
/** `all` clears an earlier list, so the last `--sections` wins whichever word it carries. */
57+
function applySections(flags: Flags, value: string): string | undefined {
58+
if (value === "all") {
59+
flags.sections = undefined;
60+
return undefined;
61+
}
62+
const keys = value
63+
.split(",")
64+
.map((key) => key.trim())
65+
.filter((key) => key !== "");
66+
if (keys.length === 0) {
67+
return "--sections got an empty list; name at least one section, or all";
68+
}
69+
flags.sections = keys;
70+
return undefined;
71+
}
72+
73+
/** One flag applied to `flags`, or the refusal it earns. */
74+
function applyFlag(flags: Flags, token: OptionToken): string | undefined {
75+
if (!Object.hasOwn(FLAGS, token.name)) {
76+
return `unknown flag ${token.rawName}`;
77+
}
78+
if (token.name === "print-stdout") {
79+
if (token.value !== undefined) {
80+
return "--print-stdout takes no value";
81+
}
82+
flags.printStdout = true;
83+
return undefined;
84+
}
85+
// parseArgs hands a string flag the next word whatever it is, so a flag or "--" standing where the value
86+
// belongs is the value missing, not a value; an inline "--scenario=-name" keeps its dash.
87+
if (token.value === undefined || (!token.inlineValue && token.value.startsWith("-"))) {
88+
return `${token.rawName} needs a value`;
89+
}
90+
if (token.name === "scenario") {
91+
flags.scenario = token.value;
92+
return undefined;
93+
}
94+
return applySections(flags, token.value);
95+
}
96+
97+
function parseFlags(argv: readonly string[]): ParsedFlags {
98+
const { tokens } = parseArgs({
99+
args: [...argv],
100+
options: FLAGS,
101+
strict: false,
102+
allowPositionals: true,
103+
tokens: true,
104+
});
33105
const flags: Flags = {};
34-
for (let i = 0; i < argv.length; i++) {
35-
const arg = argv[i];
36-
if (arg === "--sections") {
37-
const value = argv[++i] ?? "";
38-
if (value && value !== "all") {
39-
flags.sections = value.split(",").map((s) => s.trim());
40-
}
41-
} else if (arg === "--scenario") {
42-
flags.scenario = argv[++i];
43-
} else if (arg === "--print-stdout") {
44-
flags.printStdout = true;
106+
for (const token of tokens) {
107+
if (token.kind !== "option") {
108+
const argument = token.kind === "positional" ? token.value : "--";
109+
return refuse(`unexpected argument ${quote(argument)}`);
110+
}
111+
const problem = applyFlag(flags, token);
112+
if (problem !== undefined) {
113+
return refuse(problem);
45114
}
46115
}
47-
return flags;
116+
return { ok: true, flags };
48117
}
49118

50119
/**
@@ -169,7 +238,12 @@ function addCorpusWitness(tally: Tally): void {
169238
}
170239

171240
async function main(): Promise<number> {
172-
const flags = parseFlags(process.argv.slice(2));
241+
const parsed = parseFlags(process.argv.slice(2));
242+
if (!parsed.ok) {
243+
console.error(parsed.error);
244+
return 2;
245+
}
246+
const { flags } = parsed;
173247
const roots = scenarioRoots();
174248
const all = loadScenarios(roots);
175249
const scenarios = selectScenarios(all, flags);

0 commit comments

Comments
 (0)