build: clean up test command and reduce number of PR actions (#31)

* build: handle webpack@4 SSL workaround directly in tests * build: stricter types for webpack configuration in test helper * build: test all webpack versions with npm test command * build: fix test command for node 12 * build: require WEBPACK_VERSION to be specified when running mocha * build: fix linting issues --------- Co-authored-by: Kevin Montag <[email protected]>
kmontag · May 20, 2023 · 2c07d60 · 2c07d60
1 parent 7512ba0
commit 2c07d60
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 56 deletions.
diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml
@@ -11,10 +11,6 @@ jobs:
     strategy:
       matrix:
         node-version: [12.x, 16.x, 18.x, 20.x]
-        webpack-version: [2, 3, 4, 5]
-
-    env:
-      WEBPACK_VERSION: ${{ matrix.webpack-version }}
 
     steps:
       - uses: actions/checkout@v3
@@ -24,11 +20,7 @@ jobs:
           node-version: ${{ matrix.node-version }}
       - run: npm ci
       - run: npm run build --if-present
-
-      # For node 18 and 20, the webpack4 hashing functions (which get
-      # called separately from this loader during compilation) throw
-      # errors unless we force the legacy SSL provider.
-      - run: ${{ (matrix.webpack-version == '4' && (matrix.node-version == '18.x' || matrix.node-version == '20.x')) && 'NODE_OPTIONS=--openssl-legacy-provider ' || '' }}npm test
+      - run: npm test
 
   validate:
     runs-on: ubuntu-latest

diff --git a/package.json b/package.json
@@ -11,7 +11,11 @@
     "lint:script": "eslint .",
     "lint:style": "prettier -c .",
     "lint": "npm run lint:script && npm run lint:style",
-    "test": "mocha test/*.test.js"
+    "test:webpack2": "WEBPACK_VERSION=2 mocha test/*.test.js",
+    "test:webpack3": "WEBPACK_VERSION=3 mocha test/*.test.js",
+    "test:webpack4": "WEBPACK_VERSION=4 mocha test/*.test.js",
+    "test:webpack5": "WEBPACK_VERSION=5 mocha test/*.test.js",
+    "test": "WEBPACK_VERSION=2 mocha test/*.test.js && WEBPACK_VERSION=3 mocha test/*.test.js && WEBPACK_VERSION=4 mocha test/*.test.js && WEBPACK_VERSION=5 mocha test/*.test.js"
   },
   "engines": {
     "node": ">=12.0.0",

diff --git a/test/helpers/compile.js b/test/helpers/compile.js
@@ -11,8 +11,12 @@ const webpack = (() => {
       return require('webpack3');
     case '4':
       return require('webpack4');
-    default:
+    case '5':
       return require('webpack');
+    default:
+      throw new Error(
+        'Please specify a supported webpack version via the WEBPACK_VERSION environment variable.'
+      );
   }
   /* eslint-enable global-require */
 })();
@@ -34,63 +38,78 @@ const isWebpack5 =
  * @typedef {{ arguments: [string], context: import('../../index').LoaderContext, options: never }} InspectLoaderResult
  */
 
