fix handling of bareword args matching node flags; closes #3761

.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();

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

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]]);

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<string>} 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);
+}

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'
+    );
+  });
+});

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');