From 523dcace89b61dc4e824b86190d9e581f1e02579 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Fri, 5 Apr 2019 17:59:09 -0400 Subject: [PATCH] Fix #3822: Support cwd-relative --config first; fallback to cwd-relative require() --- lib/cli/config.js | 58 ++++++++++++++-- lib/utils.js | 23 +++++++ .../shared-config/mocha-config.js | 1 + .../node_modules/shared-config/package.json | 3 + .../fixtures/config/subdir/mocha-config.js | 9 +++ .../fixtures/config/subdir/mocha-config.json | 7 ++ .../fixtures/config/subdir/mocha-config.yaml | 7 ++ test/node-unit/cli/config.spec.js | 67 ++++++++++++++++--- 8 files changed, 159 insertions(+), 16 deletions(-) create mode 100644 test/integration/fixtures/config/node_modules/shared-config/mocha-config.js create mode 100644 test/integration/fixtures/config/node_modules/shared-config/package.json create mode 100644 test/integration/fixtures/config/subdir/mocha-config.js create mode 100644 test/integration/fixtures/config/subdir/mocha-config.json create mode 100644 test/integration/fixtures/config/subdir/mocha-config.yaml diff --git a/lib/cli/config.js b/lib/cli/config.js index 3691f0a079..36d262831d 100644 --- a/lib/cli/config.js +++ b/lib/cli/config.js @@ -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; @@ -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 => @@ -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; }; diff --git a/lib/utils.js b/lib/utils.js index 86ba8f0376..a5fa8c3ac2 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -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'); @@ -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); +}; diff --git a/test/integration/fixtures/config/node_modules/shared-config/mocha-config.js b/test/integration/fixtures/config/node_modules/shared-config/mocha-config.js new file mode 100644 index 0000000000..ebf83e1859 --- /dev/null +++ b/test/integration/fixtures/config/node_modules/shared-config/mocha-config.js @@ -0,0 +1 @@ +module.exports = {ok: true}; diff --git a/test/integration/fixtures/config/node_modules/shared-config/package.json b/test/integration/fixtures/config/node_modules/shared-config/package.json new file mode 100644 index 0000000000..7fa4b2c8e7 --- /dev/null +++ b/test/integration/fixtures/config/node_modules/shared-config/package.json @@ -0,0 +1,3 @@ +{ + "main": "mocha-config" +} diff --git a/test/integration/fixtures/config/subdir/mocha-config.js b/test/integration/fixtures/config/subdir/mocha-config.js new file mode 100644 index 0000000000..adec9d68ed --- /dev/null +++ b/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 +}; diff --git a/test/integration/fixtures/config/subdir/mocha-config.json b/test/integration/fixtures/config/subdir/mocha-config.json new file mode 100644 index 0000000000..53362c31ec --- /dev/null +++ b/test/integration/fixtures/config/subdir/mocha-config.json @@ -0,0 +1,7 @@ +{ + // a comment + "require": ["foo", "bar"], + "bail": true, + "reporter": "dot", + "slow": 60 +} diff --git a/test/integration/fixtures/config/subdir/mocha-config.yaml b/test/integration/fixtures/config/subdir/mocha-config.yaml new file mode 100644 index 0000000000..ac97fbd93a --- /dev/null +++ b/test/integration/fixtures/config/subdir/mocha-config.yaml @@ -0,0 +1,7 @@ +# a comment +require: + - foo + - bar +bail: true +reporter: dot +slow: 60 diff --git a/test/node-unit/cli/config.spec.js b/test/node-unit/cli/config.spec.js index eeb4bb2b82..ef3651ebb6 100644 --- a/test/node-unit/cli/config.spec.js +++ b/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'); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -88,7 +137,7 @@ describe('cli/config', function() { }); it('should assume JSON', function() { - loadConfig('foo.bar'); + parseConfig('foo.bar'); expect(parsers.json, 'was called'); }); }); @@ -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/); }); }); });