Add test parallelism

.changeset/flat-hotels-hammer.md

@@ -0,0 +1,6 @@
+---
+'@quilted/craft': patch
+'@quilted/sewing-kit': patch
+---
+
+Add test --parallel option

.changeset/silent-donkeys-share.md

@@ -0,0 +1,5 @@
+---
+'@quilted/craft': patch
+---
+
+Upgrade Jest to v28

.github/workflows/ci.yml

@@ -52,6 +52,9 @@ jobs:
     name: End-to-end tests 🔬
     runs-on: ubuntu-latest
     timeout-minutes: 10
+    strategy:
+      matrix:
+        parallel: [1/3, 2/3, 3/3]
     steps:
       - uses: actions/checkout@v3
       - uses: ./.github/workflows/actions/prepare
@@ -63,4 +66,4 @@ jobs:
           arguments: --skip-project create
       - uses: quilt-framework/action-test@v2
         with:
-          command: pnpm test:e2e
+          command: pnpm test:e2e --parallel ${{ matrix.parallel }}

packages/craft/jest/resolver.cjs

@@ -1,38 +1,29 @@
-// Jest’s support for ESM is a bit of a mess. It doesn’t support
-// export maps, which we use to declare all the multi-entry packages
-// in quilt. It has some native support for ESM, but I had a lot of
-// issues with it. It technically supports transpiling ESM in node_modules
-// to CommonJS, but it is very difficult to do so because you have to create
-// a regular expression that prevents some subset of node_modules from being
-// ignored by transformers.
+// These libraries ship with the browser export condition targeting ESM. This breaks Jest,
+// because it prefers the browser condition but does not understand ESM yet. This deletes the
+// offending package.json fields so that the CommonJS variants are preferred.
 //
-// After some thought, I am just solving one part of this for now:
-// support for export maps. This custom resolver lets the entries in Quilt
-// and other export map packages be resolved by Jest. Unfortunately, if those
-// packages are ESM-only, they will not work; for Quilt, I added a `require`
-// export condition to all packages that might be imported in tests that
-// points to a CommonJS build.
-//
-// See https://github.com/facebook/jest/issues/9771 for details on this approach.
-
-const enhancedResolve = require('enhanced-resolve');
+// @see https://github.com/microsoft/accessibility-insights-web/pull/5421#issuecomment-1109168149
+const PACKAGES_WITH_ESM_BROWSER_ENTRIES = new Set([
+  '@remix-run/web-fetch',
+  '@remix-run/web-blob',
+  '@remix-run/web-stream',
+  '@remix-run/web-form-data',
+  '@web3-storage/multipart-parser',
+  'preact',
+]);
 
