Skip to content

Commit

Permalink
Fix #3822: Support cwd-relative --config first; fallback to cwd-relat…
Browse files Browse the repository at this point in the history
…ive require()
  • Loading branch information
cspotcode committed Apr 5, 2019
1 parent e87c689 commit 523dcac
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 16 deletions.
58 changes: 51 additions & 7 deletions lib/cli/config.js
Expand Up @@ -12,6 +12,7 @@ const fs = require('fs');
const findUp = require('find-up');
const path = require('path');
const debug = require('debug')('mocha:cli:config');
const utils = require('../utils');

/**
* These are the valid config files, in order of precedence;
Expand All @@ -31,6 +32,8 @@ exports.CONFIG_FILES = [
/**
* Parsers for various config filetypes. Each accepts a filepath and
* returns an object (but could throw)
*
* Filepath *must* be absolute; external logic resolves relative paths.
*/
const parsers = (exports.parsers = {
yaml: filepath =>
Expand All @@ -45,23 +48,64 @@ const parsers = (exports.parsers = {
/**
* Loads and parses, based on file extension, a config file.
* "JSON" files may have comments.
* @param {string} filepath - Config file path to load
* @param {string} filepath - Config file path or module name to load
* @returns {Object} Parsed config object
* @private
*/
exports.loadConfig = filepath => {
const {absFilepath, discoveryMethod} = exports.resolveConfigPath(filepath);
return exports.parseConfig(absFilepath, discoveryMethod);
};

/**
* Resolve the location of a config on disk and the method of discovery:
* cwd-relative path or require.resolve
*
* As opposed to findConfig, this does not ascend the filesystem.
* When the user specifies `--config`, findConfig is not called, but
* `resolveConfigPath` is still called.
*
* @param {string} filepath - Config file path or module name to load
* @private
*/
exports.resolveConfigPath = filepath => {
/** @type {'cwd-relative' | 'node-require-resolve'} */
let discoveryMethod = 'cwd-relative';
let absFilepath = path.resolve(filepath);
if (!fs.existsSync(absFilepath) || fs.statSync(absFilepath).isDirectory()) {
discoveryMethod = 'node-require-resolve';
try {
absFilepath = utils.requireResolveRelative(filepath);
} catch (e) {
throw new Error(
`failed to locate ${filepath} as either a relative path or a require()-able node module`
);
}
}
return {discoveryMethod, absFilepath};
};

/**
* @param {string} absFilepath absolute path to config file
* @param {'cwd-relative' | 'node-require-resolve'} discoveryMethod
*/
exports.parseConfig = (absFilepath, discoveryMethod) => {
let config = {};
const ext = path.extname(filepath);
const ext = path.extname(absFilepath);
try {
if (/\.ya?ml/.test(ext)) {
config = parsers.yaml(filepath);
if (discoveryMethod === 'node-require-resolve') {
config = require(absFilepath);
} else if (/\.ya?ml/.test(ext)) {
config = parsers.yaml(absFilepath);
} else if (ext === '.js') {
config = parsers.js(filepath);
config = parsers.js(absFilepath);
} else {
config = parsers.json(filepath);
config = parsers.json(absFilepath);
}
} catch (err) {
throw new Error(`failed to parse ${filepath}: ${err}`);
throw new Error(
`failed to parse ${path.relative(process.cwd(), absFilepath)}: ${err}`
);
}
return config;
};
Expand Down
23 changes: 23 additions & 0 deletions lib/utils.js
Expand Up @@ -12,6 +12,7 @@
var fs = require('fs');
var path = require('path');
var util = require('util');
var Module = require('module');
var glob = require('glob');
var he = require('he');
var errors = require('./errors');
Expand Down Expand Up @@ -895,3 +896,25 @@ exports.defineConstants = function(obj) {
}
return Object.freeze(exports.createMap(obj));
};

/**
* require.resolve() relative to the given path.
*
* Usually used to require() modules relative to user's project root or cwd.
* - for `--config nodeModule`
* - for loading custom reporters
*
* ALTERNATIVE:
* We can use `require('require-relative').resolve()` It's tiny.
* npm.im/require-relative
* https://unpkg.com/require-relative@0.8.7/index.js
*/
exports.requireResolveRelative = function(request, cwd) {
if (cwd === undefined) cwd = process.cwd();
// Create a fake module that resides in the cwd directory
var m = new Module(path.join(cwd, '__mocha_resolve_relative.js'), null);
m.filename = m.id;
m.paths = Module._nodeModulePaths(cwd);
// Ask node's module API to resolve the requested module relative to our fake module
return Module._resolveFilename(request, m);
};

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

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

9 changes: 9 additions & 0 deletions test/integration/fixtures/config/subdir/mocha-config.js
@@ -0,0 +1,9 @@
'use strict';

// a comment
module.exports = {
require: ['foo', 'bar'],
bail: true,
reporter: 'dot',
slow: 60
};
7 changes: 7 additions & 0 deletions test/integration/fixtures/config/subdir/mocha-config.json
@@ -0,0 +1,7 @@
{
// a comment
"require": ["foo", "bar"],
"bail": true,
"reporter": "dot",
"slow": 60
}
7 changes: 7 additions & 0 deletions test/integration/fixtures/config/subdir/mocha-config.yaml
@@ -0,0 +1,7 @@
# a comment
require:
- foo
- bar
bail: true
reporter: dot
slow: 60
67 changes: 58 additions & 9 deletions test/node-unit/cli/config.spec.js
@@ -1,6 +1,12 @@
'use strict';

const {loadConfig, parsers, CONFIG_FILES} = require('../../../lib/cli/config');
const path = require('path');
const {
resolveConfigPath,
parseConfig,
parsers,
CONFIG_FILES
} = require('../../../lib/cli/config');
const {createSandbox} = require('sinon');
const rewiremock = require('rewiremock/node');

Expand All @@ -16,7 +22,50 @@ describe('cli/config', function() {
sandbox.restore();
});

describe('loadConfig()', function() {
describe('resolveConfigPath', function() {
let prevCwd;
const cwd = path.resolve(__dirname, '../../integration/fixtures/config');
const yamlpath = 'subdir/mocha-config.yaml';
const absYamlpath = path.join(cwd, yamlpath);
const modulepath = 'shared-config';
const absModulepath = path.resolve(
cwd,
'node_modules/shared-config/mocha-config.js'
);

beforeEach(function() {
prevCwd = process.cwd();
process.chdir(cwd);
});

afterEach(function() {
process.chdir(prevCwd);
});

describe('when supplied a cwd-relative path', function() {
it('should return absolute path to file', function() {
const {absFilepath, discoveryMethod} = resolveConfigPath(yamlpath);
expect(absFilepath, 'to be', absYamlpath);
expect(discoveryMethod, 'to be', 'cwd-relative');
});
});
describe('when supplied an absolute path', function() {
it('should return absolute path to file', function() {
const {absFilepath, discoveryMethod} = resolveConfigPath(absYamlpath);
expect(absFilepath, 'to be', absYamlpath);
expect(discoveryMethod, 'to be', 'cwd-relative');
});
});
describe('when supplied a require-able path', function() {
it('should return absolute path to file with require-resolve discovery', function() {
const {absFilepath, discoveryMethod} = resolveConfigPath(modulepath);
expect(absFilepath, 'to be', absModulepath);
expect(discoveryMethod, 'to be', 'node-require-resolve');
});
});
});

describe('parseConfig()', function() {
describe('when parsing succeeds', function() {
beforeEach(function() {
sandbox.stub(parsers, 'yaml').returns(config);
Expand All @@ -28,7 +77,7 @@ describe('cli/config', function() {
const filepath = 'foo.yaml';

it('should use the YAML parser', function() {
loadConfig(filepath);
parseConfig(filepath);
expect(parsers.yaml, 'to have calls satisfying', [
{args: [filepath], returned: config}
]).and('was called times', 1);
Expand All @@ -39,7 +88,7 @@ describe('cli/config', function() {
const filepath = 'foo.yml';

it('should use the YAML parser', function() {
loadConfig(filepath);
parseConfig(filepath);
expect(parsers.yaml, 'to have calls satisfying', [
{args: [filepath], returned: config}
]).and('was called times', 1);
Expand All @@ -50,7 +99,7 @@ describe('cli/config', function() {
const filepath = 'foo.js';

it('should use the JS parser', function() {
loadConfig(filepath);
parseConfig(filepath);
expect(parsers.js, 'to have calls satisfying', [
{args: [filepath], returned: config}
]).and('was called times', 1);
Expand All @@ -61,7 +110,7 @@ describe('cli/config', function() {
const filepath = 'foo.jsonc';

it('should use the JSON parser', function() {
loadConfig('foo.jsonc');
parseConfig('foo.jsonc');
expect(parsers.json, 'to have calls satisfying', [
{args: [filepath], returned: config}
]).and('was called times', 1);
Expand All @@ -72,7 +121,7 @@ describe('cli/config', function() {
const filepath = 'foo.json';

it('should use the JSON parser', function() {
loadConfig('foo.json');
parseConfig('foo.json');
expect(parsers.json, 'to have calls satisfying', [
{args: [filepath], returned: config}
]).and('was called times', 1);
Expand All @@ -88,7 +137,7 @@ describe('cli/config', function() {
});

it('should assume JSON', function() {
loadConfig('foo.bar');
parseConfig('foo.bar');
expect(parsers.json, 'was called');
});
});
Expand All @@ -99,7 +148,7 @@ describe('cli/config', function() {
});

it('should throw', function() {
expect(() => loadConfig('goo.yaml'), 'to throw', /failed to parse/);
expect(() => parseConfig('goo.yaml'), 'to throw', /failed to parse/);
});
});
});
Expand Down

0 comments on commit 523dcac

Please sign in to comment.