fix(qa): reject duplicate single-value flags

This commit is contained in:
Vincent Koc
2026-06-23 14:01:29 +02:00
parent 0e091482a3
commit 72816124c9
7 changed files with 108 additions and 3 deletions
+2
View File
@@ -31,7 +31,9 @@ function positiveIntegerFlag(flag, key) {
throw new Error(`${flag} requires a value`);
}
return {
flag,
nextIndex: index + 1,
repeatable: false,
apply(target) {
target[key] = parsePositiveInteger(rawValue, flag);
},
+21 -3
View File
@@ -143,7 +143,9 @@ export function stringFlag(flag, key, options = {}) {
return null;
}
return {
flag,
nextIndex: option.nextIndex,
repeatable: false,
apply(target) {
target[key] = option.value;
},
@@ -161,7 +163,9 @@ export function stringListFlag(flag, key, options = {}) {
return null;
}
return {
flag,
nextIndex: option.nextIndex,
repeatable: true,
apply(target) {
target[key] ??= [];
target[key].push(option.value);
@@ -171,7 +175,7 @@ export function stringListFlag(flag, key, options = {}) {
};
}
function createAssignedValueFlag(consumeOption) {
function createAssignedValueFlag(flag, consumeOption) {
return {
consume(argv, index, args) {
const option = consumeOption(argv, index, args);
@@ -179,7 +183,9 @@ function createAssignedValueFlag(consumeOption) {
return null;
}
return {
flag,
nextIndex: option.nextIndex,
repeatable: false,
apply(target) {
target[option.key] = option.value;
},
@@ -190,7 +196,7 @@ function createAssignedValueFlag(consumeOption) {
/** Create a flag spec that parses and assigns a safe integer value. */
export function intFlag(flag, key, options) {
return createAssignedValueFlag((argv, index) => {
return createAssignedValueFlag(flag, (argv, index) => {
const option = consumeIntFlag(argv, index, flag, options);
return option ? { ...option, key } : null;
});
@@ -198,7 +204,7 @@ export function intFlag(flag, key, options) {
/** Create a flag spec that parses and assigns a finite floating-point value. */
export function floatFlag(flag, key, options) {
return createAssignedValueFlag((argv, index) => {
return createAssignedValueFlag(flag, (argv, index) => {
const option = consumeFloatFlag(argv, index, flag, options);
return option ? { ...option, key } : null;
});
@@ -212,7 +218,9 @@ export function booleanFlag(flag, key, value = true) {
return null;
}
return {
flag,
nextIndex: index,
repeatable: false,
apply(target) {
target[key] = value;
},
@@ -224,6 +232,7 @@ export function booleanFlag(flag, key, value = true) {
/** Apply flag specs to argv and return the mutated parsed args object. */
export function parseFlagArgs(argv, args, specs, options = {}) {
const ignoreDoubleDash = options.ignoreDoubleDash ?? true;
const seenFlags = new Set();
for (let i = 0; i < argv.length; i += 1) {
const arg = argv[i];
if (arg === "--" && ignoreDoubleDash) {
@@ -235,6 +244,15 @@ export function parseFlagArgs(argv, args, specs, options = {}) {
if (!option) {
continue;
}
if (typeof option.flag !== "string" || !option.flag) {
throw new Error("parseFlagArgs specs must declare a flag for consumed options");
}
if (option.repeatable !== true) {
if (seenFlags.has(option.flag)) {
throw new Error(`${option.flag} was provided more than once`);
}
seenFlags.add(option.flag);
}
option.apply(args);
i = option.nextIndex;
handled = true;
+2
View File
@@ -29,7 +29,9 @@ export function budgetFloatFlag(flag, key) {
throw new Error(`${flag} requires a value`);
}
return {
flag,
nextIndex: index + 1,
repeatable: false,
apply(target) {
const parsed = parseBudgetNumber(value, flag);
if (parsed === null) {
+39
View File
@@ -1,6 +1,7 @@
// Arg Utils tests cover arg utils script behavior.
import { describe, expect, it } from "vitest";
import {
booleanFlag,
floatFlag,
intFlag,
parseFlagArgs,
@@ -43,6 +44,44 @@ describe("scripts/lib/arg-utils parseFlagArgs", () => {
expect(parsed.match).toEqual(["alpha", "beta"]);
});
it("rejects duplicate single-value flags", () => {
expect(() =>
parseFlagArgs(["--label", "first", "--label=second"], { label: "" }, [
stringFlag("--label", "label"),
]),
).toThrow("--label was provided more than once");
expect(() =>
parseFlagArgs(["--limit", "1", "--limit=2"], { limit: 10 }, [
intFlag("--limit", "limit", { min: 1 }),
]),
).toThrow("--limit was provided more than once");
expect(() =>
parseFlagArgs(["--json", "--json"], { json: false }, [booleanFlag("--json", "json")]),
).toThrow("--json was provided more than once");
});
it("requires custom specs to declare consumed flags", () => {
expect(() =>
parseFlagArgs(
["--custom"],
{},
[
{
consume(argv, index) {
if (argv[index] !== "--custom") {
return null;
}
return {
nextIndex: index,
apply() {},
};
},
},
],
),
).toThrow("parseFlagArgs specs must declare a flag for consumed options");
});
it("rejects missing string flag values before consuming the next option", () => {
expect(() =>
parseFlagArgs(["--base", "--head", "HEAD"], { base: "origin/main", head: "HEAD" }, [
+9
View File
@@ -87,6 +87,15 @@ describe("bench-test-changed script", () => {
expect(nextFlag.stderr).not.toContain("at ");
});
it("rejects duplicate max worker values before inspecting git state", () => {
const result = runBenchTestChanged(["--max-workers", "2", "--max-workers", "3"]);
expect(result.status).toBe(1);
expect(result.stdout).toBe("");
expect(result.stderr).toContain("--max-workers was provided more than once");
expect(result.stderr).not.toContain("at ");
});
it("rejects unknown options before collecting changed paths", () => {
const result = runBenchTestChanged(["--max-worker", "4"]);
+32
View File
@@ -43,6 +43,20 @@ describe("QA report source CLIs", () => {
expectNoNodeStack(result.stderr);
});
it("rejects duplicate QA coverage artifact destinations without a Node stack trace", () => {
const result = runSourceScript(
"scripts/qa-coverage-report.ts",
"--output",
".artifacts/first.md",
"--output=.artifacts/second.md",
);
expect(result.status).toBe(1);
expect(result.stdout).toBe("");
expect(result.stderr.trim()).toBe("--output was provided more than once");
expectNoNodeStack(result.stderr);
});
it("reports unknown QA parity options without a Node stack trace", () => {
const result = runSourceScript("scripts/qa-parity-report.ts", "--wat");
@@ -52,6 +66,24 @@ describe("QA report source CLIs", () => {
expectNoNodeStack(result.stderr);
});
it("rejects duplicate QA parity artifact destinations without a Node stack trace", () => {
const result = runSourceScript(
"scripts/qa-parity-report.ts",
"--candidate-summary",
"candidate.json",
"--baseline-summary",
"baseline.json",
"--output-dir",
".artifacts/first",
"--output-dir=.artifacts/second",
);
expect(result.status).toBe(1);
expect(result.stdout).toBe("");
expect(result.stderr.trim()).toBe("--output-dir was provided more than once");
expectNoNodeStack(result.stderr);
});
it("reports missing QA parity inputs without a Node stack trace", () => {
const result = runSourceScript("scripts/qa-parity-report.ts");
+3
View File
@@ -85,6 +85,9 @@ describe("test perf budget script", () => {
expect(() => testing.parseArgs(["--max-regression-pct", "-h"], {})).toThrow(
"--max-regression-pct requires a value",
);
expect(() => testing.parseArgs(["--max-wall-ms", "1000", "--max-wall-ms", "2000"], {})).toThrow(
"--max-wall-ms was provided more than once",
);
});
it("requires timed file evidence in the Vitest JSON report", () => {