-const resolve = enhancedResolve.create.sync({
-  conditionNames: [
-    // 'quilt:esnext',
-    // 'esnext',
-    // 'import',
-    'require',
-    'default',
-    'node',
-  ],
-  mainFields: [
-    // 'module',
-    'main',
-  ],
-  extensions: ['.ts', '.tsx', '.js', '.mjs', '.json', '.node'],
-});
+module.exports = (path, options) => {
+  // Call the defaultResolver, so we leverage its cache, error handling, etc.
+  return options.defaultResolver(path, {
+    ...options,
+    // Use packageFilter to process parsed `package.json` before the resolution (see https://www.npmjs.com/package/resolve#resolveid-opts-cb)
+    packageFilter: (pkg) => {
+      if (PACKAGES_WITH_ESM_BROWSER_ENTRIES.has(pkg.name)) {
+        delete pkg['exports'];
+        delete pkg['module'];
+      }
 
-module.exports = function resolver(request, options) {
-  return resolve(options.basedir, request);
+      return pkg;
+    },
+  });
 };

packages/craft/package.json

@@ -227,12 +227,12 @@
     "@rollup/plugin-json": "^4.1.0",
     "@rollup/plugin-node-resolve": "^13.0.0",
     "@rollup/plugin-replace": "^2.4.0",
-    "@types/jest": "^27.4.0",
+    "@types/jest": "^28.1.0",
     "@types/postcss-preset-env": "^6.7.0",
     "@types/prettier": "^2.4.0",
     "@types/babel__preset-env": "^7.9.0",
     "arg": "^5.0.0",
-    "babel-jest": "^27.0.0",
+    "babel-jest": "^28.1.0",
     "browserslist": "^4.0.0",
     "browserslist-useragent-regexp": "^3.0.0",
     "caniuse-api": "^3.0.0",
@@ -241,14 +241,14 @@
     "common-tags": "^1.8.0",
     "core-js": "^3.21.0",
     "dotenv": "^16.0.0",
-    "enhanced-resolve": "^5.9.0",
     "esbuild": "^0.14.0",
     "eslint": "^8.18.0",
     "express": "^4.17.0",
     "globby": "^13.0.0",
-    "jest": "^27.5.0",
-    "jest-config": "^27.5.0",
-    "jest-watch-typeahead": "^1.0.0",
+    "jest": "^28.1.0",
+    "jest-config": "^28.1.0",
+    "jest-environment-jsdom": "^28.1.0",
+    "jest-watch-typeahead": "^2.0.0",
     "magic-string": "^0.26.0",
     "mrmime": "^1.0.0",
     "pkg-dir": "^6.0.0",

packages/craft/source/cli/tasks/test.ts

@@ -1,4 +1,4 @@
-import {Task} from '../../kit';
+import {Task, DiagnosticError} from '../../kit';
 
 import type {TestTaskOptions} from '../../kit';
 
@@ -10,6 +10,7 @@ export const test = createCommand(
     '--watch': Boolean,
     '--no-watch': Boolean,
     '--debug': Boolean,
+    '--parallel': String,
     '--include-pattern': [String],
     '--exclude-pattern': [String],
   },
@@ -18,6 +19,7 @@ export const test = createCommand(
       _: includePatterns,
       '--include-pattern': includePatternsAsFlag = [],
       '--exclude-pattern': excludePatterns = [],
+      '--parallel': parallel,
       '--watch': watch = !process.env.CI,
       '--no-watch': noWatch,
       '--debug': debug = false,
@@ -29,6 +31,7 @@ export const test = createCommand(
       debug,
       includePatterns: [...includePatterns, ...includePatternsAsFlag],
       excludePatterns,
+      parallel: parallel ? validateParallel(parallel) : undefined,
     });
   },
 );
@@ -41,3 +44,16 @@ export async function runTest(context: TaskContext, options: TestTaskOptions) {
     coreHooksForWorkspace: () => ({}),
   });
 }
+
+const PARALLELISM_REGEX = /^\d+[/]\d+$/;
+
+function validateParallel(parallel: string): `${number}/${number}` {
+  if (!PARALLELISM_REGEX.test(parallel)) {
+    throw new DiagnosticError({
+      title: 'Invalid --parallel argument',
+      content: `This argument must be in the format \`\${number}/\${number}\`, where the first number is the index of the current parallel, run, and the second number is the total number of parallel runs.`,
+    });
+  }
+
+  return parallel as any;
+}

packages/craft/source/tools/jest.ts

@@ -43,6 +43,7 @@ export interface JestFlags {
   cacheDirectory?: string;
   passWithNoTests?: boolean;
   detectOpenHandles?: boolean;
+  shard?: string;
   [key: string]: unknown;
 }
 
@@ -51,6 +52,8 @@ export interface JestWorkspaceHooks
   jestConfig: WaterfallHook<JestConfig>;
   jestFlags: WaterfallHook<JestFlags>;
   jestWatchPlugins: WaterfallHook<string[]>;
