Skip to content

Commit

Permalink
fix(jest): adjust conversion of CLI args to Jest args
Browse files Browse the repository at this point in the history
This adjusts the conversion of CLI args (parsed by our `cli/parse-flags`
module) to arguments for Jest to fix a problematic situation when
passing an argument which is not recognized by our `parse-flags` module.

A common pattern in Jest is to use a filename filter to narrow which
tests are matched and run by the test runner, like so:

```
jest -- some-file-name.test.ts
```

Our CLI arg parsing module recognizes all Jest CLI args (like `--json`,
`--watch`, and more) as 'first class' args and parses them into a
`ConfigFlags` object. For the arguments shown above (`'--'`,
`'some-file-name.test.ts'`), however, it will _not_ recognize them as
first-class arguments and will instead preserve them in the
`unknownArgs` array on the `ConfigFlags` object.

The flags (known and unknown) parsed by the `parse-flags` module are
then converted into settings for Jest using the `buildJestArgv`
function. Prior to this commit we took the known and unknown args off of
the `ConfigFlags` object like so:

```ts
const args = [...config.flags.unknownArgs.slice(), ...config.flags.knownArgs.slice()];
```

If a filename match pattern is used alone (as in the example above) this
will result in an array that looks like this:

```ts
const args = ['--', 'some-file-name.test.ts'];
```

That will translate correctly to what we want to communicate to Jest,
and it will only run test files which match the pattern.

The problem comes in when a filename match pattern is used in
conjunction with a boolean flag, like so:

```
jest --json -- some-file-name.test.ts
```

Or, in a Stencil context, doing something like this:

```
npx stencil test --spec --json -- my-component.spec.ts
```

which would result in the following `args` array being passed to Jest:

```ts
const args = [
  '--',
  'some-file-name.test.ts',
  '--spec',
  '--json',
];
```

Jest expects that `'--'` is _only_ followed by filename patterns and
that all boolean flags like `--json` are before it in the argument list,
so it will interpret this array as trying to match filenames which
contain the string `'--json'`, rather than setting the `.json` flag on
the Jest config.

The solution to this is to switch the order of the arguments passed to
Jest, passing the known arguments _first_, followed by the unknown
arguments.

Note: this used to work properly before #3444 because a flag like
`--json` would have been stuck in the `unknownArgs` array, so the
argument array passed to Jest would have looked like:

```ts
const args = [
  '--json',
  '--',
  'some-file-name.test.ts'
];
```

When we added explicit support for all Jest args this behavior was
broken.

This commit adds a regression test which fails without the fix (changing
the argument order) and adds the fix as well.
  • Loading branch information
alicewriteswrongs committed Oct 18, 2022
1 parent 7bb0a27 commit 5b76a0a
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/testing/jest/jest-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function getLegacyJestOptions(): Record<string, boolean | number | string> {
export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
const yargs = require('yargs');

const args = [...config.flags.unknownArgs.slice(), ...config.flags.knownArgs.slice()];
const args = [...config.flags.knownArgs.slice(), ...config.flags.unknownArgs.slice()];

if (!args.some((a) => a.startsWith('--max-workers') || a.startsWith('--maxWorkers'))) {
args.push(`--max-workers=${config.maxConcurrentWorkers}`);
Expand Down
12 changes: 12 additions & 0 deletions src/testing/jest/test/jest-config.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,16 @@ describe('jest-config', () => {
expect(jestArgv.spec).toBe(true);
expect(jestArgv.passWithNoTests).toBe(true);
});

it('should parse a setup with a filepath constraint', () => {
const args = ['test', '--spec', '--json', '--', 'my-component.spec.ts'];
const config = mockValidatedConfig();
config.flags = parseFlags(args);

expect(config.flags.args).toEqual(['--spec', '--json', '--', 'my-component.spec.ts']);
expect(config.flags.unknownArgs).toEqual(['--', 'my-component.spec.ts']);

const jestArgv = buildJestArgv(config);
expect(jestArgv.json).toBe(true);
});
});

0 comments on commit 5b76a0a

Please sign in to comment.