From 400bf9b9319bdf8f0d9aa05ff8c7a54f6fe65d2a Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Tue, 21 Apr 2020 11:58:56 -0700 Subject: [PATCH] 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 --- lib/cli/run-helpers.js | 49 +++++--- lib/errors.js | 23 +++- .../cli/fixtures/bad-module.fixture.js | 1 + test/node-unit/cli/run-helpers.spec.js | 115 ++++++++++++------ 4 files changed, 138 insertions(+), 50 deletions(-) create mode 100644 test/node-unit/cli/fixtures/bad-module.fixture.js diff --git a/lib/cli/run-helpers.js b/lib/cli/run-helpers.js index 58bae02e6f..ebb0bdd071 100644 --- 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,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); } } } diff --git a/lib/errors.js b/lib/errors.js index fafee70eee..099bc579ab 100644 --- 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} [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 }; diff --git a/test/node-unit/cli/fixtures/bad-module.fixture.js b/test/node-unit/cli/fixtures/bad-module.fixture.js new file mode 100644 index 0000000000..18fb55ce88 --- /dev/null +++ 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 index a2a63335f5..00357bbcb5 100644 --- a/test/node-unit/cli/run-helpers.spec.js +++ b/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']); }); }); });