Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new option "node-option" #4691

Merged
merged 5 commits into from Jul 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 18 additions & 27 deletions bin/mocha
Expand Up @@ -10,7 +10,6 @@
* @private
*/

const {deprecate} = require('../lib/utils');
const {loadOptions} = require('../lib/cli/options');
const {
unparseNodeFlags,
Expand All @@ -23,6 +22,7 @@ const {aliases} = require('../lib/cli/run-option-metadata');

const mochaArgs = {};
const nodeArgs = {};
let hasInspect = false;

const opts = loadOptions(process.argv.slice(2));
debug('loaded opts', opts);
Expand Down Expand Up @@ -59,48 +59,39 @@ Object.keys(opts).forEach(opt => {

// disable 'timeout' for debugFlags
Object.keys(nodeArgs).forEach(opt => disableTimeouts(opt));
mochaArgs['node-option'] &&
mochaArgs['node-option'].forEach(opt => disableTimeouts(opt));

// Native debugger handling
// see https://nodejs.org/api/debugger.html#debugger_debugger
// look for 'inspect' or 'debug' that would launch this debugger,
// look for 'inspect' that would launch this debugger,
// remove it from Mocha's opts and prepend it to Node's opts.
// A deprecation warning will be printed by node, if applicable.
// (mochaArgs._ are "positional" arguments, not prefixed with - or --)
if (mochaArgs._) {
const i = mochaArgs._.findIndex(val => val === 'inspect' || val === 'debug');
const i = mochaArgs._.findIndex(val => val === 'inspect');
if (i > -1) {
const [command] = mochaArgs._.splice(i, 1);
mochaArgs._.splice(i, 1);
disableTimeouts('inspect');
nodeArgs._ = [command];
hasInspect = true;
}
}

// historical
if (nodeArgs.gc) {
deprecate(
'"-gc" is deprecated and will be removed from a future version of Mocha. Use "--gc-global" instead.'
);
nodeArgs['gc-global'] = nodeArgs.gc;
delete nodeArgs.gc;
}

// --require/-r is treated as Mocha flag except when 'esm' is preloaded
if (mochaArgs.require && mochaArgs.require.includes('esm')) {
nodeArgs.require = ['esm'];
mochaArgs.require = mochaArgs.require.filter(mod => mod !== 'esm');
if (!mochaArgs.require.length) {
delete mochaArgs.require;
}
}

if (Object.keys(nodeArgs).length) {
if (mochaArgs['node-option'] || Object.keys(nodeArgs).length || hasInspect) {
const {spawn} = require('child_process');
const mochaPath = require.resolve('../lib/cli/cli.js');

debug('final node args', nodeArgs);
const nodeArgv =
(mochaArgs['node-option'] && mochaArgs['node-option'].map(v => '--' + v)) ||
unparseNodeFlags(nodeArgs);

if (hasInspect) nodeArgv.unshift('inspect');
delete mochaArgs['node-option'];

debug('final node argv', nodeArgv);

const args = [].concat(
unparseNodeFlags(nodeArgs),
nodeArgv,
mochaPath,
unparse(mochaArgs, {alias: aliases})
);
Expand Down Expand Up @@ -147,5 +138,5 @@ if (Object.keys(nodeArgs).length) {
});
} else {
debug('running Mocha in-process');
require('../lib/cli/cli').main(unparse(mochaArgs, {alias: aliases}));
require('../lib/cli/cli').main([], mochaArgs);
}
19 changes: 14 additions & 5 deletions docs/index.md
Expand Up @@ -876,9 +876,7 @@ Use this option to have Mocha check for global variables that are leaked while r

### `--dry-run`

> _New in v9.0.0._

Report tests without executing any of them, neither tests nor hooks.
> _New in v9.0.0._ Report tests without executing any of them, neither tests nor hooks.

### `--exit`

Expand Down Expand Up @@ -1015,6 +1013,15 @@ Specify an explicit path to a [configuration file](#configuring-mocha-nodejs).

By default, Mocha will search for a config file if `--config` is not specified; use `--no-config` to suppress this behavior.

### `--node-option <name>, -n <name>`

> _New in v9.1.0._

For Node.js and V8 options. Mocha forwards these options to Node.js by spawning a new child-process.<br>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clear, can we specify name should be without -- nor --v8-?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the -- info. I prefer to not mention --v8- as this is a Mocha specific quirk and may be confusing.

The options are set without leading dashes `--`, e.g. `-n require=foo -n unhandled-rejections=strict`

Can also be specified as a comma-delimited list: `-n require=foo,unhandled-rejections=strict`

### `--opts <path>`

> _Removed in v8.0.0. Please use [configuration file](#configuring-mocha-nodejs) instead._
Expand Down Expand Up @@ -1159,8 +1166,6 @@ Requires either `--grep` or `--fgrep` (but not both).

### `--inspect, --inspect-brk, inspect`

> _BREAKING CHANGE in v7.0.0; `--debug` / `--debug-brk` are removed and `debug` is deprecated._

Enables Node.js' inspector.

Use `--inspect` / `--inspect-brk` to launch the V8 inspector for use with Chrome Dev Tools.
Expand Down Expand Up @@ -1209,6 +1214,8 @@ These flags vary depending on your version of Node.js.

`node` flags can be defined in Mocha's [configuration](#configuring-mocha-nodejs).

> _New in v9.1.0._ You can also pass `node` flags to Node.js using [`--node-option`](#-node-option-name-n-name).

### `--enable-source-maps`

> _New in Node.js v12.12.0_
Expand All @@ -1228,6 +1235,8 @@ Prepend `--v8-` to any flag listed in the output of `node --v8-options` (excludi

V8 flags can be defined in Mocha's [configuration](#configuring-mocha-nodejs).

> _New in v9.1.0._ You can also pass V8 flags (without `--v8-`) to Node.js using [`--node-option`](#-node-option-name-n-name).

## Parallel Tests

> _New in v.8.0.0._
Expand Down
1 change: 1 addition & 0 deletions example/config/.mocharc.js
Expand Up @@ -26,6 +26,7 @@ module.exports = {
'inline-diffs': false,
// invert: false, // needs to be used with grep or fgrep
jobs: 1,
'node-option': ['unhandled-rejections=strict'], // without leading "--", also V8 flags
package: './package.json',
parallel: false,
recursive: false,
Expand Down
2 changes: 2 additions & 0 deletions example/config/.mocharc.yml
Expand Up @@ -29,6 +29,8 @@ inline-diffs: false
# needs to be used with grep or fgrep
# invert: false
jobs: 1
node-option:
- 'unhandled-rejections=strict' # without leading "--", also V8 flags
package: './package.json'
parallel: false
recursive: false
Expand Down
5 changes: 3 additions & 2 deletions lib/cli/cli.js
Expand Up @@ -33,8 +33,9 @@ const {cwd} = require('../utils');
* @public
* @summary Mocha's main command-line entry-point.
* @param {string[]} argv - Array of arguments to parse, or by default the lovely `process.argv.slice(2)`
* @param {object} [mochaArgs] - Object of already parsed Mocha arguments (by bin/mocha)
*/
exports.main = (argv = process.argv.slice(2)) => {
exports.main = (argv = process.argv.slice(2), mochaArgs) => {
debug('entered main with raw args', argv);
// ensure we can require() from current working directory
if (typeof module.paths !== 'undefined') {
Expand All @@ -43,7 +44,7 @@ exports.main = (argv = process.argv.slice(2)) => {

Error.stackTraceLimit = Infinity; // configurable via --stack-trace-limit?

var args = loadOptions(argv);
var args = mochaArgs || loadOptions(argv);

yargs()
.scriptName('mocha')
Expand Down
7 changes: 2 additions & 5 deletions lib/cli/node-flags.js
Expand Up @@ -49,7 +49,7 @@ exports.isNodeFlag = (flag, bareword = true) => {
// and also any V8 flags with `--v8-` prefix
(!isMochaFlag(flag) && nodeFlags && nodeFlags.has(flag)) ||
debugFlags.has(flag) ||
/(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
/(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc[_-]global$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
flag
)
);
Expand All @@ -67,7 +67,6 @@ 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.
* Apparently --require in Node.js v8 does NOT want `=`.
* 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
Expand All @@ -79,9 +78,7 @@ exports.unparseNodeFlags = opts => {
? args
.join(' ')
.split(/\b/)
.map((arg, index, args) =>
arg === ' ' && args[index - 1] !== 'require' ? '=' : arg
)
.map(arg => (arg === ' ' ? '=' : arg))
.join('')
.split(' ')
: [];
Expand Down
2 changes: 2 additions & 0 deletions lib/cli/run-option-metadata.js
Expand Up @@ -18,6 +18,7 @@ const TYPES = (exports.types = {
'file',
'global',
'ignore',
'node-option',
'reporter-option',
'require',
'spec',
Expand Down Expand Up @@ -79,6 +80,7 @@ exports.aliases = {
invert: ['i'],
jobs: ['j'],
'no-colors': ['C'],
'node-option': ['n'],
parallel: ['p'],
reporter: ['R'],
'reporter-option': ['reporter-options', 'O'],
Expand Down
4 changes: 4 additions & 0 deletions lib/cli/run.js
Expand Up @@ -176,6 +176,10 @@ exports.builder = yargs =>
group: GROUPS.OUTPUT,
hidden: true
},
'node-option': {
description: 'Node or V8 option (no leading "--")',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about no leading "--" or "--v8-"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't added the --v8- part, see comment above.

group: GROUPS.CONFIG
},
package: {
description: 'Path to package.json for config',
group: GROUPS.CONFIG,
Expand Down
16 changes: 16 additions & 0 deletions test/integration/options/node-flags.spec.js
Expand Up @@ -17,3 +17,19 @@ describe('node flags', function() {
);
});
});

describe('node flags using "--node-option"', function() {
it('should pass fake option to node and fail with node exception', function(done) {
invokeMocha(
['--node-option', 'fake-flag'],
function(err, res) {
if (err) {
return done(err);
}
expect(res, 'to have failed with output', /bad option: --fake-flag/i);
done();
},
'pipe'
);
});
});
17 changes: 16 additions & 1 deletion test/integration/options/timeout.spec.js
Expand Up @@ -50,7 +50,7 @@ describe('--timeout', function() {
});
});

it('should disable timeout with --inspect', function(done) {
it('should disable timeout with "--inspect"', function(done) {
var fixture = 'options/slow-test';
runMochaJSON(fixture, ['--inspect', '--timeout', '200'], function(
err,
Expand All @@ -64,4 +64,19 @@ describe('--timeout', function() {
done();
});
});

it('should disable timeout with "-n inspect"', function(done) {
var fixture = 'options/slow-test';
runMochaJSON(fixture, ['-n', 'inspect', '--timeout', '200'], function(
err,
res
) {
if (err) {
done(err);
return;
}
expect(res, 'to have passed').and('to have passed test count', 2);
done();
});
});
});
8 changes: 1 addition & 7 deletions test/node-unit/cli/node-flags.spec.js
Expand Up @@ -42,12 +42,7 @@ describe('node-flags', function() {
it('should return true for flags starting with "preserve-symlinks"', function() {
expect(isNodeFlag('preserve-symlinks'), 'to be true');
expect(isNodeFlag('preserve-symlinks-main'), 'to be true');
// Node >= v12 both flags exist in process.allowedNodeEnvironmentFlags
const nodeVersion = parseInt(process.version.match(/^v(\d+)\./)[1], 10);
expect(
isNodeFlag('preserve_symlinks'),
nodeVersion >= 12 ? 'to be true' : 'to be false'
);
expect(isNodeFlag('preserve_symlinks'), 'to be true');
});

it('should return true for flags starting with "harmony-" or "harmony_"', function() {
Expand All @@ -69,7 +64,6 @@ describe('node-flags', function() {
it('should return true for "gc-global"', function() {
expect(isNodeFlag('gc-global'), 'to be true');
expect(isNodeFlag('gc_global'), 'to be true');
expect(isNodeFlag('gc'), 'to be true');
});

it('should return true for "es-staging"', function() {
Expand Down