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

fix extension and spec handling; closes #3808 #3834

Merged
merged 2 commits into from
May 5, 2019
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix extension handling; closes #3808
boneskull authored and juergba committed May 5, 2019

Verified

This commit was signed with the committer’s verified signature.
commit 544a84176fd81e1ba6c8b51d4876770d9df4e330
29 changes: 27 additions & 2 deletions lib/cli/options.js
Original file line number Diff line number Diff line change
@@ -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'));
Copy link
Contributor

Choose a reason for hiding this comment

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

const rcDefaults = Object.freeze(require('../mocharc.json'));

const {list} = require('./run-helpers');
const {loadConfig, findConfig} = require('./config');
const findUp = require('find-up');
@@ -321,14 +322,38 @@ 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._,
args,
rcConfig || {},
pkgConfig || {},
optsConfig || {},
mocharc
processedMocharc
);

// recombine positional arguments and "spec"
12 changes: 9 additions & 3 deletions lib/utils.js
Original file line number Diff line number Diff line change
@@ -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;
}
})
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The only reason this was here was to allow the historical usage of "test.js" via the default spec "test". IMO, we should not propagate this further.

The "Handle directory" code below already handles extensions for "test/*".

// Handle glob
files = glob.sync(filepath);
if (!files.length) {
90 changes: 83 additions & 7 deletions test/node-unit/cli/options.spec.js
Original file line number Diff line number Diff line change
@@ -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']);
});
});
});
});
});