fix --inspect and its ilk; closes #3681

- upgrade `node-environment-flags`
- add some tests
- use `mocha:cli:mocha` in `bin/mocha` as unique `DEBUG` identifier (`DEBUG=mocha:cli:mocha bin/mocha ...`)
- add some handiness to the integration test helpers
- update docstrings in integration test helpers

Author: boneskull

bin/mocha

@@ -12,9 +12,13 @@
 const {deprecate, warn} = require('../lib/utils');
 const {spawn} = require('child_process');
 const {loadOptions} = require('../lib/cli/options');
-const {isNodeFlag, impliesNoTimeouts} = require('../lib/cli/node-flags');
+const {
+  unparseNodeFlags,
+  isNodeFlag,
+  impliesNoTimeouts
+} = require('../lib/cli/node-flags');
 const unparse = require('yargs-unparser');
-const debug = require('debug')('mocha:cli');
+const debug = require('debug')('mocha:cli:mocha');
 const {aliases} = require('../lib/cli/run-option-metadata');
 
 const mochaPath = require.resolve('./_mocha');
@@ -115,8 +119,10 @@ if (nodeArgs.gc) {
   delete nodeArgs.gc;
 }
 
+debug('final node args', nodeArgs);
+
 const args = [].concat(
-  unparse(nodeArgs),
+  unparseNodeFlags(nodeArgs),
   mochaPath,
   unparse(mochaArgs, {alias: aliases})
 );

lib/cli/node-flags.js

@@ -7,6 +7,7 @@
  */
 
 const nodeFlags = require('node-environment-flags');
+const unparse = require('yargs-unparser');
 
 /**
  * These flags are considered "debug" flags.
@@ -46,3 +47,22 @@ exports.isNodeFlag = flag =>
  * @private
  */
 exports.impliesNoTimeouts = flag => debugFlags.has(flag);
+
+/**
+ * All non-strictly-boolean arguments to node--those with values--must specify those values using `=`, e.g., `--inspect=0.0.0.0`.
+ * Unparse these arguments using `yargs-unparser` (which would result in `--inspect 0.0.0.0`), then supply `=` where we have values.
+ * There's probably an easier or more robust way to do this; fixes welcome
+ * @param {Object} opts - Arguments object
+ * @returns {string[]} Unparsed arguments using `=` to specify values
+ * @private
+ */
+exports.unparseNodeFlags = opts => {
+  var args = unparse(opts);
+  return args.length
+    ? args
+        .join(' ')
+        .split(/\b/)
+        .map(arg => (arg === ' ' ? '=' : arg))
+        .join('')
+    : [];
+};

lib/cli/options.js

