Skip to content

Commit

Permalink
prefer all mocha flags over node flags; closes #4417
Browse files Browse the repository at this point in the history
- no more `require` special-case
- future-proofs `mocha` against the introduction of conflicting flags in `node`
  • Loading branch information
boneskull committed Aug 25, 2020
1 parent 4d7a171 commit 213e0c0
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 30 deletions.
13 changes: 6 additions & 7 deletions lib/cli/node-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

const nodeFlags = process.allowedNodeEnvironmentFlags;
const {isMochaFlag} = require('./run-option-metadata');
const unparse = require('yargs-unparser');

/**
Expand Down Expand Up @@ -43,16 +44,14 @@ exports.isNodeFlag = (flag, bareword = true) => {
flag = flag.replace(/^--?/, '');
}
return (
// treat --require/-r as Mocha flag even though it's also a node flag
!(flag === 'require' || flag === 'r') &&
// 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 && nodeFlags.has(flag)) ||
debugFlags.has(flag) ||
/(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
flag
))
(!isMochaFlag(flag) && nodeFlags && nodeFlags.has(flag)) ||
debugFlags.has(flag) ||
/(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
flag
)
);
};

Expand Down
27 changes: 25 additions & 2 deletions lib/cli/run-option-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* @type {{string:string[]}}
* @private
*/
exports.types = {
const TYPES = (exports.types = {
array: [
'extension',
'file',
Expand Down Expand Up @@ -58,7 +58,7 @@ exports.types = {
'slow',
'timeout'
]
};
});

/**
* Option aliases keyed by canonical option name.
Expand Down Expand Up @@ -88,3 +88,26 @@ exports.aliases = {
ui: ['u'],
watch: ['w']
};

const ALL_MOCHA_FLAGS = Object.keys(TYPES).reduce((acc, key) => {
// gets all flags from each of the fields in `types`, adds those,
// then adds aliases of each flag (if any)
TYPES[key].forEach(flag => {
acc.add(flag);
const aliases = exports.aliases[flag] || [];
aliases.forEach(alias => {
acc.add(alias);
});
});
return acc;
}, new Set());

/**
* Returns `true` if the provided `flag` is known to Mocha.
* @param {string} flag - Flag to check
* @returns {boolean} If `true`, this is a Mocha flag
* @private
*/
exports.isMochaFlag = flag => {
return ALL_MOCHA_FLAGS.has(flag.replace(/^--?/, ''));
};
40 changes: 19 additions & 21 deletions test/node-unit/cli/node-flags.spec.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,41 @@
'use strict';

const nodeEnvFlags = process.allowedNodeEnvironmentFlags;
const nodeEnvFlags = [...process.allowedNodeEnvironmentFlags];
const {
isNodeFlag,
impliesNoTimeouts,
unparseNodeFlags
} = require('../../../lib/cli/node-flags');

const {isMochaFlag} = require('../../../lib/cli/run-option-metadata');

describe('node-flags', function() {
describe('isNodeFlag()', function() {
describe('for all allowed node environment flags', function() {
// NOTE: this is not stubbing nodeEnvFlags in any way, so relies on
// the userland polyfill to be correct.
nodeEnvFlags.forEach(envFlag => {
it(`${envFlag} should return true`, function() {
expect(isNodeFlag(envFlag), 'to be true');
nodeEnvFlags
.filter(flag => !isMochaFlag(flag))
.forEach(envFlag => {
it(`${envFlag} should return true`, function() {
expect(isNodeFlag(envFlag), 'to be true');
});
});
});

describe('for all allowed node env flags which conflict with mocha flags', function() {
nodeEnvFlags
.filter(flag => isMochaFlag(flag))
.forEach(envFlag => {
it(`${envFlag} should return false`, function() {
expect(isNodeFlag(envFlag), 'to be false');
});
});
});
});

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() {
Expand Down Expand Up @@ -132,14 +139,5 @@ describe('node-flags', function() {
['--v8-numeric-one=1', '--v8-boolean-one', '--v8-numeric-two=2']
);
});

it('should special-case "--require"', function() {
// note the only way for this to happen IN REAL LIFE is if you use "--require esm";
// mocha eats all --require args otherwise.
expect(unparseNodeFlags({require: 'mcrib'}), 'to equal', [
'--require',
'mcrib'
]);
});
});
});

0 comments on commit 213e0c0

Please sign in to comment.