Skip to content

Commit

Permalink
fix(cli): fix a bug in CLI argument parsing (#5646)
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 12, 2024
1 parent 55d8e2f commit 1fdea63
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
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
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 1fdea63

Please sign in to comment.