diff --git a/scripts/bench-test-changed.mjs b/scripts/bench-test-changed.mjs index e8104791a616..a7ff040dce20 100644 --- a/scripts/bench-test-changed.mjs +++ b/scripts/bench-test-changed.mjs @@ -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); }, diff --git a/scripts/lib/arg-utils.mjs b/scripts/lib/arg-utils.mjs index 76ef2bc27c42..f57fdf00fe2f 100644 --- a/scripts/lib/arg-utils.mjs +++ b/scripts/lib/arg-utils.mjs @@ -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; diff --git a/scripts/lib/budget-number-args.mjs b/scripts/lib/budget-number-args.mjs index 2401c957d3a8..03cbe9f2ed6f 100644 --- a/scripts/lib/budget-number-args.mjs +++ b/scripts/lib/budget-number-args.mjs @@ -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) { diff --git a/test/scripts/arg-utils.test.ts b/test/scripts/arg-utils.test.ts index 45b5f7800301..ce369a1cbc35 100644 --- a/test/scripts/arg-utils.test.ts +++ b/test/scripts/arg-utils.test.ts @@ -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" }, [ diff --git a/test/scripts/bench-test-changed.test.ts b/test/scripts/bench-test-changed.test.ts index 6d4572918ef8..3486e2e6a356 100644 --- a/test/scripts/bench-test-changed.test.ts +++ b/test/scripts/bench-test-changed.test.ts @@ -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"]); diff --git a/test/scripts/qa-report-cli.test.ts b/test/scripts/qa-report-cli.test.ts index 42d910aba247..d7495c1aadef 100644 --- a/test/scripts/qa-report-cli.test.ts +++ b/test/scripts/qa-report-cli.test.ts @@ -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"); diff --git a/test/scripts/test-perf-budget.test.ts b/test/scripts/test-perf-budget.test.ts index 867fb4d4f1e6..97b11a8f5542 100644 --- a/test/scripts/test-perf-budget.test.ts +++ b/test/scripts/test-perf-budget.test.ts @@ -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", () => {