From 544a84176fd81e1ba6c8b51d4876770d9df4e330 Mon Sep 17 00:00:00 2001
From: Christopher Hiller <boneskull@boneskull.com>
Date: Wed, 13 Mar 2019 17:07:09 -0700
Subject: [PATCH 1/2] fix extension handling; closes #3808
---
lib/cli/options.js | 29 +++++++++-
lib/utils.js | 12 +++-
test/node-unit/cli/options.spec.js | 90 +++++++++++++++++++++++++++---
3 files changed, 119 insertions(+), 12 deletions(-)
diff --git a/lib/cli/options.js b/lib/cli/options.js
index a033c39ac7..8ca16d88c2 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -10,7 +10,8 @@ const fs = require('fs');
const yargsParser = require('yargs-parser');
const {types, aliases} = require('./run-option-metadata');
const {ONE_AND_DONE_ARGS} = require('./one-and-dones');
-const mocharc = require('../mocharc.json');
+// paranoia
+const mocharc = Object.freeze(require('../mocharc.json'));
const {list} = require('./run-helpers');
const {loadConfig, findConfig} = require('./config');
const findUp = require('find-up');
@@ -321,6 +322,30 @@ const loadOptions = (argv = []) => {
args.opts = false;
args._ = args._.concat(optsConfig._ || []);
}
+ // special case: "extension" option should not combine with default value.
+ // normally we want to combine "array"-type options, and we _do_ with "extension", but only
+ // within user-defined configuration (args or anything else).
+ // we must also search through any aliases of "extension" because while the arguments are ///
+ // normalized by this point, the config file values are not.
+ // only the "canonical" option name is used in `mocharc`, so we needn't worry about clearing
+ // multiple options.
+ // NOTE: as of this writing, "extension" is the only default value which is of an "array" type;
+ // it's unknown whether the the below strategy should be generalized to any other future
+ // "array"-type default option.
+ const processedMocharc = Object.assign({}, mocharc);
+ if (
+ args.extension ||
+ ['extension']
+ .concat(aliases.extension)
+ .some(
+ opt =>
+ Object.hasOwnProperty(rcConfig, opt) ||
+ Object.hasOwnProperty(pkgConfig, opt) ||
+ Object.hasOwnProperty(optsConfig, opt)
+ )
+ ) {
+ delete processedMocharc.extension;
+ }
args = parse(
args._,
@@ -328,7 +353,7 @@ const loadOptions = (argv = []) => {
rcConfig || {},
pkgConfig || {},
optsConfig || {},
- mocharc
+ processedMocharc
);
// recombine positional arguments and "spec"
diff --git a/lib/utils.js b/lib/utils.js
index 86ba8f0376..14c5ce126d 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -575,9 +575,15 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
var stat;
if (!fs.existsSync(filepath)) {
- if (fs.existsSync(filepath + '.js')) {
- filepath += '.js';
- } else {
+ // check all extensions
+ if (
+ !extensions.some(function(ext) {
+ if (fs.existsSync(filepath + '.' + ext)) {
+ filepath += '.' + ext;
+ return true;
+ }
+ })
+ ) {
// Handle glob
files = glob.sync(filepath);
if (!files.length) {
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index 3438a5564f..62a63f9fd6 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -27,7 +27,9 @@ const defaults = {
timeout: 1000,
timeouts: 1000,
t: 1000,
- opts: '/default/path/to/mocha.opts'
+ opts: '/default/path/to/mocha.opts',
+ extension: ['js'],
+ 'watch-extensions': ['js']
};
describe('options', function() {
@@ -59,6 +61,7 @@ describe('options', function() {
describe('loadOptions()', function() {
describe('when no parameter provided', function() {
beforeEach(function() {
+ this.timeout(500);
readFileSync = sandbox.stub();
readFileSync.onFirstCall().returns('{}');
readFileSync.onSecondCall().returns('--retries 3');
@@ -497,8 +500,8 @@ describe('options', function() {
beforeEach(function() {
readFileSync = sandbox.stub();
config = '/some/.mocharc.json';
- readFileSync.onFirstCall().returns('--retries 3');
- readFileSync.onSecondCall().returns('{}');
+ readFileSync.onFirstCall().returns('{}');
+ readFileSync.onSecondCall().returns('--retries 3');
findConfig = sandbox.stub();
loadConfig = sandbox.stub().throws('Error', 'failed to parse');
findupSync = sandbox.stub().returns('/some/package.json');
@@ -542,8 +545,8 @@ describe('options', function() {
beforeEach(function() {
readFileSync = sandbox.stub();
- readFileSync.onFirstCall().returns('--retries 3');
- readFileSync.onSecondCall().returns('{}');
+ readFileSync.onFirstCall().returns('{}');
+ readFileSync.onSecondCall().throws();
findConfig = sandbox.stub().returns('/some/.mocharc.json');
loadConfig = sandbox.stub().returns({});
findupSync = sandbox.stub().returns('/some/package.json');
@@ -578,8 +581,8 @@ describe('options', function() {
beforeEach(function() {
readFileSync = sandbox.stub();
- readFileSync.onFirstCall().returns('--retries 3');
- readFileSync.onSecondCall().returns('{}');
+ readFileSync.onFirstCall().returns('{}');
+ readFileSync.onSecondCall().throws();
findConfig = sandbox.stub().returns(null);
loadConfig = sandbox.stub().returns({});
findupSync = sandbox.stub().returns('/some/package.json');
@@ -716,5 +719,78 @@ describe('options', function() {
});
});
});
+
+ describe('"extension" handling', function() {
+ describe('when user supplies "extension" option', function() {
+ let result;
+
+ beforeEach(function() {
+ readFileSync = sandbox.stub();
+ readFileSync.onFirstCall().throws();
+ findConfig = sandbox.stub().returns('/some/.mocharc.json');
+ loadConfig = sandbox.stub().returns({extension: ['tsx']});
+ findupSync = sandbox.stub();
+ loadOptions = proxyLoadOptions({
+ readFileSync,
+ findConfig,
+ loadConfig,
+ findupSync
+ });
+ result = loadOptions(['--extension', 'ts']);
+ });
+
+ it('should not concatenate the default value', function() {
+ expect(result, 'to have property', 'extension', ['ts', 'tsx']);
+ });
+ });
+
+ describe('when user does not supply "extension" option', function() {
+ let result;
+
+ beforeEach(function() {
+ readFileSync = sandbox.stub();
+ readFileSync.onFirstCall().throws();
+ findConfig = sandbox.stub().returns('/some/.mocharc.json');
+ loadConfig = sandbox.stub().returns({});
+ findupSync = sandbox.stub();
+ loadOptions = proxyLoadOptions({
+ readFileSync,
+ findConfig,
+ loadConfig,
+ findupSync
+ });
+ result = loadOptions();
+ });
+
+ it('should retain the default', function() {
+ expect(result, 'to have property', 'extension', ['js']);
+ });
+ });
+ });
+
+ describe('"spec" handling', function() {
+ describe('when user supplies "spec" in config and positional arguments', function() {
+ let result;
+
+ beforeEach(function() {
+ readFileSync = sandbox.stub();
+ readFileSync.onFirstCall().throws();
+ findConfig = sandbox.stub().returns('/some/.mocharc.json');
+ loadConfig = sandbox.stub().returns({spec: '*.spec.js'});
+ findupSync = sandbox.stub();
+ loadOptions = proxyLoadOptions({
+ readFileSync,
+ findConfig,
+ loadConfig,
+ findupSync
+ });
+ result = loadOptions(['*.test.js']);
+ });
+
+ it('should place both into the positional arguments array', function() {
+ expect(result, 'to have property', '_', ['*.test.js', '*.spec.js']);
+ });
+ });
+ });
});
});
From 2d1dc343c5b73ebcc81a1a4e8a2b7980c219657d Mon Sep 17 00:00:00 2001
From: juergba <filodron@gmail.com>
Date: Thu, 25 Apr 2019 15:50:26 +0200
Subject: [PATCH 2/2] use yargs-parser options.default
utils.js: fix lookupFiles()
---
lib/cli/options.js | 35 +++++------------------------
lib/cli/run-helpers.js | 4 ++--
lib/utils.js | 20 ++++++++---------
test/integration/file-utils.spec.js | 13 +++++++----
4 files changed, 26 insertions(+), 46 deletions(-)
diff --git a/lib/cli/options.js b/lib/cli/options.js
index 8ca16d88c2..340fb01e86 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -10,8 +10,7 @@ const fs = require('fs');
const yargsParser = require('yargs-parser');
const {types, aliases} = require('./run-option-metadata');
const {ONE_AND_DONE_ARGS} = require('./one-and-dones');
-// paranoia
-const mocharc = Object.freeze(require('../mocharc.json'));
+const mocharc = require('../mocharc.json');
const {list} = require('./run-helpers');
const {loadConfig, findConfig} = require('./config');
const findUp = require('find-up');
@@ -81,11 +80,12 @@ const nargOpts = types.array
/**
* Wrapper around `yargs-parser` which applies our settings
* @param {string|string[]} args - Arguments to parse
+ * @param {Object} defaultValues - Default values of mocharc.json
* @param {...Object} configObjects - `configObjects` for yargs-parser
* @private
* @ignore
*/
-const parse = (args = [], ...configObjects) => {
+const parse = (args = [], defaultValues = {}, ...configObjects) => {
// 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
@@ -110,6 +110,7 @@ const parse = (args = [], ...configObjects) => {
const result = yargsParser.detailed(args, {
configuration,
configObjects,
+ default: defaultValues,
coerce: coerceOpts,
narg: nargOpts,
alias: aliases,
@@ -322,38 +323,14 @@ const loadOptions = (argv = []) => {
args.opts = false;
args._ = args._.concat(optsConfig._ || []);
}
- // special case: "extension" option should not combine with default value.
- // normally we want to combine "array"-type options, and we _do_ with "extension", but only
- // within user-defined configuration (args or anything else).
- // we must also search through any aliases of "extension" because while the arguments are ///
- // normalized by this point, the config file values are not.
- // only the "canonical" option name is used in `mocharc`, so we needn't worry about clearing
- // multiple options.
- // NOTE: as of this writing, "extension" is the only default value which is of an "array" type;
- // it's unknown whether the the below strategy should be generalized to any other future
- // "array"-type default option.
- const processedMocharc = Object.assign({}, mocharc);
- if (
- args.extension ||
- ['extension']
- .concat(aliases.extension)
- .some(
- opt =>
- Object.hasOwnProperty(rcConfig, opt) ||
- Object.hasOwnProperty(pkgConfig, opt) ||
- Object.hasOwnProperty(optsConfig, opt)
- )
- ) {
- delete processedMocharc.extension;
- }
args = parse(
args._,
+ mocharc,
args,
rcConfig || {},
pkgConfig || {},
- optsConfig || {},
- processedMocharc
+ optsConfig || {}
);
// recombine positional arguments and "spec"
diff --git a/lib/cli/run-helpers.js b/lib/cli/run-helpers.js
index 6f3476d496..0858d61b03 100644
--- a/lib/cli/run-helpers.js
+++ b/lib/cli/run-helpers.js
@@ -219,7 +219,7 @@ exports.singleRun = (mocha, {files = [], exit = false} = {}) => {
*/
exports.watchRun = (
mocha,
- {extension = ['js'], grep = '', ui = 'bdd', files = []} = {}
+ {extension = [], grep = '', ui = 'bdd', files = []} = {}
) => {
let runner;
@@ -291,7 +291,7 @@ exports.watchRun = (
*/
exports.runMocha = (
mocha,
- {watch = false, extension = ['js'], grep = '', ui = 'bdd', exit = false} = {},
+ {watch = false, extension = [], grep = '', ui = 'bdd', exit = false} = {},
files = []
) => {
if (watch) {
diff --git a/lib/utils.js b/lib/utils.js
index 14c5ce126d..93005cedf2 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -562,7 +562,6 @@ function isHiddenOnUnix(pathname) {
*
* @public
* @memberof Mocha.utils
- * @todo Fix extension handling
* @param {string} filepath - Base path to start searching from.
* @param {string[]} extensions - File extensions to look for.
* @param {boolean} recursive - Whether to recurse into subdirectories.
@@ -571,19 +570,18 @@ function isHiddenOnUnix(pathname) {
* @throws {TypeError} if `filepath` is directory and `extensions` not provided.
*/
exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
+ extensions = extensions || [];
var files = [];
var stat;
if (!fs.existsSync(filepath)) {
// check all extensions
- if (
- !extensions.some(function(ext) {
- if (fs.existsSync(filepath + '.' + ext)) {
- filepath += '.' + ext;
- return true;
- }
- })
- ) {
+ extensions.forEach(function(ext) {
+ if (fs.existsSync(filepath + '.' + ext)) {
+ files.push(filepath + '.' + ext);
+ }
+ });
+ if (!files.length) {
// Handle glob
files = glob.sync(filepath);
if (!files.length) {
@@ -592,8 +590,8 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
filepath
);
}
- return files;
}
+ return files;
}
// Handle file
@@ -624,7 +622,7 @@ exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
// ignore error
return;
}
- if (!extensions) {
+ if (!extensions.length) {
throw createMissingArgumentError(
util.format(
'Argument %s required when argument %s is a directory',
diff --git a/test/integration/file-utils.spec.js b/test/integration/file-utils.spec.js
index 3fe030ee46..f09018871b 100644
--- a/test/integration/file-utils.spec.js
+++ b/test/integration/file-utils.spec.js
@@ -58,7 +58,7 @@ describe('file utils', function() {
ex.and('to have length', expectedLength);
});
- it('should parse extensions from extnsions parameter', function() {
+ it('should parse extensions from extensions parameter', function() {
var nonJsFile = tmpFile('mocha-utils-text.txt');
fs.writeFileSync(nonJsFile, 'yippy skippy ying yang yow');
@@ -66,9 +66,14 @@ describe('file utils', function() {
expect(res, 'to contain', nonJsFile).and('to have length', 1);
});
- it('should not require the extensions parameter when looking up a file', function() {
- var res = utils.lookupFiles(tmpFile('mocha-utils'), undefined, false);
- expect(res, 'to be', tmpFile('mocha-utils.js'));
+ it('should require the extensions parameter when looking up a file', function() {
+ var dirLookup = function() {
+ return utils.lookupFiles(tmpFile('mocha-utils'), undefined, false);
+ };
+ expect(dirLookup, 'to throw', {
+ name: 'Error',
+ code: 'ERR_MOCHA_NO_FILES_MATCH_PATTERN'
+ });
});
it('should require the extensions parameter when looking up a directory', function() {