From 7d7154e7e33dcf8c097035c9438244d3715bd902 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 12 Mar 2019 07:33:20 -0500 Subject: [PATCH 01/17] fix(lib/cli/config.js): Add cwd-relative fallback to load config file Original code only used module-relative path loading, which made `--config` argument non-obvious. Added fallback processing. Added debug logging. Minor cleanup. #3822 --- lib/cli/config.js | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/cli/config.js b/lib/cli/config.js index dd8f987ca1..c93554bae5 100644 --- a/lib/cli/config.js +++ b/lib/cli/config.js @@ -9,9 +9,9 @@ */ const fs = require('fs'); -const findUp = require('findup-sync'); const path = require('path'); const debug = require('debug')('mocha:cli:config'); +const findUp = require('findup-sync'); /** * These are the valid config files, in order of precedence; @@ -29,13 +29,29 @@ exports.CONFIG_FILES = [ ]; /** - * Parsers for various config filetypes. Each accepts a filepath and + * Parsers for various config filetypes. Each accepts a filepath and * returns an object (but could throw) */ const parsers = (exports.parsers = { yaml: filepath => require('js-yaml').safeLoad(fs.readFileSync(filepath, 'utf8')), - js: filepath => require(filepath), + js: filepath => { + try { + debug(`parsers: load using module-relative path: "${filepath}"`); + return require(filepath); + } catch (err) { + if ( + err.code !== 'MODULE_NOT_FOUND' || + err.message.indexOf('Cannot find module') !== -1 + ) { + filepath = path.resolve(filepath); + debug(`parsers: retry using cwd-relative path: "${filepath}"`); + return require(filepath); + } else { + throw err; // rethrow + } + } + }, json: filepath => JSON.parse( require('strip-json-comments')(fs.readFileSync(filepath, 'utf8')) @@ -45,15 +61,18 @@ const parsers = (exports.parsers = { /** * Loads and parses, based on file extension, a config file. * "JSON" files may have comments. + * + * @private * @param {string} filepath - Config file path to load * @returns {Object} Parsed config object - * @private */ exports.loadConfig = filepath => { let config = {}; const ext = path.extname(filepath); + + debug(`loadConfig: "${filepath}"`); try { - if (/\.ya?ml/.test(ext)) { + if (ext === '.yml' || ext === '.yaml') { config = parsers.yaml(filepath); } else if (ext === '.js') { config = parsers.js(filepath); @@ -61,20 +80,21 @@ exports.loadConfig = filepath => { config = parsers.json(filepath); } } catch (err) { - throw new Error(`failed to parse ${filepath}: ${err}`); + throw new Error(`failed to parse config file "${filepath}": ${err}`); } return config; }; /** * Find ("find up") config file starting at `cwd` + * * @param {string} [cwd] - Current working directory * @returns {string|null} Filepath to config, if found */ exports.findConfig = (cwd = process.cwd()) => { const filepath = findUp(exports.CONFIG_FILES, {cwd}); if (filepath) { - debug(`found config at ${filepath}`); + debug(`findConfig: found "${filepath}"`); } return filepath; }; From 49c55cc9095def0d7536c15b13494129bc2f7c38 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 12 Mar 2019 15:43:02 -0500 Subject: [PATCH 02/17] test(integration/config.spec.js): Add test for loading from cwd-based relative path --- test/integration/config.spec.js | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/test/integration/config.spec.js b/test/integration/config.spec.js index 76a4a718d4..c4f301b35c 100644 --- a/test/integration/config.spec.js +++ b/test/integration/config.spec.js @@ -3,8 +3,9 @@ // this is not a "functional" test; we aren't invoking the mocha executable. // instead we just avoid test doubles. -var loadConfig = require('../../lib/cli/config').loadConfig; +var fs = require('fs'); var path = require('path'); +var loadConfig = require('../../lib/cli/config').loadConfig; describe('config', function() { it('should return the same values for all supported config types', function() { @@ -15,4 +16,32 @@ describe('config', function() { expect(js, 'to equal', json); expect(json, 'to equal', yaml); }); + + /** + * @returns {String} pathname to project root directory + */ + function getProjectRootDir() { + var searchPaths = module.parent.paths; + for (var i = 0, len = searchPaths.length; i < len; i++) { + var searchPath = searchPaths[i]; + if (fs.existsSync(searchPath)) { + return path.dirname(searchPath); + } + } + } + + it('should load a ".js" config file given relative path', function() { + var projRootDir = getProjectRootDir(); + var configDir = path.join(__dirname, 'fixtures', 'config'); + var relConfigDir = configDir.substring(projRootDir.length + 1); + var js; + + function _loadConfig() { + js = loadConfig(path.join(relConfigDir, 'mocharc.js')); + } + + expect(_loadConfig, 'not to throw'); + var json = loadConfig(path.join(configDir, 'mocharc.json')); + expect(js, 'to equal', json); + }); }); From cf62435d49717e7e0f22ff9980c4695f186bc524 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Sat, 16 Mar 2019 06:50:28 -0500 Subject: [PATCH 03/17] feat(cli/config.js): Change config file load scheme to be cwd-relative Changed parser load order to attempt local file load first. Additionally, made change that allows for Mocha config to be loaded from a package. BREAKING CHANGE: Anyone depending on previous module-based loading style will need to change to cwd-relative scheme. --- lib/cli/config.js | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/cli/config.js b/lib/cli/config.js index 69acf265b3..c100b28feb 100644 --- a/lib/cli/config.js +++ b/lib/cli/config.js @@ -28,6 +28,10 @@ exports.CONFIG_FILES = [ '.mocharc.json' ]; +const isModuleNotFoundError = err => + err.code !== 'MODULE_NOT_FOUND' || + err.message.indexOf('Cannot find module') !== -1; + /** * Parsers for various config filetypes. Each accepts a filepath and * returns an object (but could throw) @@ -36,16 +40,16 @@ const parsers = (exports.parsers = { yaml: filepath => require('js-yaml').safeLoad(fs.readFileSync(filepath, 'utf8')), js: filepath => { + const cwdFilepath = path.resolve(filepath); try { - debug(`parsers: load using module-relative path: "${filepath}"`); - return require(filepath); + debug(`parsers: load using cwd-relative path: "${cwdFilepath}"`); + return require(cwdFilepath); } catch (err) { if ( - err.code !== 'MODULE_NOT_FOUND' || - err.message.indexOf('Cannot find module') !== -1 + isModuleNotFoundError(err) && + require('validate-npm-package-name')(filepath).validForNewPackages ) { - filepath = path.resolve(filepath); - debug(`parsers: retry using cwd-relative path: "${filepath}"`); + debug(`parsers: retry load as package: "${filepath}"`); return require(filepath); } else { throw err; // rethrow @@ -74,13 +78,13 @@ exports.loadConfig = filepath => { try { if (ext === '.yml' || ext === '.yaml') { config = parsers.yaml(filepath); - } else if (ext === '.js') { - config = parsers.js(filepath); - } else { + } else if (ext === '.json') { config = parsers.json(filepath); + } else { + config = parsers.js(filepath); } } catch (err) { - throw new Error(`failed to parse config file "${filepath}": ${err}`); + throw new Error(`failed to parse config "${filepath}": ${err}`); } return config; }; From e578813a74c23e430c12c3d7e36e9edabaffc020 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Sat, 16 Mar 2019 07:30:44 -0500 Subject: [PATCH 04/17] build(package.json): Add "validate-npm-package-name" package --- package-lock.json | 13 +++++++++++++ package.json | 1 + 2 files changed, 14 insertions(+) diff --git a/package-lock.json b/package-lock.json index cce95ce7d1..7e714cc8ab 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2478,6 +2478,11 @@ "integrity": "sha1-hZgoeOIbmOHGZCXgPQF0eI9Wnug=", "dev": true }, + "builtins": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/builtins/-/builtins-1.0.3.tgz", + "integrity": "sha1-y5T662HIaWRR2zZTThQi+U8K7og=" + }, "bytes": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.0.0.tgz", @@ -20925,6 +20930,14 @@ "spdx-expression-parse": "^3.0.0" } }, + "validate-npm-package-name": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/validate-npm-package-name/-/validate-npm-package-name-3.0.0.tgz", + "integrity": "sha1-X6kS2B630MdK/BQN5zF/DKffQ34=", + "requires": { + "builtins": "^1.0.3" + } + }, "vendors": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/vendors/-/vendors-1.0.2.tgz", diff --git a/package.json b/package.json index 773bf7bcf8..0aedfb0c71 100644 --- a/package.json +++ b/package.json @@ -510,6 +510,7 @@ "object.assign": "4.1.0", "strip-json-comments": "2.0.1", "supports-color": "6.0.0", + "validate-npm-package-name": "^3.0.0", "which": "1.3.1", "wide-align": "1.1.3", "yargs": "13.2.2", From 5dcb8e3968c01cf7b9d350728e44701513fa6ff2 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Sat, 16 Mar 2019 07:42:01 -0500 Subject: [PATCH 05/17] test(cli/config.spec.js): Update unit test --- test/node-unit/cli/config.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/node-unit/cli/config.spec.js b/test/node-unit/cli/config.spec.js index eeb4bb2b82..3e2f6175ba 100644 --- a/test/node-unit/cli/config.spec.js +++ b/test/node-unit/cli/config.spec.js @@ -87,9 +87,9 @@ describe('cli/config', function() { sandbox.stub(parsers, 'js').returns(config); }); - it('should assume JSON', function() { + it('should assume package name and use JS parser', function() { loadConfig('foo.bar'); - expect(parsers.json, 'was called'); + expect(parsers.js, 'was called'); }); }); From e6a61d131f0cf07d5ef5c56bb5b867473cdfc198 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Sat, 16 Mar 2019 08:15:27 -0500 Subject: [PATCH 06/17] test(cli/config.spec.js): Changed test title --- test/node-unit/cli/config.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/node-unit/cli/config.spec.js b/test/node-unit/cli/config.spec.js index 3e2f6175ba..7de4007327 100644 --- a/test/node-unit/cli/config.spec.js +++ b/test/node-unit/cli/config.spec.js @@ -87,7 +87,7 @@ describe('cli/config', function() { sandbox.stub(parsers, 'js').returns(config); }); - it('should assume package name and use JS parser', function() { + it('should use the JS parser', function() { loadConfig('foo.bar'); expect(parsers.js, 'was called'); }); From 8bc0df6c87579b38dab29fc74ca9a51aac9ec0d3 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Sat, 16 Mar 2019 08:17:20 -0500 Subject: [PATCH 07/17] fix(cli/config.js): Forgot to commit fix for parsing JSONC --- lib/cli/config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cli/config.js b/lib/cli/config.js index c100b28feb..45f0f9cf1b 100644 --- a/lib/cli/config.js +++ b/lib/cli/config.js @@ -78,7 +78,7 @@ exports.loadConfig = filepath => { try { if (ext === '.yml' || ext === '.yaml') { config = parsers.yaml(filepath); - } else if (ext === '.json') { + } else if (ext === '.json' || ext === '.jsonc') { config = parsers.json(filepath); } else { config = parsers.js(filepath); From e34484653c34e5e5b2fb7959a4e98788c41024a1 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Sun, 17 Mar 2019 09:49:33 -0500 Subject: [PATCH 08/17] test(integration/config.spec.js): Add tests for configuring with a package As I used symlinks, some tests will likely fail on Windows... --- test/integration/config.spec.js | 76 ++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 11 deletions(-) diff --git a/test/integration/config.spec.js b/test/integration/config.spec.js index c4f301b35c..7f48430db2 100644 --- a/test/integration/config.spec.js +++ b/test/integration/config.spec.js @@ -18,7 +18,7 @@ describe('config', function() { }); /** - * @returns {String} pathname to project root directory + * @returns {String} pathname to Mocha project root directory */ function getProjectRootDir() { var searchPaths = module.parent.paths; @@ -30,18 +30,72 @@ describe('config', function() { } } - it('should load a ".js" config file given relative path', function() { - var projRootDir = getProjectRootDir(); + describe('when configuring Mocha via a ".js" file', function() { var configDir = path.join(__dirname, 'fixtures', 'config'); - var relConfigDir = configDir.substring(projRootDir.length + 1); - var js; + var json = loadConfig(path.join(configDir, 'mocharc.json')); - function _loadConfig() { - js = loadConfig(path.join(relConfigDir, 'mocharc.js')); - } + it('should load configuration given absolute path', function() { + var js; - expect(_loadConfig, 'not to throw'); - var json = loadConfig(path.join(configDir, 'mocharc.json')); - expect(js, 'to equal', json); + function _loadConfig() { + js = loadConfig(path.join(configDir, 'mocharc.js')); + } + + expect(_loadConfig, 'not to throw'); + expect(js, 'to equal', json); + }); + + it('should load configuration given relative path', function() { + var projRootDir = getProjectRootDir(); + var relConfigDir = configDir.substring(projRootDir.length + 1); + var js; + + function _loadConfig() { + js = loadConfig(path.join('.', relConfigDir, 'mocharc.js')); + } + + expect(_loadConfig, 'not to throw'); + expect(js, 'to equal', json); + }); + }); + + describe('when configuring Mocha via package', function() { + var projRootDir = getProjectRootDir(); + var configDir = path.join('fixtures', 'config'); + var pkgName = 'mocha-config'; + var pkgDir = path.join(__dirname, configDir, pkgName); + + before(function() { + fs.symlinkSync( + pkgDir, + path.join(projRootDir, 'node_modules', pkgName), + 'dir' + ); + }); + + it('should load configuration given valid package name', function() { + var configDir = path.join(__dirname, 'fixtures', 'config'); + var js; + + function _loadConfig() { + js = loadConfig(pkgName); + } + + expect(_loadConfig, 'not to throw'); + var json = loadConfig(path.join(configDir, 'mocharc.json')); + expect(js, 'to equal', json); + }); + + it('should throw given invalid package name', function() { + function _loadConfig() { + loadConfig(pkgName.toUpperCase()); + } + + expect(_loadConfig, 'to throw'); + }); + + after(function() { + fs.unlinkSync(path.join(projRootDir, 'node_modules', pkgName)); + }); }); }); From a3b048982ac1ace3c00784f040c122a43664d5bb Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Mon, 18 Mar 2019 11:00:28 -0500 Subject: [PATCH 09/17] test(integration/config.spec.js): Debug why this doesn't work in CI --- test/integration/config.spec.js | 47 ++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/test/integration/config.spec.js b/test/integration/config.spec.js index 7f48430db2..dc6cfe717a 100644 --- a/test/integration/config.spec.js +++ b/test/integration/config.spec.js @@ -61,22 +61,45 @@ describe('config', function() { describe('when configuring Mocha via package', function() { var projRootDir = getProjectRootDir(); - var configDir = path.join('fixtures', 'config'); + var modulesDir = path.join(projRootDir, 'node_modules'); + var configDir = path.join(__dirname, 'fixtures', 'config'); var pkgName = 'mocha-config'; - var pkgDir = path.join(__dirname, configDir, pkgName); + var addedPkgToNodeModules = false; + var pkgInNodeModules = false; before(function() { - fs.symlinkSync( - pkgDir, - path.join(projRootDir, 'node_modules', pkgName), - 'dir' - ); + try { + var srcPath = path.join(configDir, pkgName); + var targetPath = path.join(modulesDir, pkgName); + fs.symlinkSync(srcPath, targetPath, 'dir'); + pkgInNodeModules = true; + addedPkgToNodeModules = true; + if (process.platform !== 'win32') { + require('child_process').execSync('ls -al ' + targetPath); + } + } catch (err) { + if (err.code === 'EEXIST') { + console.log('setup:', 'package already exists in "node_modules"'); + pkgInNodeModules = true; + } else { + console.error('setup failed:', err); + } + } }); it('should load configuration given valid package name', function() { - var configDir = path.join(__dirname, 'fixtures', 'config'); var js; + if (!pkgInNodeModules) { + this.skip(); + } + + try { + console.log('require.resolve:', require.resolve(pkgName)); + } catch (err) { + console.error('require.resolve:', err); + } + function _loadConfig() { js = loadConfig(pkgName); } @@ -95,7 +118,13 @@ describe('config', function() { }); after(function() { - fs.unlinkSync(path.join(projRootDir, 'node_modules', pkgName)); + if (addedPkgToNodeModules) { + try { + fs.unlinkSync(path.join(modulesDir, pkgName)); + } catch (err) { + console.error('teardown failed:', err); + } + } }); }); }); From 00450dd2ffc15be4386306a97c91255c0674f828 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 19 Mar 2019 03:31:20 -0500 Subject: [PATCH 10/17] test(fixtures/config/mocha-config): Add fixture for Mocha configuration package That moment when you realize the tests are failing b/c you forgot to commit the fixture... --- .../fixtures/config/mocha-config/index.js | 9 +++++++++ .../fixtures/config/mocha-config/package.json | 14 ++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 test/integration/fixtures/config/mocha-config/index.js create mode 100644 test/integration/fixtures/config/mocha-config/package.json diff --git a/test/integration/fixtures/config/mocha-config/index.js b/test/integration/fixtures/config/mocha-config/index.js new file mode 100644 index 0000000000..6bf4e58d03 --- /dev/null +++ b/test/integration/fixtures/config/mocha-config/index.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/mocha-config/package.json b/test/integration/fixtures/config/mocha-config/package.json new file mode 100644 index 0000000000..b00896f3b2 --- /dev/null +++ b/test/integration/fixtures/config/mocha-config/package.json @@ -0,0 +1,14 @@ +{ + "name": "mocha-config", + "version": "1.0.0", + "description": "Configure Mocha via package", + "main": "index.js", + "peerDependencies": { + "mocha": "^6.1.0" + }, + "keywords": [ + "mocha", + "config" + ], + "license": "ISC" +} From 0af6421790935c259df96b78c166370f842a4f57 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Tue, 19 Mar 2019 06:22:31 -0500 Subject: [PATCH 11/17] test(integration/config.spec.js): Remove debug and code cleanup 'Nuff said... --- test/integration/config.spec.js | 37 +++++++++++++-------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/test/integration/config.spec.js b/test/integration/config.spec.js index dc6cfe717a..1690a2b7ab 100644 --- a/test/integration/config.spec.js +++ b/test/integration/config.spec.js @@ -1,7 +1,7 @@ 'use strict'; -// this is not a "functional" test; we aren't invoking the mocha executable. -// instead we just avoid test doubles. +// This is not a "functional" test; we aren't invoking the mocha executable. +// Instead we just avoid test doubles. var fs = require('fs'); var path = require('path'); @@ -61,26 +61,23 @@ describe('config', function() { describe('when configuring Mocha via package', function() { var projRootDir = getProjectRootDir(); - var modulesDir = path.join(projRootDir, 'node_modules'); + var nodeModulesDir = path.join(projRootDir, 'node_modules'); var configDir = path.join(__dirname, 'fixtures', 'config'); var pkgName = 'mocha-config'; - var addedPkgToNodeModules = false; - var pkgInNodeModules = false; + var installedLocally = false; + var symlinkedPkg = false; before(function() { try { var srcPath = path.join(configDir, pkgName); - var targetPath = path.join(modulesDir, pkgName); + var targetPath = path.join(nodeModulesDir, pkgName); fs.symlinkSync(srcPath, targetPath, 'dir'); - pkgInNodeModules = true; - addedPkgToNodeModules = true; - if (process.platform !== 'win32') { - require('child_process').execSync('ls -al ' + targetPath); - } + symlinkedPkg = true; + installedLocally = true; } catch (err) { if (err.code === 'EEXIST') { console.log('setup:', 'package already exists in "node_modules"'); - pkgInNodeModules = true; + installedLocally = true; } else { console.error('setup failed:', err); } @@ -90,16 +87,10 @@ describe('config', function() { it('should load configuration given valid package name', function() { var js; - if (!pkgInNodeModules) { + if (!installedLocally) { this.skip(); } - try { - console.log('require.resolve:', require.resolve(pkgName)); - } catch (err) { - console.error('require.resolve:', err); - } - function _loadConfig() { js = loadConfig(pkgName); } @@ -110,17 +101,19 @@ describe('config', function() { }); it('should throw given invalid package name', function() { + var invalidPkgName = pkgName.toUpperCase(); + function _loadConfig() { - loadConfig(pkgName.toUpperCase()); + loadConfig(invalidPkgName); } expect(_loadConfig, 'to throw'); }); after(function() { - if (addedPkgToNodeModules) { + if (symlinkedPkg) { try { - fs.unlinkSync(path.join(modulesDir, pkgName)); + fs.unlinkSync(path.join(nodeModulesDir, pkgName)); } catch (err) { console.error('teardown failed:', err); } From 24795eec3267a9925591b46becf78b7b3b39538a Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 5 Apr 2019 13:03:37 -0500 Subject: [PATCH 12/17] refactor(mocha-config/package.json): Update for later --- test/integration/fixtures/config/mocha-config/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/fixtures/config/mocha-config/package.json b/test/integration/fixtures/config/mocha-config/package.json index b00896f3b2..c6fe4df0f6 100644 --- a/test/integration/fixtures/config/mocha-config/package.json +++ b/test/integration/fixtures/config/mocha-config/package.json @@ -4,11 +4,11 @@ "description": "Configure Mocha via package", "main": "index.js", "peerDependencies": { - "mocha": "^6.1.0" + "mocha": "^7.0.0" }, "keywords": [ "mocha", "config" ], - "license": "ISC" + "license": "CC0" } From 55ce117bf72e6136976e428bc840a8d8c47699cb Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 5 Apr 2019 13:06:00 -0500 Subject: [PATCH 13/17] test(integration/config.spec.js): Updated integration test --- test/integration/config.spec.js | 111 +++++++++++++------------------- 1 file changed, 43 insertions(+), 68 deletions(-) diff --git a/test/integration/config.spec.js b/test/integration/config.spec.js index 1690a2b7ab..8d81bec9ab 100644 --- a/test/integration/config.spec.js +++ b/test/integration/config.spec.js @@ -17,20 +17,8 @@ describe('config', function() { expect(json, 'to equal', yaml); }); - /** - * @returns {String} pathname to Mocha project root directory - */ - function getProjectRootDir() { - var searchPaths = module.parent.paths; - for (var i = 0, len = searchPaths.length; i < len; i++) { - var searchPath = searchPaths[i]; - if (fs.existsSync(searchPath)) { - return path.dirname(searchPath); - } - } - } - describe('when configuring Mocha via a ".js" file', function() { + var projRootDir = path.join(__dirname, '..', '..'); var configDir = path.join(__dirname, 'fixtures', 'config'); var json = loadConfig(path.join(configDir, 'mocharc.json')); @@ -45,8 +33,7 @@ describe('config', function() { expect(js, 'to equal', json); }); - it('should load configuration given relative path', function() { - var projRootDir = getProjectRootDir(); + it('should load configuration given cwd-relative path', function() { var relConfigDir = configDir.substring(projRootDir.length + 1); var js; @@ -57,67 +44,55 @@ describe('config', function() { expect(_loadConfig, 'not to throw'); expect(js, 'to equal', json); }); - }); - describe('when configuring Mocha via package', function() { - var projRootDir = getProjectRootDir(); - var nodeModulesDir = path.join(projRootDir, 'node_modules'); - var configDir = path.join(__dirname, 'fixtures', 'config'); - var pkgName = 'mocha-config'; - var installedLocally = false; - var symlinkedPkg = false; - - before(function() { - try { - var srcPath = path.join(configDir, pkgName); - var targetPath = path.join(nodeModulesDir, pkgName); - fs.symlinkSync(srcPath, targetPath, 'dir'); - symlinkedPkg = true; - installedLocally = true; - } catch (err) { - if (err.code === 'EEXIST') { - console.log('setup:', 'package already exists in "node_modules"'); + // In other words, path does not begin with '/', './', or '../' + describe('when path is neither absolute or relative', function() { + var nodeModulesDir = path.join(projRootDir, 'node_modules'); + var pkgName = 'mocha-config'; + var installedLocally = false; + var symlinkedPkg = false; + + before(function() { + try { + var srcPath = path.join(configDir, pkgName); + var targetPath = path.join(nodeModulesDir, pkgName); + fs.symlinkSync(srcPath, targetPath, 'dir'); + symlinkedPkg = true; installedLocally = true; - } else { - console.error('setup failed:', err); + } catch (err) { + if (err.code === 'EEXIST') { + console.log('setup:', 'package already exists in "node_modules"'); + installedLocally = true; + } else { + console.error('setup failed:', err); + } } - } - }); - - it('should load configuration given valid package name', function() { - var js; - - if (!installedLocally) { - this.skip(); - } - - function _loadConfig() { - js = loadConfig(pkgName); - } + }); - expect(_loadConfig, 'not to throw'); - var json = loadConfig(path.join(configDir, 'mocharc.json')); - expect(js, 'to equal', json); - }); - - it('should throw given invalid package name', function() { - var invalidPkgName = pkgName.toUpperCase(); + it('should load configuration given module-relative path', function() { + var js; - function _loadConfig() { - loadConfig(invalidPkgName); - } + if (!installedLocally) { + return this.skip(); + } - expect(_loadConfig, 'to throw'); - }); + function _loadConfig() { + js = loadConfig(path.join(pkgName, 'index.js')); + } - after(function() { - if (symlinkedPkg) { - try { - fs.unlinkSync(path.join(nodeModulesDir, pkgName)); - } catch (err) { - console.error('teardown failed:', err); + expect(_loadConfig, 'not to throw'); + expect(js, 'to equal', json); + }); + + after(function() { + if (symlinkedPkg) { + try { + fs.unlinkSync(path.join(nodeModulesDir, pkgName)); + } catch (err) { + console.error('teardown failed:', err); + } } - } + }); }); }); }); From 9d466285279ae8d91eb71fec49c8fbe5078144b2 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 5 Apr 2019 13:08:26 -0500 Subject: [PATCH 14/17] test(cli/config.spec.js): Updated CLI test Reverted back to use JSON parser by default. --- test/node-unit/cli/config.spec.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/node-unit/cli/config.spec.js b/test/node-unit/cli/config.spec.js index 7de4007327..2823cdcd24 100644 --- a/test/node-unit/cli/config.spec.js +++ b/test/node-unit/cli/config.spec.js @@ -82,14 +82,12 @@ describe('cli/config', function() { describe('when supplied a filepath with unsupported extension', function() { beforeEach(function() { - sandbox.stub(parsers, 'yaml').returns(config); sandbox.stub(parsers, 'json').returns(config); - sandbox.stub(parsers, 'js').returns(config); }); - it('should use the JS parser', function() { + it('should use the JSON parser', function() { loadConfig('foo.bar'); - expect(parsers.js, 'was called'); + expect(parsers.json, 'was called'); }); }); From 7251c18be9b091e0d9ebff0b16fe339a2361c1d1 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 5 Apr 2019 13:12:02 -0500 Subject: [PATCH 15/17] refactor(cli/config.js): Updates per PR review Reverted `loadConfig` to default to JSON parser. Changed `parsers.js` to fallback to module-relative loading, removing the package loader. --- lib/cli/config.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/cli/config.js b/lib/cli/config.js index 45f0f9cf1b..6fa4e2dbca 100644 --- a/lib/cli/config.js +++ b/lib/cli/config.js @@ -45,11 +45,8 @@ const parsers = (exports.parsers = { debug(`parsers: load using cwd-relative path: "${cwdFilepath}"`); return require(cwdFilepath); } catch (err) { - if ( - isModuleNotFoundError(err) && - require('validate-npm-package-name')(filepath).validForNewPackages - ) { - debug(`parsers: retry load as package: "${filepath}"`); + if (isModuleNotFoundError(err)) { + debug(`parsers: retry load as module-relative path: "${filepath}"`); return require(filepath); } else { throw err; // rethrow @@ -72,16 +69,16 @@ const parsers = (exports.parsers = { */ exports.loadConfig = filepath => { let config = {}; - const ext = path.extname(filepath); - debug(`loadConfig: "${filepath}"`); + + const ext = path.extname(filepath); try { if (ext === '.yml' || ext === '.yaml') { config = parsers.yaml(filepath); - } else if (ext === '.json' || ext === '.jsonc') { - config = parsers.json(filepath); - } else { + } else if (ext === '.js') { config = parsers.js(filepath); + } else { + config = parsers.json(filepath); } } catch (err) { throw new Error(`failed to parse config "${filepath}": ${err}`); From b51af9d9bcd9e8f1593ccde35d39d0f53d2eb5f3 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 5 Apr 2019 13:15:44 -0500 Subject: [PATCH 16/17] build(package.json): Revert addition of "validate-npm-package-name" package --- package-lock.json | 8 -------- package.json | 1 - 2 files changed, 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7e714cc8ab..df7af5753e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20930,14 +20930,6 @@ "spdx-expression-parse": "^3.0.0" } }, - "validate-npm-package-name": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/validate-npm-package-name/-/validate-npm-package-name-3.0.0.tgz", - "integrity": "sha1-X6kS2B630MdK/BQN5zF/DKffQ34=", - "requires": { - "builtins": "^1.0.3" - } - }, "vendors": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/vendors/-/vendors-1.0.2.tgz", diff --git a/package.json b/package.json index 0aedfb0c71..773bf7bcf8 100644 --- a/package.json +++ b/package.json @@ -510,7 +510,6 @@ "object.assign": "4.1.0", "strip-json-comments": "2.0.1", "supports-color": "6.0.0", - "validate-npm-package-name": "^3.0.0", "which": "1.3.1", "wide-align": "1.1.3", "yargs": "13.2.2", From 4853993aa9e146c7b5b234a4f56995d3e230e910 Mon Sep 17 00:00:00 2001 From: Paul Roebuck Date: Fri, 5 Apr 2019 13:55:39 -0500 Subject: [PATCH 17/17] Revert "build(package.json): Add "validate-npm-package-name" package" --- package-lock.json | 5 ----- 1 file changed, 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index df7af5753e..cce95ce7d1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2478,11 +2478,6 @@ "integrity": "sha1-hZgoeOIbmOHGZCXgPQF0eI9Wnug=", "dev": true }, - "builtins": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/builtins/-/builtins-1.0.3.tgz", - "integrity": "sha1-y5T662HIaWRR2zZTThQi+U8K7og=" - }, "bytes": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.0.0.tgz",