@@ -259,6 +259,13 @@ module.exports.loadPkgRc = loadPkgRc;
  * @returns {external:yargsParser.Arguments} Parsed args from everything
  */
 const loadOptions = (argv = []) => {
+  // save node-specific args containing a '=' to reapply below
+  const nodeArgOptionalValues = new Map(
+    (Array.isArray(argv) ? argv : argv.split(' '))
+      .filter(arg => arg.includes('='))
+      .map(arg => arg.substring(2).split('='))
+  );
+
   let args = parse(argv);
   // short-circuit: look for a flag that would abort loading of mocha.opts
   if (
@@ -299,6 +306,11 @@ const loadOptions = (argv = []) => {
     delete args.spec;
   }
 
+  // reapply "=" arg values from above
+  nodeArgOptionalValues.forEach((value, key) => {
+    args[key] = value;
+  });
+
   return args;
 };
 

lib/cli/run-option-metadata.js

@@ -42,7 +42,15 @@ exports.types = {
     'recursive',
     'reporters',
     'sort',
-    'watch'
+    'watch',
+
+    // these are special-cased args for node which have optional values.
+    // if we don't treat them as boolean, they get greedy and might eat subsequent positional arguments.
+    // instead, we will save their "real" values before parsing via yargs-parser.
+    'debug',
+    'debug-brk',
+    'inspect',
+    'inspect-brk'
   ],
   number: ['retries', 'slow', 'timeout'],
   string: ['fgrep', 'grep', 'package', 'reporter', 'ui']

package-lock.json

@@ -498,7 +498,7 @@
     "minimatch": "3.0.4",
     "mkdirp": "0.5.1",
     "ms": "2.1.1",
-    "node-environment-flags": "1.0.2",
+    "node-environment-flags": "1.0.4",
     "object.assign": "4.1.0",
     "strip-json-comments": "2.0.1",
     "supports-color": "6.0.0",

package.json

@@ -14,7 +14,7 @@ describe('--debug', function() {
 
     it('should invoke --inspect', function(done) {
       invokeMocha(
-        ['--debug', '--file', DEFAULT_FIXTURE],
+        ['--debug', DEFAULT_FIXTURE],
         function(err, res) {
           if (err) {
             return done(err);
@@ -25,8 +25,116 @@ describe('--debug', function() {
           );
           done();
         },
-        {stdio: 'pipe'}
+        'pipe'
       );
     });
+
+    it('should invoke --inspect-brk', function(done) {
+      var proc = invokeMocha(
+        ['--debug-brk', DEFAULT_FIXTURE],
+        function(err, res) {
+          if (err) {
+            return done(err);
+          }
+          expect(res, 'to have passed').and(
+            'to contain output',
+            /Debugger listening/i
+          );
+          done();
+        },
+        'pipe'
+      );
+
+      // debugger must be manually killed
+      setTimeout(function() {
+        proc.kill('SIGINT');
+      }, 1000);
+    });
+
+    it('should respect custom host/port', function(done) {
+      invokeMocha(
+        ['--debug=127.0.0.1:9229', DEFAULT_FIXTURE],
+        function(err, res) {
+          if (err) {
+            return done(err);
+          }
+          expect(res, 'to have passed').and(
+            'to contain output',
+            /Debugger listening on .*127.0.0.1:9229/i
+          );
+          done();
+        },
+        'pipe'
+      );
+    });
+
+    it('should warn about incorrect usage for version', function(done) {
+      invokeMocha(
+        ['--debug=127.0.0.1:9229', DEFAULT_FIXTURE],
+        function(err, res) {
+          if (err) {
+            return done(err);
+          }
+          expect(res, 'to have passed').and(
+            'to contain output',
+            /"--debug" is not available/i
+          );
+          done();
+        },
+        'pipe'
+      );
+    });
+  });
+
+  describe('Node.js v6', function() {
+    // note that v6.3.0 and newer supports --inspect but still supports --debug.
+    before(function() {
+      if (process.version.substring(0, 2) !== 'v6') {
+        this.skip();
+      }
+    });
+
+    it('should start debugger', function(done) {
+      var proc = invokeMocha(
+        ['--debug', DEFAULT_FIXTURE],
+        function(err, res) {
+          if (err) {
+            return done(err);
+          }
+          expect(res, 'to have passed').and(
+            'to contain output',
+            /Debugger listening/i
+          );
+          done();
+        },
+        'pipe'
+      );
+
+      // debugger must be manually killed
+      setTimeout(function() {
+        proc.kill('SIGINT');
+      }, 1000);
+    });
+
+    it('should respect custom host/port', function(done) {
+      var proc = invokeMocha(
+        ['--debug=127.0.0.1:9229', DEFAULT_FIXTURE],
+        function(err, res) {
+          if (err) {
+            return done(err);
+          }
+          expect(res, 'to have passed').and(
+            'to contain output',
+            /Debugger listening on .*127.0.0.1:9229/i
+          );
+          done();
+        },
+        'pipe'
+      );
+
+      setTimeout(function() {
+        proc.kill('SIGINT');
+      }, 1000);
+    });
   });
 });