Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

Commit

Permalink
fix --inspect and its ilk; closes mochajs#3681 (mochajs#3699)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
boneskull authored Feb 8, 2019
1 parent c88d091 commit da1c3be
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .mocharc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ global:
- okGlobalA,okGlobalB
- okGlobalC
- callback*
timeout: 200
timeout: 300
27 changes: 16 additions & 11 deletions bin/mocha
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {};
Expand All @@ -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;
}
Expand All @@ -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];
Expand Down Expand Up @@ -90,20 +94,19 @@ 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(
`"--${opt}" is not available in Node.js ${
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
Expand All @@ -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})
);
Expand Down
21 changes: 21 additions & 0 deletions lib/cli/node-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

const nodeFlags = require('node-environment-flags');
const unparse = require('yargs-unparser');

/**
* These flags are considered "debug" flags.
Expand Down Expand Up @@ -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
));
Expand All @@ -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('')
: [];
};
49 changes: 37 additions & 12 deletions lib/cli/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
};

Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 9 additions & 3 deletions test/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
Expand Down Expand Up @@ -59,9 +59,9 @@ exports.mixinMochaAssertions = function(expect) {
}
)
.addAssertion(
'<RawResult|RawRunResult|JSONRunResult> [not] to have [completed with] [exit] code <number>',
'<RawRunResult|JSONRunResult> [not] to have completed with [exit] code <number>',
function(expect, result, code) {
expect(result, '[not] to have property', 'code', code);
expect(result.code, '[not] to be', code);
}
)
.addAssertion(
Expand Down Expand Up @@ -295,5 +295,11 @@ exports.mixinMochaAssertions = function(expect) {
function(expect, result, output) {
expect(result.output, '[not] to satisfy', output);
}
)
.addAssertion(
'<RawRunResult|JSONRunResult> to have [exit] code <number>',
function(expect, result, code) {
expect(result.code, 'to be', code);
}
);
};
8 changes: 4 additions & 4 deletions test/integration/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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
)
)
Expand Down
106 changes: 102 additions & 4 deletions test/integration/options/debug.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});

0 comments on commit da1c3be

Please sign in to comment.