Skip to content

Commit

Permalink
fix(cli): fix a bug in CLI argument parsing
Browse files Browse the repository at this point in the history
This fixes a bug related to parsing of Boolean CLI arguments using
the `--argName=value` type syntax.
We intend to support using that syntax to pass a boolean CLI flag (like
`--watch`) but actually at present the way it is 'parsed' will result in
such flags always being set to true! Not good!

This is because we were setting the value of the boolean CLI flag to
`Boolean('false')` if you pass `--argName=false`, which does _not_
result in a value of `false` but instead a value of `true`. This is
changed to instead use a check that the value equals `'true'` to turn
the value into a boolean, and regression tests are added.

This was noticed when investigating #5640, but that bug actually has to
do with how we convert CLI arguments into a Jest configuration.
  • Loading branch information
alicewriteswrongs committed Apr 10, 2024
1 parent b3886aa commit 2209502
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
13 changes: 10 additions & 3 deletions src/cli/parse-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ const parseCLITerm = (flags: ConfigFlags, args: string[]) => {

/**
* Normalize a 'negative' flag name, just to do a little pre-processing before
* we pass it to `setCLIArg`.
* we pass it to {@link setCLIArg}.
*
* @param flagName the flag name to normalize
* @returns a normalized flag name
Expand Down Expand Up @@ -221,12 +221,19 @@ const setCLIArg = (flags: ConfigFlags, rawArg: string, normalizedArg: string, va

// We're setting a boolean!
if (readOnlyArrayHasStringMember(BOOLEAN_CLI_FLAGS, normalizedArg)) {
flags[normalizedArg] =
const parsed =
typeof value === 'string'
? Boolean(value)
? // check if the value is `'true'`
value === 'true'
: // no value was supplied, default to true
true;

flags[normalizedArg] = parsed;
flags.knownArgs.push(rawArg);

if (typeof value === 'string' && value !== '') {
flags.knownArgs.push(value);
}
}

// We're setting a string!
Expand Down
6 changes: 6 additions & 0 deletions src/cli/test/parse-flags.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ describe('parseFlags', () => {
expect(flags.knownArgs).toEqual([]);
expect(flags[cliArg]).toBe(undefined);
});

it.each([true, false])(`should set the value with --${cliArg}=%p`, (value) => {
const flags = parseFlags([`--${cliArg}=${value}`]);
expect(flags.knownArgs).toEqual([`--${cliArg}`, String(value)]);
expect(flags[cliArg]).toBe(value);
});
});

describe.each(STRING_CLI_FLAGS)('should parse string flag %s', (cliArg) => {
Expand Down

0 comments on commit 2209502

Please sign in to comment.