refactor validatePlugins to throw coded errors

- add `createInvalidPluginError` for reporters, UIs, and future plugins - ensures the original error is output if the module exists, but it throws (see `test/node-unit/cli/fixtures/bad-module.fixture.js`) - remove unneeded `process.cwd()` from call to `path.resolve()` Ref: #4198
mochajs · Apr 22, 2020 · c2f55a2 · c2f55a2
1 parent 38d579a
commit c2f55a2
Show file tree

Hide file tree

Showing 4 changed files with 128 additions and 37 deletions.
diff --git a/lib/cli/run-helpers.js b/lib/cli/run-helpers.js
@@ -12,8 +12,10 @@ const path = require('path');
 const debug = require('debug')('mocha:cli:run:helpers');
 const watchRun = require('./watch-run');
 const collectFiles = require('./collect-files');
+const {format} = require('util');
 
 const cwd = (exports.cwd = process.cwd());
+const {createInvalidPluginError} = require('../errors');
 
 /**
  * Exits Mocha when tests + code under test has finished execution (default)
@@ -146,35 +148,44 @@ exports.runMocha = async (mocha, options) => {
 };
 
 /**
- * Used for `--reporter` and `--ui`.  Ensures there's only one, and asserts
- * that it actually exists.
- * @todo XXX This must get run after requires are processed, as it'll prevent
- * interfaces from loading.
+ * Used for `--reporter` and `--ui`.  Ensures there's only one, and asserts that
+ * it actually exists. This must be run _after_ requires are processed (see
+ * {@link handleRequires}), as it'll prevent interfaces from loading otherwise.
  * @param {Object} opts - Options object
  * @param {string} key - Resolvable module name or path
- * @param {Object} [map] - An object perhaps having key `key`
+ * @param {Object} [map] - An object perhaps having key `key`. Used as a cache
+ * of sorts; `Mocha.reporters` is one, where each key corresponds to a reporter
+ * name
  * @private
  */
 exports.validatePlugin = (opts, key, map = {}) => {
   if (Array.isArray(opts[key])) {
-    throw new TypeError(`"--${key} <${key}>" can only be specified once`);
+    throw createInvalidPluginError(
+      `"--${key} <${key}>" can only be specified once`,
+      key
+    );
   }
 
-  const unknownError = () => new Error(`Unknown "${key}": ${opts[key]}`);
+  const unknownError = err =>
+    createInvalidPluginError(
+      format('Could not load %s "%s":\n\n %O', key, opts[key], err),
+      key
+    );
 
+  // if this exists, then it's already loaded, so nothing more to do.
   if (!map[opts[key]]) {
     try {
       opts[key] = require(opts[key]);
     } catch (err) {
       if (err.code === 'MODULE_NOT_FOUND') {
         // Try to load reporters from a path (absolute or relative)
         try {
-          opts[key] = require(path.resolve(process.cwd(), opts[key]));
+          opts[key] = require(path.resolve(opts[key]));
         } catch (err) {
-          throw unknownError();
+          throw unknownError(err);
         }
       } else {
-        throw unknownError();
+        throw unknownError(err);
       }
     }
   }

diff --git a/lib/errors.js b/lib/errors.js
@@ -129,6 +129,26 @@ function createInvalidExceptionError(message, value) {
   return err;
 }
 
+/**
+ * Dynamically creates a plugin-type-specific error based on plugin type
+ * @param {string} message - Error message
+ * @param {"reporter"|"interface"} pluginType - Plugin type. Future: expand as needed
+ * @param {string} pluginName - Name of plugin
+ * @throws When `pluginType` is not known
+ * @public
+ * @returns {Error}
+ */
+function createInvalidPluginError(message, pluginType, pluginName) {
+  switch (pluginType) {
+    case 'reporter':
+      return createInvalidReporterError(message, pluginName);
+    case 'interface':
+      return createInvalidInterfaceError(message, pluginName);
+    default:
+      throw new Error('unknown pluginType "' + pluginType + '"');
+  }
+}
+
 module.exports = {
   createInvalidArgumentTypeError: createInvalidArgumentTypeError,
   createInvalidArgumentValueError: createInvalidArgumentValueError,
@@ -137,5 +157,6 @@ module.exports = {
   createInvalidReporterError: createInvalidReporterError,
   createMissingArgumentError: createMissingArgumentError,
   createNoFilesMatchPatternError: createNoFilesMatchPatternError,
-  createUnsupportedError: createUnsupportedError
+  createUnsupportedError: createUnsupportedError,
+  createInvalidPluginError: createInvalidPluginError
 };
diff --git a/test/node-unit/cli/fixtures/bad-module.fixture.js b/test/node-unit/cli/fixtures/bad-module.fixture.js
@@ -0,0 +1 @@
+throw new Error('this module is wonky');
diff --git a/test/node-unit/cli/run-helpers.spec.js b/test/node-unit/cli/run-helpers.spec.js
@@ -3,7 +3,7 @@
 const {validatePlugin, list} = require('../../../lib/cli/run-helpers');
 const {createSandbox} = require('sinon');
 
-describe('cli "run" command', function() {
+describe('run helper functions', function() {
   let sandbox;
 
   beforeEach(function() {
@@ -14,36 +14,94 @@ describe('cli "run" command', function() {
     sandbox.restore();
   });
 
-  describe('helpers', function() {
-    describe('validatePlugin()', function() {
-      it('should disallow an array of module names', function() {
+  describe('validatePlugin()', function() {
+    describe('when used with "reporter" key', function() {
+      it('should disallow an array of names', function() {
         expect(
-          () => validatePlugin({foo: ['bar']}, 'foo'),
-          'to throw a',
-          TypeError
+          () => validatePlugin({reporter: ['bar']}, 'reporter'),
+          'to throw',
+          {
+            code: 'ERR_MOCHA_INVALID_REPORTER',
+            message: /can only be specified once/i
+          }
+        );
+      });
+
+      it('should fail to recognize an unknown reporter', function() {
+        expect(
+          () => validatePlugin({reporter: 'bar'}, 'reporter'),
+          'to throw',
+          {code: 'ERR_MOCHA_INVALID_REPORTER', message: /cannot find module/i}
+        );
+      });
+    });
+
+    describe('when used with an "interfaces" key', function() {
+      it('should disallow an array of names', function() {
+        expect(
+          () => validatePlugin({interface: ['bar']}, 'interface'),
+          'to throw',
+          {
+            code: 'ERR_MOCHA_INVALID_INTERFACE',
+            message: /can only be specified once/i
+          }
+        );
+      });
+
+      it('should fail to recognize an unknown interface', function() {
+        expect(
+          () => validatePlugin({interface: 'bar'}, 'interface'),
+          'to throw',
+          {code: 'ERR_MOCHA_INVALID_INTERFACE', message: /cannot find module/i}
+        );
+      });
+    });
+
+    describe('when used with an unknown plugin type', function() {
+      it('should fail', function() {
+        expect(
+          () => validatePlugin({frog: 'bar'}, 'frog'),
+          'to throw',
+          /unknown plugin/i
+        );
+      });
+    });
+
+    describe('when a plugin throws an exception upon load', function() {
+      it('should fail and report the original error', function() {
+        expect(
+          () =>
+            validatePlugin(
+              {
+                reporter: require.resolve('./fixtures/bad-module.fixture.js')
+              },
+              'reporter'
+            ),
+          'to throw',
+          {message: /wonky/, code: 'ERR_MOCHA_INVALID_REPORTER'}
         );
       });
     });
+  });
 
-    describe('list()', function() {
-      describe('when provided a flat array', function() {
-        it('should return a flat array', function() {
-          expect(list(['foo', 'bar']), 'to equal', ['foo', 'bar']);
-        });
-      });
-      describe('when provided a nested array', function() {
-        it('should return a flat array', function() {
-          expect(list([['foo', 'bar'], 'baz']), 'to equal', [
-            'foo',
-            'bar',
-            'baz'
-          ]);
-        });
-      });
-      describe('when given a comma-delimited string', function() {
-        it('should return a flat array', function() {
-          expect(list('foo,bar'), 'to equal', ['foo', 'bar']);
-        });
+  describe('list()', function() {
+    describe('when provided a flat array', function() {
+      it('should return a flat array', function() {
+        expect(list(['foo', 'bar']), 'to equal', ['foo', 'bar']);
+      });
+    });
+    describe('when provided a nested array', function() {
+      it('should return a flat array', function() {
+        expect(list([['foo', 'bar'], 'baz']), 'to equal', [
+          'foo',
+          'bar',
+          'baz'
+        ]);
+      });
+    });
+    describe('when given a comma-delimited string', function() {
+      it('should return a flat array', function() {
+        expect(list('foo,bar'), 'to equal', ['foo', 'bar']);
       });
     });
   });