From 410a116dc22da0f4fa244241b6e045dfc93bd5a9 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Sun, 24 Feb 2019 21:04:39 -0800 Subject: [PATCH] fix handling of bareword args matching node flags; closes #3761 --- .wallaby.js | 2 - lib/cli/node-flags.js | 32 ++- lib/cli/options.js | 5 +- test/integration/_helpers.js | 298 ++++++++++++++++++++ test/integration/options/node-flags.spec.js | 19 ++ test/node-unit/cli/node-flags.spec.js | 12 + 6 files changed, 356 insertions(+), 12 deletions(-) create mode 100644 test/integration/_helpers.js create mode 100644 test/integration/options/node-flags.spec.js diff --git a/.wallaby.js b/.wallaby.js index a54f3576fc..d6adef99e0 100644 --- a/.wallaby.js +++ b/.wallaby.js @@ -35,8 +35,6 @@ module.exports = () => { setup(wallaby) { // running mocha instance is not the same as mocha under test, // running mocha is the project's source code mocha, mocha under test is instrumented version of the source code - const runningMocha = wallaby.testFramework; - runningMocha.timeout(200); // to expose it/describe etc. on the mocha under test const MochaUnderTest = require('./'); const mochaUnderTest = new MochaUnderTest(); diff --git a/lib/cli/node-flags.js b/lib/cli/node-flags.js index c2c77c54b0..33658e44f5 100644 --- a/lib/cli/node-flags.js +++ b/lib/cli/node-flags.js @@ -29,16 +29,32 @@ const debugFlags = new Set(['debug', 'debug-brk', 'inspect', 'inspect-brk']); * - `--v8-*` (but *not* `--v8-options`) * @summary Whether or not to pass a flag along to the `node` executable. * @param {string} flag - Flag to test - * @returns {boolean} + * @param {boolean} [bareword=true] - If `false`, we expect `flag` to have one or two leading dashes. + * @returns {boolean} If the flag is considered a "Node" flag. * @private */ -exports.isNodeFlag = flag => - !/^(?:require|r)$/.test(flag) && - (nodeFlags.has(flag) || - debugFlags.has(flag) || - /(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test( - flag - )); +exports.isNodeFlag = (flag, bareword = true) => { + if (!bareword) { + // check if the flag begins with dashes; if not, not a node flag. + if (!/^--?/.test(flag)) { + return false; + } + // strip the leading dashes to match against subsequent regexes + flag = flag.replace(/^--?/, ''); + } + return ( + // treat --require/-r as Mocha flag even though it's also a node flag + !/^r(?:equire)?$/.test(flag) && + // check actual node flags from `process.allowedNodeEnvironmentFlags`, + // then historical support for various V8 and non-`NODE_OPTIONS` flags + // and also any V8 flags with `--v8-` prefix + (nodeFlags.has(flag) || + debugFlags.has(flag) || + /(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test( + flag + )) + ); +}; /** * Returns `true` if the flag is a "debug-like" flag. These require timeouts diff --git a/lib/cli/options.js b/lib/cli/options.js index e332c58c89..1e72dee034 100644 --- a/lib/cli/options.js +++ b/lib/cli/options.js @@ -85,8 +85,9 @@ const parse = (args = [], ...configObjects) => { const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce( (acc, arg) => { const pair = arg.split('='); - const flag = pair[0].replace(/^--?/, ''); - if (isNodeFlag(flag)) { + let flag = pair[0]; + if (isNodeFlag(flag, false)) { + flag = flag.replace(/^--?/, ''); return arg.includes('=') ? acc.concat([[flag, pair[1]]]) : acc.concat([[flag, true]]); diff --git a/test/integration/_helpers.js b/test/integration/_helpers.js new file mode 100644 index 0000000000..b774bb7890 --- /dev/null +++ b/test/integration/_helpers.js @@ -0,0 +1,298 @@ +'use strict'; + +var format = require('util').format; +var spawn = require('cross-spawn').spawn; +var path = require('path'); +var Base = require('../../lib/reporters/base'); + +var DEFAULT_FIXTURE = resolveFixturePath('__default__'); +var MOCHA_EXECUTABLE = require.resolve('../../bin/mocha'); +var _MOCHA_EXECUTABLE = require.resolve('../../bin/_mocha'); + +module.exports = { + DEFAULT_FIXTURE: DEFAULT_FIXTURE, + /** + * Invokes the mocha binary for the given fixture with color output disabled. + * Accepts an array of additional command line args to pass. The callback is + * invoked with a summary of the run, in addition to its output. The summary + * includes the number of passing, pending, and failing tests, as well as the + * exit code. Useful for testing different reporters. + * + * By default, `STDERR` is ignored. Pass `{stdio: 'pipe'}` as `opts` if you + * want it. + * Example response: + * { + * pending: 0, + * passing: 0, + * failing: 1, + * code: 1, + * output: '...' + * } + * + * @param {string} fixturePath - Path to fixture .js file + * @param {string[]} args - Extra args to mocha executable + * @param {Function} fn - Callback + * @param {Object} [opts] - Options for `spawn()` + */ + runMocha: function(fixturePath, args, fn, opts) { + if (typeof args === 'function') { + opts = fn; + fn = args; + args = []; + } + + var path; + + path = resolveFixturePath(fixturePath); + args = args || []; + + invokeSubMocha( + args.concat(['-C', path]), + function(err, res) { + if (err) { + return fn(err); + } + + fn(null, getSummary(res)); + }, + opts + ); + }, + + /** + * Invokes the mocha binary for the given fixture using the JSON reporter, + * returning the parsed output, as well as exit code. + * + * By default, `STDERR` is ignored. Pass `{stdio: 'pipe'}` as `opts` if you + * want it. + * @param {string} fixturePath - Path from __dirname__ + * @param {string[]} args - Array of args + * @param {Function} fn - Callback + * @param {Object} [opts] - Opts for `spawn()` + * @returns {*} Parsed object + */ + runMochaJSON: function(fixturePath, args, fn, opts) { + if (typeof args === 'function') { + opts = fn; + fn = args; + args = []; + } + + var path; + + path = resolveFixturePath(fixturePath); + args = (args || []).concat('--reporter', 'json', path); + + return invokeMocha( + args, + function(err, res) { + if (err) { + return fn(err); + } + + var result; + try { + // attempt to catch a JSON parsing error *only* here. + // previously, the callback was called within this `try` block, + // which would result in errors thrown from the callback + // getting caught by the `catch` block below. + result = toJSONRunResult(res); + } catch (err) { + return fn( + new Error( + format( + 'Failed to parse JSON reporter output. Error:\n%O\nResult:\n%O', + err, + res + ) + ) + ); + } + fn(null, result); + }, + opts + ); + }, + /** + * Invokes the mocha binary for the given fixture using the JSON reporter, + * returning the **raw** string output, as well as exit code. + * + * By default, `STDERR` is ignored. Pass `{stdio: 'pipe'}` as `opts` if you + * want it. + * @param {string} fixturePath - Path from __dirname__ + * @param {string[]} args - Array of args + * @param {Function} fn - Callback + * @param {Object} [opts] - Opts for `spawn()` + * @returns {string} Raw output + */ + runMochaJSONRaw: function(fixturePath, args, fn, opts) { + var path; + + path = resolveFixturePath(fixturePath); + args = args || []; + + return invokeSubMocha( + args.concat(['--reporter', 'json', path]), + function(err, resRaw) { + if (err) return fn(err); + + fn(null, resRaw); + }, + opts + ); + }, + + /** + * regular expression used for splitting lines based on new line / dot symbol. + */ + splitRegExp: new RegExp('[\\n' + Base.symbols.dot + ']+'), + + /** + * Invokes the mocha binary. Accepts an array of additional command line args + * to pass. The callback is invoked with the exit code and output. Optional + * current working directory as final parameter. + * + * By default, `STDERR` is ignored. Pass `{stdio: 'pipe'}` as `opts` if you + * want it. + * + * In most cases runMocha should be used instead. + * + * Example response: + * { + * code: 1, + * output: '...' + * } + * + * @param {Array} args - Extra args to mocha executable + * @param {Function} done - Callback + * @param {Object} [opts] - Options for `spawn()` + */ + invokeMocha: invokeMocha, + + /** + * Resolves the path to a fixture to the full path. + */ + resolveFixturePath: resolveFixturePath, + + toJSONRunResult: toJSONRunResult, + + /** + * Given a regexp-like string, escape it so it can be used with the `RegExp` constructor + * @param {string} str - string to be escaped + * @returns {string} Escaped string + */ + escapeRegExp: function escapeRegExp(str) { + return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); // $& means the whole matched string + } +}; + +/** + * Coerce output as returned by _spawnMochaWithListeners using JSON reporter into a JSONRunResult as + * recognized by our custom unexpected assertions + * @param {string} result - Raw stdout from Mocha run using JSON reporter + * @private + */ +function toJSONRunResult(result) { + var code = result.code; + result = JSON.parse(result.output); + result.code = code; + return result; +} + +/** + * Creates arguments loading a default fixture if none provided + * + * @param {string[]|*} [args] - Arguments to `spawn` + * @returns string[] + */ +function defaultArgs(args) { + return !args || !args.length ? ['--file', DEFAULT_FIXTURE] : args; +} + +function invokeMocha(args, fn, opts) { + if (typeof args === 'function') { + opts = fn; + fn = args; + args = []; + } + return _spawnMochaWithListeners( + defaultArgs([MOCHA_EXECUTABLE].concat(args)), + fn, + opts + ); +} + +function invokeSubMocha(args, fn, opts) { + if (typeof args === 'function') { + opts = fn; + fn = args; + args = []; + } + return _spawnMochaWithListeners( + defaultArgs([_MOCHA_EXECUTABLE].concat(args)), + fn, + opts + ); +} + +/** + * Spawns Mocha in a subprocess and returns an object containing its output and exit code + * + * @param {string[]} args - Path to executable and arguments + * @param {Function} fn - Callback + * @param {Object|string} [opts] - Options to `cross-spawn`, or 'pipe' for shortcut to `{stdio: pipe}` + * @returns {ChildProcess} + * @private + */ +function _spawnMochaWithListeners(args, fn, opts) { + var output = ''; + if (opts === 'pipe') { + opts = {stdio: 'pipe'}; + } + opts = Object.assign( + { + cwd: process.cwd(), + stdio: ['ignore', 'pipe', 'ignore'] + }, + opts || {} + ); + var mocha = spawn(process.execPath, args, opts); + var listener = function(data) { + output += data; + }; + + mocha.stdout.on('data', listener); + if (mocha.stderr) { + mocha.stderr.on('data', listener); + } + mocha.on('error', fn); + + mocha.on('close', function(code) { + fn(null, { + output: output, + code: code, + args: args + }); + }); + + return mocha; +} + +function resolveFixturePath(fixture) { + if (path.extname(fixture) !== '.js') { + fixture += '.fixture.js'; + } + return path.join('test', 'integration', 'fixtures', fixture); +} + +function getSummary(res) { + return ['passing', 'pending', 'failing'].reduce(function(summary, type) { + var pattern, match; + + pattern = new RegExp(' (\\d+) ' + type + '\\s'); + match = pattern.exec(res.output); + summary[type] = match ? parseInt(match, 10) : 0; + + return summary; + }, res); +} diff --git a/test/integration/options/node-flags.spec.js b/test/integration/options/node-flags.spec.js new file mode 100644 index 0000000000..b884482527 --- /dev/null +++ b/test/integration/options/node-flags.spec.js @@ -0,0 +1,19 @@ +'use strict'; + +var invokeMocha = require('../helpers').invokeMocha; + +describe('node flags', function() { + it('should not consider argument values to be node flags', function(done) { + invokeMocha( + ['--require', 'trace-dependency'], + function(err, res) { + if (err) { + return done(err); + } + expect(res, 'not to have failed with output', /bad option/i); + done(); + }, + 'pipe' + ); + }); +}); diff --git a/test/node-unit/cli/node-flags.spec.js b/test/node-unit/cli/node-flags.spec.js index 22868fe584..b118d5d5e6 100644 --- a/test/node-unit/cli/node-flags.spec.js +++ b/test/node-unit/cli/node-flags.spec.js @@ -15,6 +15,18 @@ describe('node-flags', function() { }); }); + describe('when expecting leading dashes', function() { + it('should require leading dashes', function() { + expect(isNodeFlag('throw-deprecation', false), 'to be false'); + expect(isNodeFlag('--throw-deprecation', false), 'to be true'); + }); + + it('should return false for --require/-r', function() { + expect(isNodeFlag('--require', false), 'to be false'); + expect(isNodeFlag('-r', false), 'to be false'); + }); + }); + describe('special cases', function() { it('should return true for flags starting with "preserve-symlinks"', function() { expect(isNodeFlag('preserve-symlinks'), 'to be true');