diff --git a/lib/cli/options.js b/lib/cli/options.js index b25fe64db8..9768206bcc 100644 --- a/lib/cli/options.js +++ b/lib/cli/options.js @@ -114,15 +114,31 @@ const nargOpts = types.array const parse = (args = [], defaultValues = {}, ...configObjects) => { // save node-specific args for special handling. // 1. when these args have a "=" they should be considered to have values - // 2. if they don't, they just boolean flags + // 2. if they don't, they are just boolean flags // 3. to avoid explicitly defining the set of them, we tell yargs-parser they // are ALL boolean flags. // 4. we can then reapply the values after yargs-parser is done. const allArgs = Array.isArray(args) ? args : args.split(' '); - const nodeArgs = allArgs.reduce((acc, arg) => { - if (typeof arg !== 'string') { - throw new Error(`Invalid option ${arg} passed to mocha cli`); + const nodeArgs = allArgs.reduce((acc, arg, index, allArgs) => { + const maybeFlag = allArgs[index - 1]; + + if (isNumeric(arg) && (!maybeFlag || !isMochaFlag(maybeFlag))) { + throw createUnsupportedError( + `Option ${arg} is unsupported by the mocha cli` + ); + } + if ( + isNumeric(arg) && + isMochaFlag(maybeFlag) && + expectedTypeForFlag(maybeFlag) !== 'number' + ) { + throw createInvalidArgumentTypeError( + `Mocha flag '--${maybeFlag}' given invalid option: '${arg}'`, + Number(arg), + expectedTypeForFlag(maybeFlag) + ); } + const pair = arg.split('='); let flag = pair[0]; if (isNodeFlag(flag, false)) { @@ -156,30 +172,6 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => { result.argv[key] = value; }); - const numericPositionalArgs = result.argv._.filter(arg => isNumeric(arg)); - numericPositionalArgs.forEach(numericArg => { - const flag = allArgs - .map(arg => arg.replace(/^--?/, '')) - .find((arg, index) => { - return ( - isMochaFlag(arg) && - args[index + 1] === String(numericArg) && - String(result.argv[arg]) !== String(numericArg) - ); - }); - if (flag) { - throw createInvalidArgumentTypeError( - `Flag ${flag} has invalid arg ${numericArg}`, - numericArg, - expectedTypeForFlag(flag) - ); - } else { - throw createUnsupportedError( - `Invalid option ${numericArg} passed to mocha cli` - ); - } - }); - return result.argv; }; @@ -227,7 +219,8 @@ const loadPkgRc = (args = {}) => { filepath ); } else { - debug('failed to read default package.json at %s; ignoring', filepath); + debug('failed to read default package.json at %s; ignoring', + filepath); return result; } } diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js index 44ebf1e988..7deda70784 100644 --- a/lib/cli/run-option-metadata.js +++ b/lib/cli/run-option-metadata.js @@ -50,17 +50,8 @@ const TYPES = (exports.types = { 'sort', 'watch' ], - number: ['retries', 'jobs'], - string: [ - 'config', - 'fgrep', - 'grep', - 'package', - 'reporter', - 'ui', - 'slow', - 'timeout' - ] + number: ['retries', 'jobs', 'slow', 'timeout'], + string: ['config', 'fgrep', 'grep', 'package', 'reporter', 'ui'] }); /** diff --git a/test/node-unit/cli/mocha-flags.spec.js b/test/node-unit/cli/mocha-flags.spec.js new file mode 100644 index 0000000000..dbd56334b2 --- /dev/null +++ b/test/node-unit/cli/mocha-flags.spec.js @@ -0,0 +1,27 @@ +'use strict'; + +const { + types, + expectedTypeForFlag +} = require('../../../lib/cli/run-option-metadata'); + +describe('mocha-flags', function () { + describe('expectedTypeForFlag()', function () { + it('returns expected type for all mocha flags', function () { + Object.entries(types).forEach(([dataType, flags]) => { + flags.forEach(flag => { + expect(expectedTypeForFlag(flag), 'to equal', dataType); + }); + }); + }); + + it('returns undefined for node flags', function () { + expect(expectedTypeForFlag('--throw-deprecation'), 'to equal', undefined); + expect(expectedTypeForFlag('throw-deprecation'), 'to equal', undefined); + }); + + it('returns undefined for unsupported flags', function () { + expect(expectedTypeForFlag('--foo'), 'to equal', undefined); + }); + }); +}); diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index 7f38002fa7..cf48731925 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -3,6 +3,7 @@ const sinon = require('sinon'); const rewiremock = require('rewiremock/node'); const {ONE_AND_DONE_ARGS} = require('../../../lib/cli/one-and-dones'); +const {constants} = require('../../../lib/errors'); const modulePath = require.resolve('../../../lib/cli/options'); const mocharcPath = require.resolve('../../../lib/mocharc.json'); @@ -524,7 +525,7 @@ describe('options', function () { loadOptions('--timeout 500'), 'to have property', 'timeout', - '500' + 500 ); }); @@ -678,6 +679,23 @@ describe('options', function () { describe('"numeric arguments"', function () { const numericArg = 123; + const unsupportedError = arg => { + return { + message: `Option ${arg} is unsupported by the mocha cli`, + code: constants.UNSUPPORTED + }; + }; + + const invalidArgError = (flag, arg, expectedType = 'string') => { + return { + message: `Mocha flag '--${flag}' given invalid option: '${arg}'`, + code: constants.INVALID_ARG_TYPE, + argument: arg, + actual: 'number', + expected: expectedType + }; + }; + beforeEach(function () { readFileSync = sinon.stub(); findConfig = sinon.stub(); @@ -691,16 +709,51 @@ describe('options', function () { }); }); - it('throws error when numeric option is passed to cli', function () { - expect(() => loadOptions(`${numericArg}`), 'to throw', { - message: `Invalid option ${numericArg} passed to mocha cli` - }); + it('throws UNSUPPORTED error when numeric option is passed to cli', function () { + expect( + () => loadOptions(`${numericArg}`), + 'to throw', + unsupportedError(numericArg) + ); }); - it('throws error when numeric argument is passed to mocha flag that does not accept numeric value', function () { - expect(() => loadOptions(`--delay ${numericArg}`), 'to throw', { - message: `Invalid option ${numericArg} passed to mocha cli` - }); + it('throws INVALID_ARG_TYPE error when numeric argument is passed to mocha flag that does not accept numeric value', function () { + const flag = '--delay'; + expect( + () => loadOptions(`${flag} ${numericArg}`), + 'to throw', + invalidArgError(flag, numericArg, 'boolean') + ); + }); + + it('throws INVALID_ARG_TYPE error when incompatible flag does not have preceding "--"', function () { + const flag = 'delay'; + expect( + () => loadOptions(`${flag} ${numericArg}`), + 'to throw', + invalidArgError(flag, numericArg, 'boolean') + ); + }); + + it('shows correct flag in error when multiple mocha flags have numeric values', function () { + const flag = '--delay'; + expect( + () => + loadOptions( + `--timeout ${numericArg} ${flag} ${numericArg} --retries ${numericArg}` + ), + 'to throw', + invalidArgError(flag, numericArg, 'boolean') + ); + }); + + it('throws UNSUPPORTED error when numeric arg is passed to unsupported flag', function () { + const invalidFlag = 'foo'; + expect( + () => loadOptions(`${invalidFlag} ${numericArg}`), + 'to throw', + unsupportedError(numericArg) + ); }); it('does not throw error if numeric value is passed to a compatible mocha flag', function () { diff --git a/test/node-unit/utils.spec.js b/test/node-unit/utils.spec.js index 1381244a33..d910de496c 100644 --- a/test/node-unit/utils.spec.js +++ b/test/node-unit/utils.spec.js @@ -45,5 +45,16 @@ describe('utils', function () { ); }); }); + describe('isNumeric()', function () { + it('returns true for a number type', function () { + expect(utils.isNumeric(42), 'to equal', true); + }); + it('returns true for a string that can be parsed as a number', function () { + expect(utils.isNumeric('42'), 'to equal', true); + }); + it('returns false for a string that cannot be parsed as a number', function () { + expect(utils.isNumeric('foo'), 'to equal', false); + }); + }); }); });