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

Author: boneskull

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,52 @@ 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 {"reporter"|"interface"} pluginType - Type of plugin.
+ * @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`);
+exports.validatePlugin = (opts, pluginType, map = {}) => {
+  /**
+   * This should be a unique identifier; either a string (present in `map`),
+   * or a resolvable (via `require.resolve`) module ID/path.
+   * @type {string}
+   */
+  const pluginId = opts[pluginType];
+
+  if (Array.isArray(pluginId)) {
+    throw createInvalidPluginError(
+      `"--${pluginType}" can only be specified once`,
+      pluginType
+    );
   }
 
-  const unknownError = () => new Error(`Unknown "${key}": ${opts[key]}`);
+  const unknownError = err =>
+    createInvalidPluginError(
+      format('Could not load %s "%s":\n\n %O', pluginType, pluginId, err),
+      pluginType,
+      pluginId
+    );
 
-  if (!map[opts[key]]) {
+  // if this exists, then it's already loaded, so nothing more to do.
+  if (!map[pluginId]) {
     try {
-      opts[key] = require(opts[key]);
+      opts[pluginType] = require(pluginId);
     } 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[pluginType] = require(path.resolve(pluginId));
         } catch (err) {
-          throw unknownError();
+          throw unknownError(err);
         }
       } else {
-        throw unknownError();
+        throw unknownError(err);
       }
     }
   }

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} [pluginId] - Name/path of plugin, if any
+ * @throws When `pluginType` is not known
+ * @public
+ * @returns {Error}
+ */
+function createInvalidPluginError(message, pluginType, pluginId) {
+  switch (pluginType) {
+    case 'reporter':
+      return createInvalidReporterError(message, pluginId);
+    case 'interface':
+      return createInvalidInterfaceError(message, pluginId);
+    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
 };

test/node-unit/cli/fixtures/bad-module.fixture.js

@@ -0,0 +1 @@
+throw new Error('this module is wonky');

test/node-unit/cli/run-helpers.spec.js

@@ -1,49 +1,96 @@
 'use strict';
 
 const {validatePlugin, list} = require('../../../lib/cli/run-helpers');
-const {createSandbox} = require('sinon');
 
-describe('cli "run" command', function() {
-  let sandbox;
+describe('run helper functions', function() {
+  describe('validatePlugin()', function() {
+    describe('when used with "reporter" key', function() {
+      it('should disallow an array of names', function() {
+        expect(
+          () => validatePlugin({reporter: ['bar']}, 'reporter'),
+          'to throw',
+          {
+            code: 'ERR_MOCHA_INVALID_REPORTER',
+            message: /can only be specified once/i
+          }
+        );
+      });
 
-  beforeEach(function() {
-    sandbox = createSandbox();
-  });
+      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}
+        );
+      });
+    });
 
-  afterEach(function() {
-    sandbox.restore();
-  });
+    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
+          }
+        );
+      });
 
-  describe('helpers', function() {
-    describe('validatePlugin()', function() {
-      it('should disallow an array of module names', function() {
+      it('should fail to recognize an unknown interface', function() {
         expect(
-          () => validatePlugin({foo: ['bar']}, 'foo'),
-          'to throw a',
-          TypeError
+          () => validatePlugin({interface: 'bar'}, 'interface'),
+          'to throw',
+          {code: 'ERR_MOCHA_INVALID_INTERFACE', message: /cannot find module/i}
         );
       });
     });
 
-    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('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']);
       });
     });
   });