From 21ba5ce8b2be76277a690d3e164948754e94c1e6 Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Fri, 8 Feb 2019 11:01:20 -0800 Subject: [PATCH] fix --inspect and its ilk; closes #3681 (#3699) This seems to make `--inspect` work properly with or without a parameter (`--inspect=0.0.0.0:12345`). The `=` is required, as it's required by `node`. In v6 of Node, `--debug --port=12345` should also work as expected. Due to ignorance, skip Windows for now --- .mocharc.yml | 2 +- bin/mocha | 27 ++++--- lib/cli/node-flags.js | 21 +++++ lib/cli/options.js | 49 +++++++++--- package-lock.json | 6 +- package.json | 2 +- test/assertions.js | 12 ++- test/integration/helpers.js | 8 +- test/integration/options/debug.spec.js | 106 ++++++++++++++++++++++++- 9 files changed, 194 insertions(+), 39 deletions(-) diff --git a/.mocharc.yml b/.mocharc.yml index bcfa0f5b7b..fc4c97339c 100644 --- a/.mocharc.yml +++ b/.mocharc.yml @@ -4,4 +4,4 @@ global: - okGlobalA,okGlobalB - okGlobalC - callback* -timeout: 200 +timeout: 300 diff --git a/bin/mocha b/bin/mocha index 9acf079f54..cec93a3366 100755 --- a/bin/mocha +++ b/bin/mocha @@ -12,10 +12,15 @@ 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 nodeEnv = require('node-environment-flags'); const mochaPath = require.resolve('./_mocha'); const mochaArgs = {}; @@ -32,7 +37,7 @@ debug('loaded opts', opts); const disableTimeouts = value => { if (impliesNoTimeouts(value)) { debug(`option "${value}" disabled timeouts`); - mochaArgs.timeout = false; + mochaArgs.timeout = 0; delete mochaArgs.timeouts; delete mochaArgs.t; } @@ -45,13 +50,12 @@ const disableTimeouts = value => { * @ignore */ const trimV8Option = value => - value !== 'v8-options' && /^v8-/.test(value) ? value.slice(2) : value; + value !== 'v8-options' && /^v8-/.test(value) ? value.slice(3) : value; // sort options into "node" and "mocha" buckets Object.keys(opts).forEach(opt => { - opt = trimV8Option(opt); if (isNodeFlag(opt)) { - nodeArgs[opt] = opts[opt]; + nodeArgs[trimV8Option(opt)] = opts[opt]; disableTimeouts(opt); } else { mochaArgs[opt] = opts[opt]; @@ -90,9 +94,8 @@ if (/^(debug|inspect)$/.test(mochaArgs._[0])) { } // allow --debug to invoke --inspect on Node.js v8 or newer. -// these show up in childOpts because they are not recognized as valid node flags in this version of node. ['debug', 'debug-brk'] - .filter(opt => opt in mochaArgs) + .filter(opt => opt in nodeArgs && !nodeEnv.has(opt)) .forEach(opt => { const newOpt = opt === 'debug' ? 'inspect' : 'inspect-brk'; warn( @@ -100,10 +103,10 @@ if (/^(debug|inspect)$/.test(mochaArgs._[0])) { process.version }; use "--${newOpt}" instead.` ); - nodeArgs[newOpt] = mochaArgs[opt]; + nodeArgs[newOpt] = nodeArgs[opt]; mochaArgs.timeout = false; debug(`--${opt} -> ${newOpt}`); - delete mochaArgs[opt]; + delete nodeArgs[opt]; }); // historical @@ -115,8 +118,10 @@ if (nodeArgs.gc) { delete nodeArgs.gc; } +debug('final node args', nodeArgs); + const args = [].concat( - unparse(nodeArgs), + unparseNodeFlags(nodeArgs), mochaPath, unparse(mochaArgs, {alias: aliases}) ); diff --git a/lib/cli/node-flags.js b/lib/cli/node-flags.js index 9b4fd09d3c..c2c77c54b0 100644 --- a/lib/cli/node-flags.js +++ b/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. @@ -34,6 +35,7 @@ const debugFlags = new Set(['debug', 'debug-brk', 'inspect', 'inspect-brk']); 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 )); @@ -46,3 +48,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('') + : []; +}; diff --git a/lib/cli/options.js b/lib/cli/options.js index 854120418f..e332c58c89 100644 --- a/lib/cli/options.js +++ b/lib/cli/options.js @@ -18,6 +18,7 @@ const findup = require('findup-sync'); const {deprecate} = require('../utils'); const debug = require('debug')('mocha:cli:options'); const {createMissingArgumentError} = require('../errors'); +const {isNodeFlag} = require('./node-flags'); /** * The `yargs-parser` namespace @@ -75,22 +76,46 @@ const nargOpts = types.array * @ignore */ const parse = (args = [], ...configObjects) => { - const result = yargsParser.detailed( - args, - Object.assign( - { - configuration, - configObjects, - coerce: coerceOpts, - narg: nargOpts, - alias: aliases - }, - types - ) + // 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 + // 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 nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce( + (acc, arg) => { + const pair = arg.split('='); + const flag = pair[0].replace(/^--?/, ''); + if (isNodeFlag(flag)) { + return arg.includes('=') + ? acc.concat([[flag, pair[1]]]) + : acc.concat([[flag, true]]); + } + return acc; + }, + [] ); + + const result = yargsParser.detailed(args, { + configuration, + configObjects, + coerce: coerceOpts, + narg: nargOpts, + alias: aliases, + string: types.string, + array: types.array, + number: types.number, + boolean: types.boolean.concat(nodeArgs.map(pair => pair[0])) + }); if (result.error) { throw createMissingArgumentError(result.error.message); } + + // reapply "=" arg values from above + nodeArgs.forEach(([key, value]) => { + result.argv[key] = value; + }); + return result.argv; }; diff --git a/package-lock.json b/package-lock.json index ecebbdc3f3..8ecbb86ace 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13633,9 +13633,9 @@ } }, "node-environment-flags": { - "version": "1.0.2", - "resolved": "https://registry.npmjs.org/node-environment-flags/-/node-environment-flags-1.0.2.tgz", - "integrity": "sha512-5SQqz9JbLi9OOc/lsPVLB2S5Rezhy7tdCjct3mm7dP9Tmd3T/N8eQ01XRmKPgkugrB9M+mfTqmAphWcRlORBZQ==", + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/node-environment-flags/-/node-environment-flags-1.0.4.tgz", + "integrity": "sha512-M9rwCnWVLW7PX+NUWe3ejEdiLYinRpsEre9hMkU/6NS4h+EEulYaDH1gCEZ2gyXsmw+RXYDaV2JkkTNcsPDJ0Q==", "requires": { "object.getownpropertydescriptors": "^2.0.3" } diff --git a/package.json b/package.json index 154438b0bc..1067be24e0 100644 --- a/package.json +++ b/package.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", diff --git a/test/assertions.js b/test/assertions.js index fac66ce94a..4ed725a0d4 100644 --- a/test/assertions.js +++ b/test/assertions.js @@ -9,7 +9,7 @@ exports.mixinMochaAssertions = function(expect) { return ( Object.prototype.toString.call(v) === '[object Object]' && typeof v.output === 'string' && - typeof v.code === 'number' && + 'code' in v && // may be null Array.isArray(v.args) ); } @@ -59,9 +59,9 @@ exports.mixinMochaAssertions = function(expect) { } ) .addAssertion( - ' [not] to have [completed with] [exit] code ', + ' [not] to have completed with [exit] code ', function(expect, result, code) { - expect(result, '[not] to have property', 'code', code); + expect(result.code, '[not] to be', code); } ) .addAssertion( @@ -295,5 +295,11 @@ exports.mixinMochaAssertions = function(expect) { function(expect, result, output) { expect(result.output, '[not] to satisfy', output); } + ) + .addAssertion( + ' to have [exit] code ', + function(expect, result, code) { + expect(result.code, 'to be', code); + } ); }; diff --git a/test/integration/helpers.js b/test/integration/helpers.js index b2e98434c7..bba6f1ff5c 100644 --- a/test/integration/helpers.js +++ b/test/integration/helpers.js @@ -81,10 +81,10 @@ module.exports = { var path; path = resolveFixturePath(fixturePath); - args = args || []; + args = (args || []).concat('--reporter', 'json', path); return invokeMocha( - args.concat(['--reporter', 'json', path]), + args, function(err, res) { if (err) return fn(err); @@ -95,8 +95,8 @@ module.exports = { fn( new Error( format( - 'Failed to parse JSON reporter output.\nArgs: %O\nResult:\n\n%O', - args, + 'Failed to parse JSON reporter output. Error:\n%O\nResponse:\n%O', + err, res ) ) diff --git a/test/integration/options/debug.spec.js b/test/integration/options/debug.spec.js index 5c073156da..e7518283c4 100644 --- a/test/integration/options/debug.spec.js +++ b/test/integration/options/debug.spec.js @@ -14,19 +14,117 @@ 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); } - expect(res, 'to have passed').and( + expect(res, 'to contain output', /Debugger listening/i); + done(); + }, + '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 contain output', /Debugger listening/i); + done(); + }, + 'pipe' + ); + + // debugger must be manually killed + setTimeout(function() { + process.kill(proc.pid, 'SIGINT'); + }, 2000); + }); + + 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 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 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 contain output', /Debugger listening/i); + done(); + }, + 'pipe' + ); + + // debugger must be manually killed + setTimeout(function() { + process.kill(proc.pid, 'SIGINT'); + }, 2000); + }); + + 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 contain output', - /Debugger listening/i + /Debugger listening on .*127.0.0.1:9229/i ); done(); }, - {stdio: 'pipe'} + 'pipe' ); + + setTimeout(function() { + process.kill(proc.pid, 'SIGINT'); + }, 2000); }); }); });