fix(cli): ensure that argument order is correct for Jest (#3827)

This ensures that CLI arguments are ordered correctly when we pass them
to Jest. In particular, we want to ensure that all flags of the type
`--maxWorkers=0` and the like are _before_ any flags which we don't know
about, like `my-spec-file.ts` (the latter being actually a match pattern
for which spec files to run).

In other words, if the user passes arguments like this:

```
--e2e foo.spec.ts
```

or

```
--e2e -- foo.spec.ts
```

We need to ensure we use something like

```
["--e2e", "foo.spec.ts"]
```

to generate the Jest config.

The wrinkle is that after we've parsed our known CLI arguments (in the
`cli` module) we do some addition modification of the arguments, based
on values set in the `Config`, in the `buildJestArgv` function. In
particular, we'll look to see if a `maxWorkers` flag is already passed
and, if not, we add one to the args before they're used to build the
Jest configuration.

Before this change that would result in an array like this being passed
to Jest:

```
["--e2e", "foo.spec.ts", "--maxWorkers=0"]
```

because we simply stuck the new `--maxWorkers` arg right on the end of
the already-combined array (combined because it's basically `knownArgs +
unknownArgs`). No good!

Why no good? Well, basically because Jest works best if the filename
matches are at the end. It's behavior if they're _not_ is,
unfortunately, inconsistent and it will work sometimes, but it (as far
as I have tested it!) _always_ works if the pattern is at the end.

So in order to provide a guarantee the pattern is at the end, we modify
a copy of `knownArgs` with flags like this and then produce a new,
combined array to pass to Jest when we're all done, so it looks like
this instead:

```
["--e2e", "--maxWorkers=0", "foo.spec.ts"]
```

Much better!

Author: alicewriteswrongs

src/testing/jest/jest-config.ts

@@ -46,15 +46,27 @@ function getLegacyJestOptions(): Record<string, boolean | number | string> {
 export function buildJestArgv(config: d.ValidatedConfig): Config.Argv {
   const yargs = require('yargs');
 
-  const args = [...config.flags.knownArgs.slice(), ...config.flags.unknownArgs.slice()];
+  const knownArgs = config.flags.knownArgs.slice();
 
-  if (!args.some((a) => a.startsWith('--max-workers') || a.startsWith('--maxWorkers'))) {
-    args.push(`--max-workers=${config.maxConcurrentWorkers}`);
+  if (!knownArgs.some((a) => a.startsWith('--max-workers') || a.startsWith('--maxWorkers'))) {
+    knownArgs.push(`--max-workers=${config.maxConcurrentWorkers}`);
   }
 
   if (config.flags.devtools) {
-    args.push('--runInBand');
-  }
+    knownArgs.push('--runInBand');
+  }
+
+  // we combine the modified args and the unknown args here and declare the
+  // result read only, providing some typesystem-level assurance that we won't
+  // mutate it after this point.
+  //
+  // We want that assurance because Jest likes to have any filepath match
+  // patterns at the end of the args it receives. Those args are going to be
+  // found in our `unknownArgs`, so while we want to do some stuff in this
+  // function that adds to `knownArgs` we need a guarantee that all of the
+  // `unknownArgs` are _after_ all the `knownArgs` in the array we end up
+  // generating the Jest configuration from.
+  const args: ReadonlyArray<string> = [...knownArgs, ...config.flags.unknownArgs];
 
   config.logger.info(config.logger.magenta(`jest args: ${args.join(' ')}`));
 

src/testing/jest/test/jest-config.spec.ts

@@ -42,7 +42,7 @@ describe('jest-config', () => {
     expect(jestArgv.maxWorkers).toBe(2);
   });
 
-  it('forces --maxWorkers=4 arg when e2e test and --ci', () => {
+  it('passes default --maxWorkers=0 arg when e2e test and --ci', () => {
     const args = ['test', '--ci', '--e2e'];
     const config = mockValidatedConfig();
     config.flags = parseFlags(args);
@@ -55,6 +55,20 @@ describe('jest-config', () => {
     expect(jestArgv.maxWorkers).toBe(0);
   });
 
+  it('passed default --maxWorkers=0 arg when e2e test and --ci with filepath', () => {
+    const args = ['test', '--ci', '--e2e', '--', 'my-specfile.spec.ts'];
+    const config = mockValidatedConfig();
+    config.flags = parseFlags(args);
+
+    expect(config.flags.args).toEqual(['--ci', '--e2e', '--', 'my-specfile.spec.ts']);
+    expect(config.flags.unknownArgs).toEqual(['--', 'my-specfile.spec.ts']);
+
+    const jestArgv = buildJestArgv(config);
+    expect(jestArgv.ci).toBe(true);
+    expect(jestArgv.maxWorkers).toBe(0);
+    expect(jestArgv._).toEqual(['my-specfile.spec.ts']);
+  });
+
   it('pass --maxWorkers=2 arg to jest', () => {
     const args = ['test', '--maxWorkers=2'];
     const config = mockValidatedConfig();
@@ -200,5 +214,15 @@ describe('jest-config', () => {
 
     const jestArgv = buildJestArgv(config);
     expect(jestArgv.json).toBe(true);
+    // the `_` field holds any filename pattern matches
+    expect(jestArgv._).toEqual(['my-component.spec.ts']);
+  });
+
+  it('should parse multiple file patterns', () => {
+    const args = ['test', '--spec', '--json', '--', 'foobar/*', 'my-spec.ts'];
+    const jestArgv = buildJestArgv(mockValidatedConfig({ flags: parseFlags(args) }));
+    expect(jestArgv.json).toBe(true);
+    // the `_` field holds any filename pattern matches
+    expect(jestArgv._).toEqual(['foobar/*', 'my-spec.ts']);
   });
 });