+  jestShard: WaterfallHook<`${number}/${number}` | undefined>;
+  jestReporters: WaterfallHook<(string | Config.ReporterConfig)[]>;
 }
 
 declare module '@quilted/sewing-kit' {
@@ -83,6 +86,8 @@ export function jest() {
         jestTransforms: waterfall(),
         jestIgnore: waterfall(),
         jestWatchIgnore: waterfall(),
+        jestShard: waterfall(),
+        jestReporters: waterfall(),
       }));
 
       project(({hooks}) => {
@@ -121,7 +126,9 @@ export function jest() {
                 jestSetupTests,
                 jestTransforms,
                 jestIgnore,
+                jestShard,
                 jestWatchIgnore,
+                jestReporters,
               },
             ] = await Promise.all([import('jest-config'), configuration()]);
 
@@ -130,7 +137,8 @@ export function jest() {
               (envVar) => Boolean(envVar) && truthyEnvValues.has(envVar!),
             );
 
-            const {watch, debug, includePatterns, excludePatterns} = options;
+            const {watch, debug, parallel, includePatterns, excludePatterns} =
+              options;
 
             // We create an alias map of the repo’s internal packages. This prevents
             // issues where Jest might try to use the built output for a package (as
@@ -159,6 +167,7 @@ export function jest() {
                 moduleMapper,
                 setupEnvironmentFiles,
                 setupTestsFiles,
+                reporters,
               ] = await Promise.all([
                 // TODO should this be inferred...
                 jestEnvironment!.run('node'),
@@ -182,6 +191,11 @@ export function jest() {
                 jestModuleMapper!.run({...internalModuleMap}),
                 jestSetupEnv!.run([]),
                 jestSetupTests!.run([]),
+                jestReporters!.run(
+                  process.env.GITHUB_ACTIONS
+                    ? ['default', 'github-actions']
+                    : [],
+                ),
               ]);
 
               const normalizedExtensions = extensions.map((extension) =>
@@ -205,6 +219,7 @@ export function jest() {
                 watchPathIgnorePatterns: watchIgnore,
                 transform,
                 resolver: RESOLVER_MODULE,
+                reporters: reporters.length > 0 ? reporters : undefined,
                 cacheDirectory: workspace.fs.temporaryPath('jest/cache'),
               });
 
@@ -227,19 +242,6 @@ export function jest() {
                     jestWatchIgnore,
                   } = await projectConfiguration(project);
 
-                  // TODO move to craft
-                  // const [
-                  //   setupEnvironment,
-                  //   setupEnvironmentIndexes,
-                  //   setupTests,
-                  //   setupTestsIndexes,
-                  // ] = await Promise.all([
-                  //   project.fs.glob('tests/setup/environment.*'),
-                  //   project.fs.glob('tests/setup/environment/index.*'),
-                  //   project.fs.glob('tests/setup/tests.*'),
-                  //   project.fs.glob('tests/setup/tests/index.*'),
-                  // ]);
-
                   const [
                     environment,
                     testIgnore,
@@ -301,12 +303,13 @@ export function jest() {
               ),
             );
 
-            const [watchPlugins] = await Promise.all([
+            const [watchPlugins, shard] = await Promise.all([
               jestWatchPlugins!.run([
                 // These are so useful, they should be on by default. Sue me.
                 require.resolve('jest-watch-typeahead/filename'),
                 require.resolve('jest-watch-typeahead/testname'),
               ]),
+              jestShard!.run(parallel),
             ]);
 
             const config = await jestConfig!.run({
@@ -347,6 +350,7 @@ export function jest() {
               forceExit: isCi || debug,
               runInBand: debug,
               detectOpenHandles: debug,
+              shard,
             });
 
             const spawned = runner.spawn(

packages/sewing-kit/source/hooks/test.ts

@@ -92,6 +92,19 @@ export interface TestTaskOptions {
    * hooks, and other configuration changes.
    */
   readonly debug: boolean;
+
+  /**
+   * A string representing what part of the tests in the project to run, when running
+   * with multiple parallel test runners. By default, parallelism is not used. When
+   * parallelism is explicitly enabled, it is passed as a string in the format
+   * `${number}/${number}`, where the first number is the index of the current parallel
+   * run, and the second number is the total number of parallel runs. For example,
+   * the string `"2/3"` indicates that this is the second of three parallel runs.
+   *
+   * Test tools that support parallelism should use this string to determine an appropriate
+   * subset of the project’s tests that should be run.
+   */
+  readonly parallel?: `${number}/${number}`;
 }
 
 /**