+/**
+ * Helper types for each webpack config schema.
+ *
+ * @typedef { import('webpack2').Configuration } Webpack2Config
+ * @typedef { import('webpack3').Configuration } Webpack3Config
+ * @typedef { import('webpack4').Configuration } Webpack4Config
+ * @typedef { import('webpack').Configuration } Webpack5Config
+ */
+
 /** @type { (fixture: string, loaderOpts?: object, webpackOpts?: object) => Promise<{ inspect: InspectLoaderResult }> } */
 module.exports = function compile(fixture, loaderOpts, webpackOpts) {
   return new Promise((resolve, reject) => {
     /** @type { InspectLoaderResult } */
     let inspect;
 
     /**
-     * @type { ReturnType<typeof webpack> }
+     * @type { Webpack2Config | Webpack3Config | Webpack4Config | Webpack5Config }
      */
-    const compiler =
-      // The function signatures for different webpack versions
-      // aren't compatible, so the compiler thinks this call is
-      // impossible. Note we should still get a runtime error if
-      // the configuration schema isn't valid.
-      //
-      // @ts-ignore
-      webpack(
-        // @ts-ignore
-        {
-          entry: path.resolve(fixturePath, `${fixture}.proto`),
-          output: {
-            path: '/',
-            filename: 'compiled.js',
-          },
-          module: {
-            rules: [
+    const config = {
+      entry: path.resolve(fixturePath, `${fixture}.proto`),
+      output: {
+        path: '/',
+        filename: 'compiled.js',
+        // By default, webpack@4 uses a hash function (md4) which
+        // is not supported by the Node 17+ SSL provider. Set it
+        // explicitly to avoid a compilation error unrelated to
+        // protobufjs. See
+        // https://stackoverflow.com/a/73465262/13264260.
+        ...(isWebpack4Plus ? { hashFunction: 'md5' } : {}),
+      },
+      module: {
+        rules: [
+          {
+            test: /\.proto$/,
+            use: [
               {
-                test: /\.proto$/,
-                use: [
-                  {
-                    loader: 'inspect-loader',
-                    options: {
-                      /** @type { (inspect: InspectLoaderResult) => any } */
-                      callback(callbackInspect) {
-                        inspect = callbackInspect;
-                      },
-                    },
-                  },
-                  {
-                    loader: path.resolve(__dirname, '..', '..', 'index.js'),
-                    options: loaderOpts,
+                loader: 'inspect-loader',
+                options: {
+                  /** @type { (inspect: InspectLoaderResult) => any } */
+                  callback(callbackInspect) {
+                    inspect = callbackInspect;
                   },
-                ],
+                },
+              },
+              {
+                loader: path.resolve(__dirname, '..', '..', 'index.js'),
+                options: loaderOpts,
               },
             ],
           },
-          // webpack@4 adds the `mode` configuration option, which adds some
-          // additional config defaults that we want to avoid for
-          // consistency.
-          ...(isWebpack4Plus ? { mode: 'none' } : {}),
-          // Make sure to test webpack@5 without backwards-compatibility
-          // enabled. See
-          // https://webpack.js.org/configuration/experiments/#experimentsbackcompat.
-          ...(isWebpack5 ? { experiments: { backCompat: false } } : {}),
-          ...webpackOpts,
-        }
-      );
+        ],
+      },
+      // webpack@4 adds the `mode` configuration option, which adds some
+      // additional config defaults that we want to avoid for
+      // consistency.
+      ...(isWebpack4Plus ? { mode: 'none' } : {}),
+      // Make sure to test webpack@5 without backwards-compatibility
+      // enabled. See
+      // https://webpack.js.org/configuration/experiments/#experimentsbackcompat.
+      ...(isWebpack5 ? { experiments: { backCompat: false } } : {}),
+      ...webpackOpts,
+    };
+
+    /**
+     * @type { ReturnType<typeof webpack> }
+     */
+    const compiler =
+      // The function signatures for different webpack versions aren't
+      // compatible, so the compiler thinks this call is impossible.
+      //
+      // @ts-ignore
+      webpack(config);
 
     const fs = new MemoryFS();
 
@@ -131,6 +150,7 @@ module.exports = function compile(fixture, loaderOpts, webpackOpts) {
 
         return undefined;
       })();
+
       if (problem) {
         reject(problem);
       } else {

diff --git a/test/index.test.js b/test/index.test.js
@@ -35,7 +35,8 @@ const minify = (contents) => {
   return result.code;
 };
 
-describe('protobufjs-loader', function () {
+const webpackVersion = process.env.WEBPACK_VERSION;
+describe(`protobufjs-loader with webpack ${webpackVersion}`, function () {
   before(function (done) {
     // The first time the compiler gets run (e.g. in a CI environment),
     // some additional packages will be installed in the