From dc87eba1dcae4e6087be6723bfaff431c37357c0 Mon Sep 17 00:00:00 2001 From: "Trevor D. McKeown" Date: Fri, 2 Feb 2024 10:16:00 -0500 Subject: [PATCH 1/6] Additional Configuration File Support (#436) feat: Adding additional configuration file options test: unit test for config file detection test: unit test for config files pass via the cli test: unit test case for early exit of hideInstrumenteeArgs test: unit test for Node.js argument handling fix: bug in spawning of c8 process that dropped coverage significantly --- README.md | 20 +++- lib/parse-args.js | 58 ++++++++++- package-lock.json | 33 +++--- package.json | 1 + test/fixtures/config/.c8.config.cjs | 11 ++ test/fixtures/config/.c8.config.js | 11 ++ test/fixtures/config/.c8.config.py | 11 ++ test/fixtures/config/.c8rc | 10 ++ test/fixtures/config/.c8rc.cjs | 11 ++ test/fixtures/config/.c8rc.js | 11 ++ test/fixtures/config/.c8rc.yaml | 8 ++ test/fixtures/config/.c8rc.yml | 8 ++ test/fixtures/config/.nyc.config.cjs | 12 +++ test/fixtures/config/.nyc.config.js | 12 +++ test/fixtures/config/.nycrc | 7 ++ test/fixtures/config/.nycrc.json | 9 ++ test/fixtures/config/.nycrc.yaml | 7 ++ test/fixtures/config/.nycrc.yml | 7 ++ test/fixtures/config/c8.config.cjs | 12 +++ test/fixtures/config/c8.config.js | 12 +++ test/fixtures/config/nyc.config.cjs | 11 ++ test/fixtures/config/nyc.config.js | 11 ++ test/integration.js.snap | 20 +++- test/parse-args-helper.js | 34 ++++++ test/parse-args.js | 148 ++++++++++++++++++++++++++- 25 files changed, 461 insertions(+), 34 deletions(-) create mode 100644 test/fixtures/config/.c8.config.cjs create mode 100644 test/fixtures/config/.c8.config.js create mode 100644 test/fixtures/config/.c8.config.py create mode 100644 test/fixtures/config/.c8rc create mode 100644 test/fixtures/config/.c8rc.cjs create mode 100644 test/fixtures/config/.c8rc.js create mode 100644 test/fixtures/config/.c8rc.yaml create mode 100644 test/fixtures/config/.c8rc.yml create mode 100644 test/fixtures/config/.nyc.config.cjs create mode 100644 test/fixtures/config/.nyc.config.js create mode 100644 test/fixtures/config/.nycrc create mode 100644 test/fixtures/config/.nycrc.json create mode 100644 test/fixtures/config/.nycrc.yaml create mode 100644 test/fixtures/config/.nycrc.yml create mode 100644 test/fixtures/config/c8.config.cjs create mode 100644 test/fixtures/config/c8.config.js create mode 100644 test/fixtures/config/nyc.config.cjs create mode 100644 test/fixtures/config/nyc.config.js create mode 100644 test/parse-args-helper.js diff --git a/README.md b/README.md index a266a1da..2712e420 100644 --- a/README.md +++ b/README.md @@ -18,13 +18,25 @@ The above example will output coverage metrics for `foo.js`. ## CLI Options / Configuration -c8 can be configured via command-line flags, a `c8` section in `package.json`, or a JSON configuration file on disk. - -A configuration file can be specified by passing its path on the command line with `--config` or `-c`. If no config option is provided, c8 searches for files named `.c8rc`, `.c8rc.json`, `.nycrc`, or `.nycrc.json`, starting from -`cwd` and walking up the filesystem tree. +c8 can be configured via command-line flags, a `c8` section in `package.json`, or a configuration file on disk. When using `package.json` configuration or a dedicated configuration file, omit the `--` prefix from the long-form of the desired command-line option. +A configuration file can be specified by passing its path on the command line with `--config` or `-c`. If no config option is provided, c8 searches for files named in the table below starting from `cwd` and walking up the filesystem tree. + +A robust configuration file naming convention is available in an effort to stay compatible with nyc configuration options and ensure dynamic configuration. + +| File name | File Association | +|-----------------------------------------------------------------------------------------------------|--------------------| +| `.c8rc`, `.c8rc.json` | JSON | +| `.c8rc.yml`, `.c8rc.yaml` | YAML | +| `.c8rc.js`, `.c8rc.cjs`, `.c8.config.js`, `.c8.config.cjs`, `c8.config.js`, `c8.config.cjs` | CommonJS export* | +| `.nycrc`, `.nycrc.json` | JSON | +| `.nycrc.yaml`, `.nycrc.yml` | YAML | +| `.nycrc.js`, `.nycrc.cjs`, `nyc.config.js`, `nyc.config.cjs`, `.nyc.config.js`, `.nyc.config.cjs` | CommonJS export* | + +For packages written in ESM module syntax, a static configuration option is supported in JSON or YAML syntax. A dynamic configuration is also supported. These configuration files must be written in CommonJS utilizing one of the .cjs file options in the table above. At the moment ESM syntax is not supported for writing c8 configuration files. This may change in the future, but please note, C8 is written in CommonJS syntax. + Here is a list of common options. Run `c8 --help` for the full list and documentation. | Option | Description | Type | Default | diff --git a/lib/parse-args.js b/lib/parse-args.js index 21a9cd7a..9c9d08f2 100644 --- a/lib/parse-args.js +++ b/lib/parse-args.js @@ -5,7 +5,7 @@ const { readFileSync } = require('fs') const Yargs = require('yargs/yargs') const { applyExtends } = require('yargs/helpers') const parser = require('yargs-parser') -const { resolve } = require('path') +const { resolve, extname, basename } = require('path') function buildYargs (withCommands = false) { const yargs = Yargs([]) @@ -13,12 +13,12 @@ function buildYargs (withCommands = false) { .options('config', { alias: 'c', config: true, - describe: 'path to JSON configuration file', + describe: 'path to configuration file', configParser: (path) => { - const config = JSON.parse(readFileSync(path)) + const config = loadConfigFile(path) return applyExtends(config, process.cwd(), true) }, - default: () => findUp.sync(['.c8rc', '.c8rc.json', '.nycrc', '.nycrc.json']) + default: () => findUp.sync(getConfigFileNames()) }) .option('reporter', { alias: 'r', @@ -194,6 +194,52 @@ function buildYargs (withCommands = false) { return yargs } +function loadConfigFile (path) { + let config = {} + const jsExts = ['.js', '.cjs'] + const ymlExts = ['.yml', '.yaml'] + const fileName = basename(path) + + const ext = extname(path).toLowerCase() + if (jsExts.includes(ext)) { + config = require(path) + } else if (ymlExts.includes(ext)) { + config = require('js-yaml').load(readFileSync(path, 'utf8')) + } else if (ext === '.json' || fileName.slice(-2) === 'rc') { + config = JSON.parse(readFileSync(path, 'utf8')) + } + + // Should the process die if none of these filename extensions are found? + if (Object.keys(config).length === 0) { + throw new Error(`Unsupported file type ${ext} while reading file ${path}`) + } + + return config +} + +function getConfigFileNames () { + return [ + '.c8rc', + '.c8rc.json', + '.c8rc.yml', + '.c8rc.yaml', + '.c8rc.js', + '.c8rc.cjs', + '.c8.config.js', + '.c8.config.cjs', + 'c8.config.js', + 'c8.config.cjs', + '.nycrc', + '.nycrc.json', + '.nycrc.yml', + '.nycrc.yaml', + '.nyc.config.js', + '.nyc.config.cjs', + 'nyc.config.js', + 'nyc.config.cjs' + ] +} + function hideInstrumenterArgs (yargv) { let argv = process.argv.slice(1) argv = argv.slice(argv.indexOf(yargv._[0])) @@ -220,5 +266,7 @@ function hideInstrumenteeArgs () { module.exports = { buildYargs, hideInstrumenterArgs, - hideInstrumenteeArgs + hideInstrumenteeArgs, + getConfigFileNames, + loadConfigFile } diff --git a/package-lock.json b/package-lock.json index 2c60c943..4df36c53 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,6 +16,7 @@ "istanbul-lib-coverage": "^3.2.0", "istanbul-lib-report": "^3.0.1", "istanbul-reports": "^3.1.6", + "js-yaml": "^4.1.0", "test-exclude": "^6.0.0", "v8-to-istanbul": "^9.0.0", "yargs": "^17.7.2", @@ -222,9 +223,9 @@ "dev": true }, "node_modules/@types/node": { - "version": "20.10.6", - "resolved": "https://registry.npmjs.org/@types/node/-/node-20.10.6.tgz", - "integrity": "sha512-Vac8H+NlRNNlAmDfGUP7b5h/KA+AtWIzuXy0E6OyP8f1tCLYAtPvKRRDJjAPqhpCb0t6U2j7/xqAuLEebW2kiw==", + "version": "20.10.7", + "resolved": "https://registry.npmjs.org/@types/node/-/node-20.10.7.tgz", + "integrity": "sha512-fRbIKb8C/Y2lXxB5eVMj4IU7xpdox0Lh8bUPEdtLysaylsml1hOOx1+STloRs/B9nf7C6kPRmmg/V7aQW7usNg==", "dev": true, "dependencies": { "undici-types": "~5.26.4" @@ -334,8 +335,7 @@ "node_modules/argparse": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", - "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", - "dev": true + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==" }, "node_modules/array-buffer-byte-length": { "version": "1.0.0", @@ -533,9 +533,9 @@ } }, "node_modules/chai": { - "version": "4.3.10", - "resolved": "https://registry.npmjs.org/chai/-/chai-4.3.10.tgz", - "integrity": "sha512-0UXG04VuVbruMUYbJ6JctvH0YnC/4q3/AkT18q4NaITo91CUm0liMS9VqzT9vZhVQ/1eqPanMWjBM+Juhfb/9g==", + "version": "4.4.0", + "resolved": "https://registry.npmjs.org/chai/-/chai-4.4.0.tgz", + "integrity": "sha512-x9cHNq1uvkCdU+5xTkNh5WtgD4e4yDFCsp9jVc7N7qVeKeftv3gO/ZrviX5d+3ZfxdYnZXZYujjRInu1RogU6A==", "dev": true, "dependencies": { "assertion-error": "^1.1.0", @@ -2463,7 +2463,6 @@ "version": "4.1.0", "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", "integrity": "sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==", - "dev": true, "dependencies": { "argparse": "^2.0.1" }, @@ -4737,9 +4736,9 @@ "dev": true }, "@types/node": { - "version": "20.10.6", - "resolved": "https://registry.npmjs.org/@types/node/-/node-20.10.6.tgz", - "integrity": "sha512-Vac8H+NlRNNlAmDfGUP7b5h/KA+AtWIzuXy0E6OyP8f1tCLYAtPvKRRDJjAPqhpCb0t6U2j7/xqAuLEebW2kiw==", + "version": "20.10.7", + "resolved": "https://registry.npmjs.org/@types/node/-/node-20.10.7.tgz", + "integrity": "sha512-fRbIKb8C/Y2lXxB5eVMj4IU7xpdox0Lh8bUPEdtLysaylsml1hOOx1+STloRs/B9nf7C6kPRmmg/V7aQW7usNg==", "dev": true, "requires": { "undici-types": "~5.26.4" @@ -4822,8 +4821,7 @@ "argparse": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/argparse/-/argparse-2.0.1.tgz", - "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==", - "dev": true + "integrity": "sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q==" }, "array-buffer-byte-length": { "version": "1.0.0", @@ -4964,9 +4962,9 @@ "dev": true }, "chai": { - "version": "4.3.10", - "resolved": "https://registry.npmjs.org/chai/-/chai-4.3.10.tgz", - "integrity": "sha512-0UXG04VuVbruMUYbJ6JctvH0YnC/4q3/AkT18q4NaITo91CUm0liMS9VqzT9vZhVQ/1eqPanMWjBM+Juhfb/9g==", + "version": "4.4.0", + "resolved": "https://registry.npmjs.org/chai/-/chai-4.4.0.tgz", + "integrity": "sha512-x9cHNq1uvkCdU+5xTkNh5WtgD4e4yDFCsp9jVc7N7qVeKeftv3gO/ZrviX5d+3ZfxdYnZXZYujjRInu1RogU6A==", "dev": true, "requires": { "assertion-error": "^1.1.0", @@ -6380,7 +6378,6 @@ "version": "4.1.0", "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.0.tgz", "integrity": "sha512-wpxZs9NoxZaJESJGIZTyDEaYpl0FKSA+FB9aJiyemKhMwkxQg63h4T1KJgUGHpTqPDNRcmmYLugrRjJlBtWvRA==", - "dev": true, "requires": { "argparse": "^2.0.1" } diff --git a/package.json b/package.json index 59446446..4ef63bf7 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "istanbul-lib-coverage": "^3.2.0", "istanbul-lib-report": "^3.0.1", "istanbul-reports": "^3.1.6", + "js-yaml": "^4.1.0", "test-exclude": "^6.0.0", "v8-to-istanbul": "^9.0.0", "yargs": "^17.7.2", diff --git a/test/fixtures/config/.c8.config.cjs b/test/fixtures/config/.c8.config.cjs new file mode 100644 index 00000000..290e529a --- /dev/null +++ b/test/fixtures/config/.c8.config.cjs @@ -0,0 +1,11 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "lines": 45, + "branches": "72", + "statements": "65" +} + +module.exports = config diff --git a/test/fixtures/config/.c8.config.js b/test/fixtures/config/.c8.config.js new file mode 100644 index 00000000..4e420f80 --- /dev/null +++ b/test/fixtures/config/.c8.config.js @@ -0,0 +1,11 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "lines": 47, + "branches": "72", + "statements": "65" +} + +module.exports = config diff --git a/test/fixtures/config/.c8.config.py b/test/fixtures/config/.c8.config.py new file mode 100644 index 00000000..290e529a --- /dev/null +++ b/test/fixtures/config/.c8.config.py @@ -0,0 +1,11 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "lines": 45, + "branches": "72", + "statements": "65" +} + +module.exports = config diff --git a/test/fixtures/config/.c8rc b/test/fixtures/config/.c8rc new file mode 100644 index 00000000..acbcfcee --- /dev/null +++ b/test/fixtures/config/.c8rc @@ -0,0 +1,10 @@ + { + "reporter": [ + "lcov", + "json" + ], + "statements": 29, + "branches": 81, + "lines": 78, + "functions": 93 + } diff --git a/test/fixtures/config/.c8rc.cjs b/test/fixtures/config/.c8rc.cjs new file mode 100644 index 00000000..52218a57 --- /dev/null +++ b/test/fixtures/config/.c8rc.cjs @@ -0,0 +1,11 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "lines": 32, + "branches": "72", + "statements": "65" +} + +module.exports = config diff --git a/test/fixtures/config/.c8rc.js b/test/fixtures/config/.c8rc.js new file mode 100644 index 00000000..b13cb027 --- /dev/null +++ b/test/fixtures/config/.c8rc.js @@ -0,0 +1,11 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "lines": 22, + "branches": "72", + "statements": "65" +} + +module.exports = config diff --git a/test/fixtures/config/.c8rc.yaml b/test/fixtures/config/.c8rc.yaml new file mode 100644 index 00000000..fb8519bb --- /dev/null +++ b/test/fixtures/config/.c8rc.yaml @@ -0,0 +1,8 @@ +--- +reporter: +- html +- text +statements: 83 +branches: 88 +lines: 10 +functions: 12 diff --git a/test/fixtures/config/.c8rc.yml b/test/fixtures/config/.c8rc.yml new file mode 100644 index 00000000..93795b31 --- /dev/null +++ b/test/fixtures/config/.c8rc.yml @@ -0,0 +1,8 @@ +--- +reporter: +- html +- text +statements: 89 +branches: 58 +lines: 69 +functions: 54 diff --git a/test/fixtures/config/.nyc.config.cjs b/test/fixtures/config/.nyc.config.cjs new file mode 100644 index 00000000..cda7b494 --- /dev/null +++ b/test/fixtures/config/.nyc.config.cjs @@ -0,0 +1,12 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "statements": 54, + "branches": 40, + "lines": 71, + "functions": 0 +} + +module.exports = config diff --git a/test/fixtures/config/.nyc.config.js b/test/fixtures/config/.nyc.config.js new file mode 100644 index 00000000..19ca1525 --- /dev/null +++ b/test/fixtures/config/.nyc.config.js @@ -0,0 +1,12 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "statements": 54, + "branches": 40, + "lines": 85, + "functions": 0 +} + +module.exports = config diff --git a/test/fixtures/config/.nycrc b/test/fixtures/config/.nycrc new file mode 100644 index 00000000..ed44728c --- /dev/null +++ b/test/fixtures/config/.nycrc @@ -0,0 +1,7 @@ + { + "statements": 95, + "branches": 40, + "lines": 51, + "functions": 89 + } + \ No newline at end of file diff --git a/test/fixtures/config/.nycrc.json b/test/fixtures/config/.nycrc.json new file mode 100644 index 00000000..b9c9a301 --- /dev/null +++ b/test/fixtures/config/.nycrc.json @@ -0,0 +1,9 @@ +{ + "reporter": [ + "html", + "text" + ], + "lines": 96, + "branches": "82", + "statements": "95" +} diff --git a/test/fixtures/config/.nycrc.yaml b/test/fixtures/config/.nycrc.yaml new file mode 100644 index 00000000..c7d8c2ff --- /dev/null +++ b/test/fixtures/config/.nycrc.yaml @@ -0,0 +1,7 @@ +--- +reporter: +- html +- text +lines: 98 +branches: '82' +statements: '95' diff --git a/test/fixtures/config/.nycrc.yml b/test/fixtures/config/.nycrc.yml new file mode 100644 index 00000000..602b4999 --- /dev/null +++ b/test/fixtures/config/.nycrc.yml @@ -0,0 +1,7 @@ +--- +reporter: +- html +- text +lines: 99 +branches: '82' +statements: '95' diff --git a/test/fixtures/config/c8.config.cjs b/test/fixtures/config/c8.config.cjs new file mode 100644 index 00000000..400266dc --- /dev/null +++ b/test/fixtures/config/c8.config.cjs @@ -0,0 +1,12 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "statements": 95, + "branches": 40, + "lines": 51, + "functions": 89 +} + +module.exports = config diff --git a/test/fixtures/config/c8.config.js b/test/fixtures/config/c8.config.js new file mode 100644 index 00000000..e23d4ae5 --- /dev/null +++ b/test/fixtures/config/c8.config.js @@ -0,0 +1,12 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "statements": 54, + "branches": 40, + "lines": 47, + "functions": 0 +} + +module.exports = config diff --git a/test/fixtures/config/nyc.config.cjs b/test/fixtures/config/nyc.config.cjs new file mode 100644 index 00000000..c602be3b --- /dev/null +++ b/test/fixtures/config/nyc.config.cjs @@ -0,0 +1,11 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "lines": 94, + "branches": "82", + "statements": "95" +} + +module.exports = config diff --git a/test/fixtures/config/nyc.config.js b/test/fixtures/config/nyc.config.js new file mode 100644 index 00000000..6b03355c --- /dev/null +++ b/test/fixtures/config/nyc.config.js @@ -0,0 +1,11 @@ +const config = { + "reporter": [ + "html", + "text" + ], + "lines": 95, + "branches": "82", + "statements": "95" +} + +module.exports = config diff --git a/test/integration.js.snap b/test/integration.js.snap index a9279c7e..b2a45086 100644 --- a/test/integration.js.snap +++ b/test/integration.js.snap @@ -156,7 +156,7 @@ hey ---------------------------------------|---------|----------|---------|---------|------------------------ File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------------|---------|----------|---------|---------|------------------------ -All files | 3.52 | 12.5 | 6.52 | 3.52 | +All files | 3.31 | 11.32 | 5.88 | 3.31 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -166,7 +166,7 @@ All files | 3.52 | 12.5 | 6.52 | 3.52 prettify.js | 0 | 0 | 0 | 0 | 1-2 sorter.js | 0 | 0 | 0 | 0 | 1-196 c8/lib | 0 | 0 | 0 | 0 | - parse-args.js | 0 | 0 | 0 | 0 | 1-224 + parse-args.js | 0 | 0 | 0 | 0 | 1-272 report.js | 0 | 0 | 0 | 0 | 1-402 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 c8/lib/commands | 0 | 0 | 0 | 0 | @@ -194,6 +194,12 @@ All files | 3.52 | 12.5 | 6.52 | 3.52 main.js | 0 | 0 | 0 | 0 | 1-4 c8/test/fixtures/all/vanilla/dir | 0 | 0 | 0 | 0 | unloaded.js | 0 | 0 | 0 | 0 | 1-5 + c8/test/fixtures/config | 0 | 0 | 0 | 0 | + .c8.config.js | 0 | 0 | 0 | 0 | 1-11 + .c8rc.js | 0 | 0 | 0 | 0 | 1-11 + .nyc.config.js | 0 | 0 | 0 | 0 | 1-12 + c8.config.js | 0 | 0 | 0 | 0 | 1-12 + nyc.config.js | 0 | 0 | 0 | 0 | 1-11 c8/test/fixtures/multidir1 | 0 | 0 | 0 | 0 | file1.js | 0 | 0 | 0 | 0 | 1 c8/test/fixtures/multidir2 | 0 | 0 | 0 | 0 | @@ -521,7 +527,7 @@ hey ---------------------------------------|---------|----------|---------|---------|------------------------ File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------------|---------|----------|---------|---------|------------------------ -All files | 3.52 | 12.5 | 6.52 | 3.52 | +All files | 3.31 | 11.32 | 5.88 | 3.31 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -531,7 +537,7 @@ All files | 3.52 | 12.5 | 6.52 | 3.52 prettify.js | 0 | 0 | 0 | 0 | 1-2 sorter.js | 0 | 0 | 0 | 0 | 1-196 c8/lib | 0 | 0 | 0 | 0 | - parse-args.js | 0 | 0 | 0 | 0 | 1-224 + parse-args.js | 0 | 0 | 0 | 0 | 1-272 report.js | 0 | 0 | 0 | 0 | 1-402 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 c8/lib/commands | 0 | 0 | 0 | 0 | @@ -559,6 +565,12 @@ All files | 3.52 | 12.5 | 6.52 | 3.52 main.js | 0 | 0 | 0 | 0 | 1-4 c8/test/fixtures/all/vanilla/dir | 0 | 0 | 0 | 0 | unloaded.js | 0 | 0 | 0 | 0 | 1-5 + c8/test/fixtures/config | 0 | 0 | 0 | 0 | + .c8.config.js | 0 | 0 | 0 | 0 | 1-11 + .c8rc.js | 0 | 0 | 0 | 0 | 1-11 + .nyc.config.js | 0 | 0 | 0 | 0 | 1-12 + c8.config.js | 0 | 0 | 0 | 0 | 1-12 + nyc.config.js | 0 | 0 | 0 | 0 | 1-11 c8/test/fixtures/multidir1 | 0 | 0 | 0 | 0 | file1.js | 0 | 0 | 0 | 0 | 1 c8/test/fixtures/multidir2 | 0 | 0 | 0 | 0 | diff --git a/test/parse-args-helper.js b/test/parse-args-helper.js new file mode 100644 index 00000000..93b39d1a --- /dev/null +++ b/test/parse-args-helper.js @@ -0,0 +1,34 @@ +const { mkdirSync, copyFileSync, rmSync, existsSync } = require('fs') +const { join } = require('path') + +const testPath = './test/fixtures/tmp-config-test' + +const testConfigFile = function (filePath, fileNameLineNumberMap, callback) { + Object.keys(fileNameLineNumberMap).forEach((fileName) => { + const expectedLines = fileNameLineNumberMap[fileName] + callback(fileName, expectedLines) + }) +} + +const beforeTestReadingConfigFile = (configFileName) => { + afterTestReadingConfigFile() + // make the directory tmp-config-test + mkdirSync(testPath) + + // Copy config file in fileName and test/fixtures/normal.js to dir above + copyFileSync('./test/fixtures/normal.js', join(testPath, '/normal.js')) + copyFileSync('./test/fixtures/async.js', join(testPath, '/async.js')) + copyFileSync('./test/fixtures/config/' + configFileName, join(testPath, configFileName)) +} + +const afterTestReadingConfigFile = () => { + if (existsSync(testPath)) { + rmSync(testPath, { recursive: true, force: true }) + } +} + +module.exports = { + testConfigFile: testConfigFile, + beforeTestReadingConfigFile: beforeTestReadingConfigFile, + afterTestReadingConfigFile: afterTestReadingConfigFile +} diff --git a/test/parse-args.js b/test/parse-args.js index fa2d5267..8e4b5b7c 100644 --- a/test/parse-args.js +++ b/test/parse-args.js @@ -1,12 +1,31 @@ -/* global describe, it */ +/* global describe, it, beforeEach, after */ + +const { assert } = require('chai') +const { existsSync } = require('fs') +const { resolve, join } = require('path') +const chaiJestSnapshot = require('chai-jest-snapshot') +const { spawnSync } = require('child_process') const { buildYargs, hideInstrumenteeArgs, - hideInstrumenterArgs + hideInstrumenterArgs, + getConfigFileNames, + loadConfigFile } = require('../lib/parse-args') -const { join, resolve } = require('path') +const { + testConfigFile, + beforeTestReadingConfigFile, + afterTestReadingConfigFile +} = require('./parse-args-helper.js') + +require('chai') + .use(chaiJestSnapshot) + .should() + +const c8Path = require.resolve('../bin/c8') +const nodePath = process.execPath describe('parse-args', () => { describe('hideInstrumenteeArgs', () => { @@ -15,6 +34,12 @@ describe('parse-args', () => { const instrumenterArgs = hideInstrumenteeArgs() instrumenterArgs.should.eql(['--foo=99', 'my-app']) }) + + it('test early exit from function if no arguments are passed', () => { + process.argv = [] + const instrumenterArgs = hideInstrumenteeArgs() + instrumenterArgs.length.should.eql(0) + }) }) describe('hideInstrumenterArgs', () => { @@ -25,6 +50,14 @@ describe('parse-args', () => { instrumenteeArgs.should.eql(['my-app', '--help']) argv.tempDirectory.endsWith(join('coverage', 'tmp')).should.be.equal(true) }) + + it('interprets first args after -- as Node.js execArgv', async () => { + const expected = [process.execPath, '--expose-gc', 'index.js'] + process.argv = ['node', 'c8', '--', '--expose-gc', 'index.js'] + const argv = buildYargs().parse(hideInstrumenteeArgs()) + const munged = hideInstrumenterArgs(argv) + munged.should.deep.equal(expected) + }) }) describe('with NODE_V8_COVERAGE already set', () => { @@ -39,6 +72,113 @@ describe('parse-args', () => { }) describe('--config', () => { + it('c8 process should throw an error message if an invalid configuration file name is passed', function () { + const invalidConfig = './fixtures/config/.c8.config.py' + const loadInvalidConfigFile = function (file, callBack) { + try { + callBack(file) + assert.fail('Invalid configuration file loaded') + } catch (error) { + const expectErrorValue = `Error: Unsupported file type .py while reading file ${invalidConfig}` + String(error).should.eql(expectErrorValue) + } + } + + loadInvalidConfigFile(invalidConfig, function (file) { + loadConfigFile(file) + }) + }) + + it('config directory should contain all variations of the config file naming convention', () => { + let count = 0 + const fileMessages = [] + const configFileList = getConfigFileNames() + configFileList.forEach((file) => { + const fullPath = './test/fixtures/config/' + file + if (existsSync(fullPath)) { + count++ + } else { + fileMessages.push(`Missing ${file} from ./test/fixtures/config directory`) + } + }) + + if (count === configFileList.length) { + assert.equal(count, configFileList.length) + } else { + const msg = fileMessages.join(' \n ') + assert.equal(fileMessages.length, 0, msg) + } + }) + + describe('should be able to read config files with .json, .yml, .yaml, .js, .cjs extensions', () => { + const filePath = './fixtures/config/' + const testData = { + c8: { + description: 'c8 variations of config file', + fileNameLineNumberMap: { + '.c8rc.json': 101, + '.c8rc.yml': 69, + '.c8rc.yaml': 10, + 'c8.config.js': 47, + 'c8.config.cjs': 51, + '.c8rc.js': 22, + '.c8rc.cjs': 32, + '.c8.config.js': 47, + '.c8.config.cjs': 45 + } + }, + nyc: { + description: 'nyc variations of config file', + fileNameLineNumberMap: { + '.nycrc': 51, + '.nycrc.json': 96, + '.nycrc.yml': 99, + '.nycrc.yaml': 98, + 'nyc.config.js': 95, + 'nyc.config.cjs': 94, + '.nyc.config.js': 85, + '.nyc.config.cjs': 71 + } + } + } + + Object.keys(testData).forEach(key => { + const { description, fileNameLineNumberMap } = testData[key] + describe(description, function () { + testConfigFile(filePath, fileNameLineNumberMap, function (fileName, expectedLines) { + beforeEach(() => beforeTestReadingConfigFile(fileName)) + it(`should be able to resolve config file ${fileName} with --config flag`, () => { + const configFilePath = join(filePath, fileName) + const argv = buildYargs().parse(['node', 'c8', 'my-app', '--config', require.resolve(`./${configFilePath}`)]) + argv.lines.should.be.equal(expectedLines) + }) + + // skipping for the moment. Need another patch for this test to successfully run + it.skip(`should be able to resolve config file ${fileName} by detection`, function () { + // set the snapshot filename + chaiJestSnapshot.setTestName(`should be able to resolve config file ${fileName} by detection`) + chaiJestSnapshot.setFilename('./test/fixtures/config/snapshots/' + fileName + '.snap') + + // Run V8 in the dir above + const { output } = spawnSync(nodePath, + [ + c8Path, + '--temp-directory=tmp/normal', + '--all', + '--src=./test/fixtures/tmp-config-test', + nodePath, + require.resolve('./fixtures/tmp-config-test/normal.js') + ], + { cwd: './test/fixtures/tmp-config-test' } + ) + output.toString('utf8').should.matchSnapshot() + }) + after(afterTestReadingConfigFile) + }) + }) + }) + }) + it('should resolve to .nycrc at cwd', () => { const argv = buildYargs().parse(['node', 'c8', 'my-app']) argv.lines.should.be.equal(95) @@ -47,11 +187,13 @@ describe('parse-args', () => { const argv = buildYargs().parse(['node', 'c8', '--config', require.resolve('./fixtures/config/.c8rc.json')]) argv.lines.should.be.equal(101) argv.tempDirectory.should.be.equal('./foo') + argv.functions.should.be.equal(24) }) it('should have -c as an alias', () => { const argv = buildYargs().parse(['node', 'c8', '-c', require.resolve('./fixtures/config/.c8rc.json')]) argv.lines.should.be.equal(101) argv.tempDirectory.should.be.equal('./foo') + argv.functions.should.be.equal(24) }) it('should respect options on the command line over config file', () => { const argv = buildYargs().parse(['node', 'c8', '--lines', '100', '--config', require.resolve('./fixtures/config/.c8rc.json')]) From 59dfada5cfba67daebe00de3f9227a150ed0b562 Mon Sep 17 00:00:00 2001 From: Ricky C Date: Tue, 23 Jan 2024 22:47:57 -0800 Subject: [PATCH 2/6] refactor: Move config file loading functions into own module Logically these are not part of the argument parsing. They are configuration loading and validation. Having them in the parse-args module was only making the file and the tests harder to work with. This change only has two semantic differences: 1. The list of known config files names was converted to a const immutable array instead of beating a mutable array returned by a function. 2. The parse-args file is no longer exporting those two internal utility functions, instead the equivalents are coming from a dedicated file. --- lib/load-config.js | 51 +++++++ lib/parse-args.js | 57 +------- test/integration.js.snap | 10 +- ...e-args-helper.js => load-config-helper.js} | 0 test/load-config.js | 128 ++++++++++++++++++ test/parse-args.js | 125 +---------------- 6 files changed, 192 insertions(+), 179 deletions(-) create mode 100644 lib/load-config.js rename test/{parse-args-helper.js => load-config-helper.js} (100%) create mode 100644 test/load-config.js diff --git a/lib/load-config.js b/lib/load-config.js new file mode 100644 index 00000000..c10c57c5 --- /dev/null +++ b/lib/load-config.js @@ -0,0 +1,51 @@ +const { readFileSync } = require('fs') +const { extname, basename } = require('path') + +const CONFIG_FILE_NAMES = Object.freeze([ + '.c8rc', + '.c8rc.json', + '.c8rc.yml', + '.c8rc.yaml', + '.c8rc.js', + '.c8rc.cjs', + '.c8.config.js', + '.c8.config.cjs', + 'c8.config.js', + 'c8.config.cjs', + '.nycrc', + '.nycrc.json', + '.nycrc.yml', + '.nycrc.yaml', + '.nyc.config.js', + '.nyc.config.cjs', + 'nyc.config.js', + 'nyc.config.cjs' +]) + +function loadConfigFile (path) { + let config = {} + const jsExts = ['.js', '.cjs'] + const ymlExts = ['.yml', '.yaml'] + const fileName = basename(path) + + const ext = extname(path).toLowerCase() + if (jsExts.includes(ext)) { + config = require(path) + } else if (ymlExts.includes(ext)) { + config = require('js-yaml').load(readFileSync(path, 'utf8')) + } else if (ext === '.json' || fileName.slice(-2) === 'rc') { + config = JSON.parse(readFileSync(path, 'utf8')) + } + + // Should the process die if none of these filename extensions are found? + if (Object.keys(config).length === 0) { + throw new Error(`Unsupported file type ${ext} while reading file ${path}`) + } + + return config +} + +module.exports = { + CONFIG_FILE_NAMES, + loadConfigFile +} diff --git a/lib/parse-args.js b/lib/parse-args.js index 9c9d08f2..40a9d1dd 100644 --- a/lib/parse-args.js +++ b/lib/parse-args.js @@ -1,11 +1,12 @@ const defaultExclude = require('@istanbuljs/schema/default-exclude') const defaultExtension = require('@istanbuljs/schema/default-extension') const findUp = require('find-up') -const { readFileSync } = require('fs') const Yargs = require('yargs/yargs') const { applyExtends } = require('yargs/helpers') const parser = require('yargs-parser') -const { resolve, extname, basename } = require('path') +const { resolve } = require('path') + +const { CONFIG_FILE_NAMES, loadConfigFile } = require('./load-config') function buildYargs (withCommands = false) { const yargs = Yargs([]) @@ -18,7 +19,7 @@ function buildYargs (withCommands = false) { const config = loadConfigFile(path) return applyExtends(config, process.cwd(), true) }, - default: () => findUp.sync(getConfigFileNames()) + default: () => findUp.sync(CONFIG_FILE_NAMES) }) .option('reporter', { alias: 'r', @@ -194,52 +195,6 @@ function buildYargs (withCommands = false) { return yargs } -function loadConfigFile (path) { - let config = {} - const jsExts = ['.js', '.cjs'] - const ymlExts = ['.yml', '.yaml'] - const fileName = basename(path) - - const ext = extname(path).toLowerCase() - if (jsExts.includes(ext)) { - config = require(path) - } else if (ymlExts.includes(ext)) { - config = require('js-yaml').load(readFileSync(path, 'utf8')) - } else if (ext === '.json' || fileName.slice(-2) === 'rc') { - config = JSON.parse(readFileSync(path, 'utf8')) - } - - // Should the process die if none of these filename extensions are found? - if (Object.keys(config).length === 0) { - throw new Error(`Unsupported file type ${ext} while reading file ${path}`) - } - - return config -} - -function getConfigFileNames () { - return [ - '.c8rc', - '.c8rc.json', - '.c8rc.yml', - '.c8rc.yaml', - '.c8rc.js', - '.c8rc.cjs', - '.c8.config.js', - '.c8.config.cjs', - 'c8.config.js', - 'c8.config.cjs', - '.nycrc', - '.nycrc.json', - '.nycrc.yml', - '.nycrc.yaml', - '.nyc.config.js', - '.nyc.config.cjs', - 'nyc.config.js', - 'nyc.config.cjs' - ] -} - function hideInstrumenterArgs (yargv) { let argv = process.argv.slice(1) argv = argv.slice(argv.indexOf(yargv._[0])) @@ -266,7 +221,5 @@ function hideInstrumenteeArgs () { module.exports = { buildYargs, hideInstrumenterArgs, - hideInstrumenteeArgs, - getConfigFileNames, - loadConfigFile + hideInstrumenteeArgs } diff --git a/test/integration.js.snap b/test/integration.js.snap index b2a45086..e33c5b9f 100644 --- a/test/integration.js.snap +++ b/test/integration.js.snap @@ -156,7 +156,7 @@ hey ---------------------------------------|---------|----------|---------|---------|------------------------ File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------------|---------|----------|---------|---------|------------------------ -All files | 3.31 | 11.32 | 5.88 | 3.31 | +All files | 3.3 | 11.11 | 5.76 | 3.3 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -166,7 +166,8 @@ All files | 3.31 | 11.32 | 5.88 | 3.31 prettify.js | 0 | 0 | 0 | 0 | 1-2 sorter.js | 0 | 0 | 0 | 0 | 1-196 c8/lib | 0 | 0 | 0 | 0 | - parse-args.js | 0 | 0 | 0 | 0 | 1-272 + load-config.js | 0 | 0 | 0 | 0 | 1-51 + parse-args.js | 0 | 0 | 0 | 0 | 1-225 report.js | 0 | 0 | 0 | 0 | 1-402 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 c8/lib/commands | 0 | 0 | 0 | 0 | @@ -527,7 +528,7 @@ hey ---------------------------------------|---------|----------|---------|---------|------------------------ File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------------|---------|----------|---------|---------|------------------------ -All files | 3.31 | 11.32 | 5.88 | 3.31 | +All files | 3.3 | 11.11 | 5.76 | 3.3 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -537,7 +538,8 @@ All files | 3.31 | 11.32 | 5.88 | 3.31 prettify.js | 0 | 0 | 0 | 0 | 1-2 sorter.js | 0 | 0 | 0 | 0 | 1-196 c8/lib | 0 | 0 | 0 | 0 | - parse-args.js | 0 | 0 | 0 | 0 | 1-272 + load-config.js | 0 | 0 | 0 | 0 | 1-51 + parse-args.js | 0 | 0 | 0 | 0 | 1-225 report.js | 0 | 0 | 0 | 0 | 1-402 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 c8/lib/commands | 0 | 0 | 0 | 0 | diff --git a/test/parse-args-helper.js b/test/load-config-helper.js similarity index 100% rename from test/parse-args-helper.js rename to test/load-config-helper.js diff --git a/test/load-config.js b/test/load-config.js new file mode 100644 index 00000000..687587bc --- /dev/null +++ b/test/load-config.js @@ -0,0 +1,128 @@ +/* global describe, it, beforeEach, after */ + +const { assert, expect } = require('chai') +const { existsSync } = require('fs') +const { join } = require('path') +const { buildYargs } = require('../lib/parse-args') +const chaiJestSnapshot = require('chai-jest-snapshot') +const { spawnSync } = require('child_process') + +require('chai').should() + +const { + CONFIG_FILE_NAMES, + loadConfigFile +} = require('../lib/load-config') + +const { + testConfigFile, + beforeTestReadingConfigFile, + afterTestReadingConfigFile +} = require('./load-config-helper.js') + +const c8Path = require.resolve('../bin/c8') +const nodePath = process.execPath + +describe(__filename, () => { + describe(loadConfigFile.name, () => { + it('config directory should contain all variations of the config file naming convention', () => { + let count = 0 + const fileMessages = [] + CONFIG_FILE_NAMES.forEach((file) => { + const fullPath = './test/fixtures/config/' + file + if (existsSync(fullPath)) { + count++ + } else { + fileMessages.push( + `Missing ${file} from ./test/fixtures/config directory` + ) + } + }) + + if (count === CONFIG_FILE_NAMES.length) { + assert.equal(count, CONFIG_FILE_NAMES.length) + } else { + const msg = fileMessages.join(' \n ') + assert.equal(fileMessages.length, 0, msg) + } + }) + + describe('should be able to read config files with .json, .yml, .yaml, .js, .cjs extensions', () => { + const filePath = './fixtures/config/' + const testData = { + c8: { + description: 'c8 variations of config file', + fileNameLineNumberMap: { + '.c8rc.json': 101, + '.c8rc.yml': 69, + '.c8rc.yaml': 10, + 'c8.config.js': 47, + 'c8.config.cjs': 51, + '.c8rc.js': 22, + '.c8rc.cjs': 32, + '.c8.config.js': 47, + '.c8.config.cjs': 45 + } + }, + nyc: { + description: 'nyc variations of config file', + fileNameLineNumberMap: { + '.nycrc': 51, + '.nycrc.json': 96, + '.nycrc.yml': 99, + '.nycrc.yaml': 98, + 'nyc.config.js': 95, + 'nyc.config.cjs': 94, + '.nyc.config.js': 85, + '.nyc.config.cjs': 71 + } + } + } + + Object.keys(testData).forEach(key => { + const { description, fileNameLineNumberMap } = testData[key] + describe(description, function () { + testConfigFile(filePath, fileNameLineNumberMap, function (fileName, expectedLines) { + beforeEach(() => beforeTestReadingConfigFile(fileName)) + it(`should be able to resolve config file ${fileName} with --config flag`, () => { + const configFilePath = join(filePath, fileName) + const argv = buildYargs().parse(['node', 'c8', 'my-app', '--config', require.resolve(`./${configFilePath}`)]) + argv.lines.should.be.equal(expectedLines) + }) + + // skipping for the moment. Need another patch for this test to successfully run + it.skip(`should be able to resolve config file ${fileName} by detection`, function () { + // set the snapshot filename + chaiJestSnapshot.setTestName(`should be able to resolve config file ${fileName} by detection`) + chaiJestSnapshot.setFilename('./test/fixtures/config/snapshots/' + fileName + '.snap') + + // Run V8 in the dir above + const { output } = spawnSync(nodePath, + [ + c8Path, + '--temp-directory=tmp/normal', + '--all', + '--src=./test/fixtures/tmp-config-test', + nodePath, + require.resolve('./fixtures/tmp-config-test/normal.js') + ], + { cwd: './test/fixtures/tmp-config-test' } + ) + output.toString('utf8').should.matchSnapshot() + }) + after(afterTestReadingConfigFile) + }) + }) + }) + }) + + it('throws an error message if an invalid configuration file name is passed', function () { + const invalidConfig = './fixtures/config/.c8.config.py' + + expect(() => loadConfigFile(invalidConfig)).to.throw( + Error, + `Unsupported file type .py while reading file ${invalidConfig}` + ) + }) + }) +}) diff --git a/test/parse-args.js b/test/parse-args.js index 8e4b5b7c..5ea45d32 100644 --- a/test/parse-args.js +++ b/test/parse-args.js @@ -1,32 +1,18 @@ -/* global describe, it, beforeEach, after */ +/* global describe, it */ -const { assert } = require('chai') -const { existsSync } = require('fs') const { resolve, join } = require('path') const chaiJestSnapshot = require('chai-jest-snapshot') -const { spawnSync } = require('child_process') const { buildYargs, hideInstrumenteeArgs, - hideInstrumenterArgs, - getConfigFileNames, - loadConfigFile + hideInstrumenterArgs } = require('../lib/parse-args') -const { - testConfigFile, - beforeTestReadingConfigFile, - afterTestReadingConfigFile -} = require('./parse-args-helper.js') - require('chai') .use(chaiJestSnapshot) .should() -const c8Path = require.resolve('../bin/c8') -const nodePath = process.execPath - describe('parse-args', () => { describe('hideInstrumenteeArgs', () => { it('hides arguments passed to instrumented app', () => { @@ -72,113 +58,6 @@ describe('parse-args', () => { }) describe('--config', () => { - it('c8 process should throw an error message if an invalid configuration file name is passed', function () { - const invalidConfig = './fixtures/config/.c8.config.py' - const loadInvalidConfigFile = function (file, callBack) { - try { - callBack(file) - assert.fail('Invalid configuration file loaded') - } catch (error) { - const expectErrorValue = `Error: Unsupported file type .py while reading file ${invalidConfig}` - String(error).should.eql(expectErrorValue) - } - } - - loadInvalidConfigFile(invalidConfig, function (file) { - loadConfigFile(file) - }) - }) - - it('config directory should contain all variations of the config file naming convention', () => { - let count = 0 - const fileMessages = [] - const configFileList = getConfigFileNames() - configFileList.forEach((file) => { - const fullPath = './test/fixtures/config/' + file - if (existsSync(fullPath)) { - count++ - } else { - fileMessages.push(`Missing ${file} from ./test/fixtures/config directory`) - } - }) - - if (count === configFileList.length) { - assert.equal(count, configFileList.length) - } else { - const msg = fileMessages.join(' \n ') - assert.equal(fileMessages.length, 0, msg) - } - }) - - describe('should be able to read config files with .json, .yml, .yaml, .js, .cjs extensions', () => { - const filePath = './fixtures/config/' - const testData = { - c8: { - description: 'c8 variations of config file', - fileNameLineNumberMap: { - '.c8rc.json': 101, - '.c8rc.yml': 69, - '.c8rc.yaml': 10, - 'c8.config.js': 47, - 'c8.config.cjs': 51, - '.c8rc.js': 22, - '.c8rc.cjs': 32, - '.c8.config.js': 47, - '.c8.config.cjs': 45 - } - }, - nyc: { - description: 'nyc variations of config file', - fileNameLineNumberMap: { - '.nycrc': 51, - '.nycrc.json': 96, - '.nycrc.yml': 99, - '.nycrc.yaml': 98, - 'nyc.config.js': 95, - 'nyc.config.cjs': 94, - '.nyc.config.js': 85, - '.nyc.config.cjs': 71 - } - } - } - - Object.keys(testData).forEach(key => { - const { description, fileNameLineNumberMap } = testData[key] - describe(description, function () { - testConfigFile(filePath, fileNameLineNumberMap, function (fileName, expectedLines) { - beforeEach(() => beforeTestReadingConfigFile(fileName)) - it(`should be able to resolve config file ${fileName} with --config flag`, () => { - const configFilePath = join(filePath, fileName) - const argv = buildYargs().parse(['node', 'c8', 'my-app', '--config', require.resolve(`./${configFilePath}`)]) - argv.lines.should.be.equal(expectedLines) - }) - - // skipping for the moment. Need another patch for this test to successfully run - it.skip(`should be able to resolve config file ${fileName} by detection`, function () { - // set the snapshot filename - chaiJestSnapshot.setTestName(`should be able to resolve config file ${fileName} by detection`) - chaiJestSnapshot.setFilename('./test/fixtures/config/snapshots/' + fileName + '.snap') - - // Run V8 in the dir above - const { output } = spawnSync(nodePath, - [ - c8Path, - '--temp-directory=tmp/normal', - '--all', - '--src=./test/fixtures/tmp-config-test', - nodePath, - require.resolve('./fixtures/tmp-config-test/normal.js') - ], - { cwd: './test/fixtures/tmp-config-test' } - ) - output.toString('utf8').should.matchSnapshot() - }) - after(afterTestReadingConfigFile) - }) - }) - }) - }) - it('should resolve to .nycrc at cwd', () => { const argv = buildYargs().parse(['node', 'c8', 'my-app']) argv.lines.should.be.equal(95) From 1b334edfbacbcf08b7e9d1ff944de7765b975e91 Mon Sep 17 00:00:00 2001 From: Ricky C Date: Fri, 2 Feb 2024 08:09:07 -0800 Subject: [PATCH 3/6] fix: Enhance config file loader to handle more use cases Specifically: - Handling the cases of empty files and files that have simple forms of improper content. - Reporting easier to understand errors when the parsers fail. Adding schema validators was deemed beyond the scope of this particular commit, though notes were added for where they should be added. Incorporated many of mcknasty's suggestions. However I didn't keep them all, and many other elements were reinterpreted. --- lib/error-reporting.js | 69 +++++++ lib/load-config.js | 113 +++++++++-- test/error-reporting.js | 67 ++++++ test/fixtures/config/blank.cjs | 0 test/fixtures/config/blank.js | 0 test/fixtures/config/nonstandard.json | 5 + test/integration.js.snap | 12 +- test/load-config.js | 282 ++++++++++++++++++++++---- test/parse-args.js | 6 + 9 files changed, 498 insertions(+), 56 deletions(-) create mode 100644 lib/error-reporting.js create mode 100644 test/error-reporting.js create mode 100644 test/fixtures/config/blank.cjs create mode 100644 test/fixtures/config/blank.js create mode 100644 test/fixtures/config/nonstandard.json diff --git a/lib/error-reporting.js b/lib/error-reporting.js new file mode 100644 index 00000000..353e7d6b --- /dev/null +++ b/lib/error-reporting.js @@ -0,0 +1,69 @@ +/* TODO: Refactor: + * + * Not sure if this the name we want to use for this class + * In the parameters it supports an error that has already + * been thrown, then appends it's own error message + * Is there a more general name we can use like + * AppendableError or MultiError? Is this a good + * Candidate for the decorator design pattern? + * + * + * Note: this is different than javascript Decorators + */ +/** + * To be thrown when there's a problem parsing a configuration file. + * + * More often than not the errors from the parsing engines are opaque and + * unhelpful, so this gives us the opportunity to provide better information + * to the user. + */ +class ConfigParsingError extends Error { + /** + * Constructs the error, given the path and error hints. + * + * @param {string} path The path to the file that had a parsing problem. + * @param {string} errorHints Any user-helpful hints. + * @param {unknown} [originalError] Optional: The original error thrown by the underlying parser. + */ + constructor (path, errorHints, originalError) { + const originalErrorMessage = + originalError instanceof Error + ? ` Original error: ${originalError.message}` + : '' + + super( + `Error loading configuration from file "${path}": ${errorHints}${originalErrorMessage}` + ) + + // this.name = ConfigParsingError.name + + if (originalError instanceof Error) { + this.stack = originalError.stack + } + } +} + +/** + * To be thrown when a file is loaded that is not one of the supported file + * types, especially when the file type is determined from the file extension. + */ +class UnsupportedFileTypeError extends Error { + /** + * Constructs the error, given the path and supported file types. + * + * @param {string} path The path to the file that is not supported. + * @param {string[]} supportedFileTypes An array of supported file types that will help the user understand when they need to do. + */ + constructor (path, supportedFileTypes) { + const types = supportedFileTypes.join(', ') + super( + `Unsupported file type while reading file "${path}". Please make sure your file of one of the following file types: ${types}` + ) + // this.name = UnsupportedFileTypeError.name + } +} + +module.exports = { + ConfigParsingError, + UnsupportedFileTypeError +} diff --git a/lib/load-config.js b/lib/load-config.js index c10c57c5..c484017e 100644 --- a/lib/load-config.js +++ b/lib/load-config.js @@ -1,5 +1,11 @@ const { readFileSync } = require('fs') const { extname, basename } = require('path') +const JsYaml = require('js-yaml') + +const { + UnsupportedFileTypeError, + ConfigParsingError +} = require('./error-reporting') const CONFIG_FILE_NAMES = Object.freeze([ '.c8rc', @@ -22,26 +28,103 @@ const CONFIG_FILE_NAMES = Object.freeze([ 'nyc.config.cjs' ]) -function loadConfigFile (path) { - let config = {} - const jsExts = ['.js', '.cjs'] - const ymlExts = ['.yml', '.yaml'] - const fileName = basename(path) +const JS_EXTS = Object.freeze(['.js', '.cjs']) +const JSON_EXTS = Object.freeze(['.json']) +const YAML_EXTS = Object.freeze(['.yml', '.yaml']) - const ext = extname(path).toLowerCase() - if (jsExts.includes(ext)) { - config = require(path) - } else if (ymlExts.includes(ext)) { - config = require('js-yaml').load(readFileSync(path, 'utf8')) - } else if (ext === '.json' || fileName.slice(-2) === 'rc') { - config = JSON.parse(readFileSync(path, 'utf8')) +const NO_EXPORTS = Symbol('no exports') + +/** + * Loads a configuration file of whatever format from the given path. + * + * @param {string} path The path to load the configuration from. + * @param {readFile(path: string) => string, readJs(path: string) => C8Config} [_di] For test suite use only. Do not use. + * @returns {object} An object containing + * @throws {UnsupportedFileTypeError} When the given configuration file is of a type that is unsupported. + * @throws {ConfigParsingError} When the configuration file fails to be read. E.g. a syntax error, such as not using quoted keys in JSON. + */ +function loadConfigFile (path, _di) { + const di = { + // A variation of the Dependency Injection pattern that allows the test suites to overide any of these functions with mocks. + readFile: (path) => readFileSync(path, 'utf8'), + readJs: (path) => require(path), + ..._di // Note that making the DI argument a hidden argument by using the `arguments` array isn't a viable option in TypeScript, so this has been written in a way that is compatible with that. } - // Should the process die if none of these filename extensions are found? - if (Object.keys(config).length === 0) { - throw new Error(`Unsupported file type ${ext} while reading file ${path}`) + let config + + const fileName = basename(path) + const ext = extname(path).toLowerCase() + + if (YAML_EXTS.includes(ext)) { + try { + // TODO: add YAML schema so that we get better errors for YAML users. + config = JsYaml.load(di.readFile(path)) + } catch (error) { + if (error instanceof JsYaml.YAMLException) { + throw new ConfigParsingError( + path, + 'must contain a valid c8 configuration object.', + error + ) + } + + throw error + } + + if (!config) { + // TODO: remove this check once we get YAML schema validation. + throw new ConfigParsingError(path, 'invalid configuration') + } + } else if (JS_EXTS.includes(ext)) { + // Add a loader that allows us to check to see if anything was ever exported. + // Thank you to https://stackoverflow.com/a/70999950 for the inspiration. Please note that this code won't port to TypeScript nicely. + const extensions = module.constructor._extensions + const cjsLoader = extensions['.cjs'] + const jsLoader = extensions['.js'] + extensions['.cjs'] = extensions['.js'] = (module, filename) => { + module.exports[NO_EXPORTS] = filename + jsLoader(module, filename) + } + try { + config = di.readJs(path) + } finally { + // Undo the global state mutation, even if an error was thrown. + extensions['.cjs'] = cjsLoader + extensions['.js'] = jsLoader + } + if (NO_EXPORTS in config) { + throw new ConfigParsingError( + path, + 'does not export a c8 configuration object.' + ) + } + } else if (JSON_EXTS.includes(ext) || CONFIG_FILE_NAMES.includes(fileName)) { + try { + config = JSON.parse(di.readFile(path)) + } catch (error) { + if (error instanceof SyntaxError) { + throw new ConfigParsingError( + path, + 'must contain a valid c8 configuration object.', + error + ) + } + + throw error + } + } else { + // If the user supplied a bad configuration file that we can't figure out how to read, then it's on them to solve it. + // Attempting to use some other config, even a default one, will result in unexpected behavior: aka ignoring the config that was explicitly specified is not intuitive. + throw new UnsupportedFileTypeError(path, [ + ...JSON_EXTS, + ...JS_EXTS, + ...YAML_EXTS + ]) } + // TODO: validate the object schema so that we get validation of JS-like configs. Might want to refactor the above test cases so that the YAML isn't being validated twice. + return config } diff --git a/test/error-reporting.js b/test/error-reporting.js new file mode 100644 index 00000000..2dfc2f51 --- /dev/null +++ b/test/error-reporting.js @@ -0,0 +1,67 @@ +/* global describe, it */ + +const { expect } = require('chai') + +const { + UnsupportedFileTypeError, + ConfigParsingError +} = require('../lib/error-reporting') + +describe(__filename, () => { + describe(ConfigParsingError.name, () => { + it('is an Error subclass', () => { + expect(new ConfigParsingError('', '')).to.be.instanceof(Error) + }) + + it('has the correct static name property', () => { + expect(ConfigParsingError).to.have.property('name', 'ConfigParsingError') + }) + + it('creates the correct error message when not provided an original error', () => { + expect( + new ConfigParsingError('path/goes/here', 'a hint.') + ).to.have.property( + 'message', + 'Error loading configuration from file "path/goes/here": a hint.' + ) + }) + + it('creates the correct error message when provided an original error', () => { + expect( + new ConfigParsingError( + 'path/goes/here', + 'a hint.', + new Error('some error thrown by a parsing engine') + ) + ).to.have.property( + 'message', + 'Error loading configuration from file "path/goes/here": a hint. Original error: some error thrown by a parsing engine' + ) + }) + }) + + describe(UnsupportedFileTypeError.name, () => { + it('is an Error subclass', () => { + expect(new UnsupportedFileTypeError('', [])).to.be.instanceof(Error) + }) + + it('has the correct static name property', () => { + expect(UnsupportedFileTypeError).to.have.property( + 'name', + 'UnsupportedFileTypeError' + ) + }) + + it('creates the correct error message', () => { + expect( + new UnsupportedFileTypeError('path/goes/here', [ + 'fileType1', + 'fileType2' + ]) + ).to.have.property( + 'message', + 'Unsupported file type while reading file "path/goes/here". Please make sure your file of one of the following file types: fileType1, fileType2' + ) + }) + }) +}) diff --git a/test/fixtures/config/blank.cjs b/test/fixtures/config/blank.cjs new file mode 100644 index 00000000..e69de29b diff --git a/test/fixtures/config/blank.js b/test/fixtures/config/blank.js new file mode 100644 index 00000000..e69de29b diff --git a/test/fixtures/config/nonstandard.json b/test/fixtures/config/nonstandard.json new file mode 100644 index 00000000..00e307d1 --- /dev/null +++ b/test/fixtures/config/nonstandard.json @@ -0,0 +1,5 @@ +{ + "temp-directory": "./foo", + "lines": 420, + "functions": 24 +} diff --git a/test/integration.js.snap b/test/integration.js.snap index e33c5b9f..ee9d0bbe 100644 --- a/test/integration.js.snap +++ b/test/integration.js.snap @@ -156,7 +156,7 @@ hey ---------------------------------------|---------|----------|---------|---------|------------------------ File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------------|---------|----------|---------|---------|------------------------ -All files | 3.3 | 11.11 | 5.76 | 3.3 | +All files | 3.03 | 10.71 | 5.55 | 3.03 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -166,7 +166,8 @@ All files | 3.3 | 11.11 | 5.76 | 3.3 prettify.js | 0 | 0 | 0 | 0 | 1-2 sorter.js | 0 | 0 | 0 | 0 | 1-196 c8/lib | 0 | 0 | 0 | 0 | - load-config.js | 0 | 0 | 0 | 0 | 1-51 + error-reporting.js | 0 | 0 | 0 | 0 | 1-69 + load-config.js | 0 | 0 | 0 | 0 | 1-134 parse-args.js | 0 | 0 | 0 | 0 | 1-225 report.js | 0 | 0 | 0 | 0 | 1-402 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 @@ -199,6 +200,7 @@ All files | 3.3 | 11.11 | 5.76 | 3.3 .c8.config.js | 0 | 0 | 0 | 0 | 1-11 .c8rc.js | 0 | 0 | 0 | 0 | 1-11 .nyc.config.js | 0 | 0 | 0 | 0 | 1-12 + blank.js | 0 | 0 | 0 | 0 | 1 c8.config.js | 0 | 0 | 0 | 0 | 1-12 nyc.config.js | 0 | 0 | 0 | 0 | 1-11 c8/test/fixtures/multidir1 | 0 | 0 | 0 | 0 | @@ -528,7 +530,7 @@ hey ---------------------------------------|---------|----------|---------|---------|------------------------ File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------------|---------|----------|---------|---------|------------------------ -All files | 3.3 | 11.11 | 5.76 | 3.3 | +All files | 3.03 | 10.71 | 5.55 | 3.03 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -538,7 +540,8 @@ All files | 3.3 | 11.11 | 5.76 | 3.3 prettify.js | 0 | 0 | 0 | 0 | 1-2 sorter.js | 0 | 0 | 0 | 0 | 1-196 c8/lib | 0 | 0 | 0 | 0 | - load-config.js | 0 | 0 | 0 | 0 | 1-51 + error-reporting.js | 0 | 0 | 0 | 0 | 1-69 + load-config.js | 0 | 0 | 0 | 0 | 1-134 parse-args.js | 0 | 0 | 0 | 0 | 1-225 report.js | 0 | 0 | 0 | 0 | 1-402 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 @@ -571,6 +574,7 @@ All files | 3.3 | 11.11 | 5.76 | 3.3 .c8.config.js | 0 | 0 | 0 | 0 | 1-11 .c8rc.js | 0 | 0 | 0 | 0 | 1-11 .nyc.config.js | 0 | 0 | 0 | 0 | 1-12 + blank.js | 0 | 0 | 0 | 0 | 1 c8.config.js | 0 | 0 | 0 | 0 | 1-12 nyc.config.js | 0 | 0 | 0 | 0 | 1-11 c8/test/fixtures/multidir1 | 0 | 0 | 0 | 0 | diff --git a/test/load-config.js b/test/load-config.js index 687587bc..ed2b7893 100644 --- a/test/load-config.js +++ b/test/load-config.js @@ -7,12 +7,11 @@ const { buildYargs } = require('../lib/parse-args') const chaiJestSnapshot = require('chai-jest-snapshot') const { spawnSync } = require('child_process') -require('chai').should() - const { - CONFIG_FILE_NAMES, - loadConfigFile -} = require('../lib/load-config') + UnsupportedFileTypeError, + ConfigParsingError +} = require('../lib/error-reporting') +const { CONFIG_FILE_NAMES, loadConfigFile } = require('../lib/load-config') const { testConfigFile, @@ -24,6 +23,18 @@ const c8Path = require.resolve('../bin/c8') const nodePath = process.execPath describe(__filename, () => { + const baseDI = { + // These specify default functions for all the DI calls. + // Each should throw an error if called so that the tests have to override the ones that are expected. + // Any test that actually fails with one of these errors was either not set up correctly or the code that's being tested isn't writen correctly. + readFile () { + throw new Error('Test not set up to handle calls to readFile!') + }, + readJs () { + throw new Error('Test not set up to handle calls to readJs!') + } + } + describe(loadConfigFile.name, () => { it('config directory should contain all variations of the config file naming convention', () => { let count = 0 @@ -79,39 +90,54 @@ describe(__filename, () => { } } - Object.keys(testData).forEach(key => { + Object.keys(testData).forEach((key) => { const { description, fileNameLineNumberMap } = testData[key] describe(description, function () { - testConfigFile(filePath, fileNameLineNumberMap, function (fileName, expectedLines) { - beforeEach(() => beforeTestReadingConfigFile(fileName)) - it(`should be able to resolve config file ${fileName} with --config flag`, () => { - const configFilePath = join(filePath, fileName) - const argv = buildYargs().parse(['node', 'c8', 'my-app', '--config', require.resolve(`./${configFilePath}`)]) - argv.lines.should.be.equal(expectedLines) - }) + testConfigFile( + filePath, + fileNameLineNumberMap, + function (fileName, expectedLines) { + beforeEach(() => beforeTestReadingConfigFile(fileName)) + it(`should be able to resolve config file ${fileName} with --config flag`, () => { + const configFilePath = join(filePath, fileName) + const argv = buildYargs().parse([ + 'node', + 'c8', + 'my-app', + '--config', + require.resolve(`./${configFilePath}`) + ]) + argv.lines.should.be.equal(expectedLines) + }) + + // skipping for the moment. Need another patch for this test to successfully run + it.skip(`should be able to resolve config file ${fileName} by detection`, function () { + // set the snapshot filename + chaiJestSnapshot.setTestName( + `should be able to resolve config file ${fileName} by detection` + ) + chaiJestSnapshot.setFilename( + './test/fixtures/config/snapshots/' + fileName + '.snap' + ) - // skipping for the moment. Need another patch for this test to successfully run - it.skip(`should be able to resolve config file ${fileName} by detection`, function () { - // set the snapshot filename - chaiJestSnapshot.setTestName(`should be able to resolve config file ${fileName} by detection`) - chaiJestSnapshot.setFilename('./test/fixtures/config/snapshots/' + fileName + '.snap') - - // Run V8 in the dir above - const { output } = spawnSync(nodePath, - [ - c8Path, - '--temp-directory=tmp/normal', - '--all', - '--src=./test/fixtures/tmp-config-test', + // Run V8 in the dir above + const { output } = spawnSync( nodePath, - require.resolve('./fixtures/tmp-config-test/normal.js') - ], - { cwd: './test/fixtures/tmp-config-test' } - ) - output.toString('utf8').should.matchSnapshot() - }) - after(afterTestReadingConfigFile) - }) + [ + c8Path, + '--temp-directory=tmp/normal', + '--all', + '--src=./test/fixtures/tmp-config-test', + nodePath, + require.resolve('./fixtures/tmp-config-test/normal.js') + ], + { cwd: './test/fixtures/tmp-config-test' } + ) + output.toString('utf8').should.matchSnapshot() + }) + after(afterTestReadingConfigFile) + } + ) }) }) }) @@ -119,10 +145,192 @@ describe(__filename, () => { it('throws an error message if an invalid configuration file name is passed', function () { const invalidConfig = './fixtures/config/.c8.config.py' - expect(() => loadConfigFile(invalidConfig)).to.throw( - Error, - `Unsupported file type .py while reading file ${invalidConfig}` + expect(() => loadConfigFile(invalidConfig, baseDI)).to.throw( + UnsupportedFileTypeError ) }) + + describe('handles file extensions in a case insensitive manner', () => { + for (const ext of ['.jSoN', '.JSON']) { + it(`reads a JSON file with extension ${ext}`, () => { + expect( + loadConfigFile(`test${ext}`, { + ...baseDI, + readFile: () => '{"statements":12}' + }) + ).to.deep.equal({ statements: 12 }) + }) + } + + for (const ext of ['.CJS', '.CJs', '.jS', '.JS']) { + it(`reads a Common JS file with extension ${ext}`, () => { + expect( + loadConfigFile(`test${ext}`, { + ...baseDI, + readJs: () => ({ + statements: 12 + }) + }) + ).to.deep.equal({ statements: 12 }) + }) + } + + for (const ext of ['.Yml', '.YAML']) { + it(`reads a YAML file with extension ${ext}`, () => { + expect( + loadConfigFile(`test${ext}`, { + ...baseDI, + readFile: () => 'statements: 12' + }) + ).to.deep.equal({ statements: 12 }) + }) + } + }) + + describe('.json', () => { + it('throws an error if the JSON file is empty', () => { + expect(() => + loadConfigFile('test.json', { + ...baseDI, + readFile: () => '' + }) + ).to.throw(ConfigParsingError, /Unexpected end of JSON input/) + }) + + it('throws an error if the JSON file is invalid JSON', () => { + expect(() => + loadConfigFile('test.json', { + ...baseDI, + readFile: () => 'not valid json' + }) + ).to.throw(ConfigParsingError, /Unexpected token /) + }) + + it('returns an empty config if the JSON file has an empty object', () => { + expect( + loadConfigFile('test.json', { + ...baseDI, + readFile: () => '{}' + }) + ).to.deep.equal({}) + }) + + it('throws the expected error if the file reader throws', () => { + const expectedError = new Error('the expected error') + + expect(() => + loadConfigFile('test.json', { + ...baseDI, + readFile: () => { + throw expectedError + } + }) + ).to.throw(Error, expectedError.message) + }) + }) + + describe('.cjs', () => { + it('throws an error if the CJS file is empty', () => { + const { readJs: _readJs, ...di } = baseDI + // Note: allowing require to access the file will only work on one test in the entire test suite due to NodeJS global module caching. + expect(() => + loadConfigFile('../test/fixtures/config/blank.cjs', di) + ).to.throw( + ConfigParsingError, + /does not export a c8 configuration object/ + ) + }) + + it('throws the error given by require if the CJS file is invalid Common JS', () => { + expect(() => + loadConfigFile('test.cjs', { + ...baseDI, + readJs: () => { + throw new Error('invalid CJS test') + } + }) + ).to.throw(Error, 'invalid CJS test') + }) + + it('returns an empty config if the CJS file exports an empty object', () => { + expect( + loadConfigFile('test.cjs', { + ...baseDI, + readJs: () => ({}) // require('./test.cjs') returns {} when the file contains module.exports = {} + }) + ).to.deep.equal({}) + }) + }) + + describe('.js', () => { + it('returns an empty config if the JS file is empty', () => { + const { readJs: _readJs, ...di } = baseDI + // Note: allowing require to access the file will only work on one test in the entire test suite due to NodeJS global module caching. + expect(() => + loadConfigFile('../test/fixtures/config/blank.js', di) + ).to.throw( + ConfigParsingError, + /does not export a c8 configuration object/ + ) + }) + + it('throws the error given by require if the JS file is invalid Common JS', () => { + expect(() => + loadConfigFile('test.js', { + ...baseDI, + readJs: () => { + throw new Error('invalid JS test') + } + }) + ).to.throw(Error, 'invalid JS test') + }) + + it('returns an empty config if the JS file exports an empty object', () => { + expect( + loadConfigFile('test.js', { + ...baseDI, + readJs: () => ({}) // require('./test.js') returns {} when the file contains module.exports = {} + }) + ).to.deep.equal({}) + }) + }) + + for (const ext of ['.yaml', '.yml']) { + describe(ext, () => { + it('throws an error if the YAML file is empty', () => { + expect(() => + loadConfigFile(`test${ext}`, { + ...baseDI, + readFile: () => '' + }) + ).to.throw(ConfigParsingError, /invalid configuration/i) + }) + + it('throws an error if the YAML file is invalid', () => { + expect(() => + loadConfigFile(`test${ext}`, { + ...baseDI, + readFile: () => '%not valid' + }) + ).to.throw( + ConfigParsingError, + /must contain a valid c8 configuration object/i + ) + }) + + it('throws the expected error if the file reader throws', () => { + const expectedError = new Error('the expected error') + + expect(() => + loadConfigFile(`test${ext}`, { + ...baseDI, + readFile: () => { + throw expectedError + } + }) + ).to.throw(Error, expectedError.message) + }) + }) + } }) }) diff --git a/test/parse-args.js b/test/parse-args.js index 5ea45d32..ae441ea6 100644 --- a/test/parse-args.js +++ b/test/parse-args.js @@ -68,6 +68,12 @@ describe('parse-args', () => { argv.tempDirectory.should.be.equal('./foo') argv.functions.should.be.equal(24) }) + it('should use config file specified in --config even if it is not a known file name', () => { + const argv = buildYargs().parse(['node', 'c8', '--config', require.resolve('./fixtures/config/nonstandard.json')]) + argv.lines.should.be.equal(420) + argv.tempDirectory.should.be.equal('./foo') + argv.functions.should.be.equal(24) + }) it('should have -c as an alias', () => { const argv = buildYargs().parse(['node', 'c8', '-c', require.resolve('./fixtures/config/.c8rc.json')]) argv.lines.should.be.equal(101) From dd09251a241630b10ec6ecead3ec7abb7e885886 Mon Sep 17 00:00:00 2001 From: "Trevor D. McKeown" Date: Sat, 3 Feb 2024 23:10:48 -0500 Subject: [PATCH 4/6] refactor: merge commit to adjust some white space --- test/error-reporting.js | 2 +- test/load-config.js | 503 ++++++++++++++++++++-------------------- 2 files changed, 253 insertions(+), 252 deletions(-) diff --git a/test/error-reporting.js b/test/error-reporting.js index 2dfc2f51..578130ee 100644 --- a/test/error-reporting.js +++ b/test/error-reporting.js @@ -7,7 +7,7 @@ const { ConfigParsingError } = require('../lib/error-reporting') -describe(__filename, () => { +describe('c8 error handling', () => { describe(ConfigParsingError.name, () => { it('is an Error subclass', () => { expect(new ConfigParsingError('', '')).to.be.instanceof(Error) diff --git a/test/load-config.js b/test/load-config.js index ed2b7893..7b2f04ad 100644 --- a/test/load-config.js +++ b/test/load-config.js @@ -7,6 +7,9 @@ const { buildYargs } = require('../lib/parse-args') const chaiJestSnapshot = require('chai-jest-snapshot') const { spawnSync } = require('child_process') +require('chai') + .should() + const { UnsupportedFileTypeError, ConfigParsingError @@ -22,7 +25,7 @@ const { const c8Path = require.resolve('../bin/c8') const nodePath = process.execPath -describe(__filename, () => { +describe(loadConfigFile.name, () => { const baseDI = { // These specify default functions for all the DI calls. // Each should throw an error if called so that the tests have to override the ones that are expected. @@ -35,302 +38,300 @@ describe(__filename, () => { } } - describe(loadConfigFile.name, () => { - it('config directory should contain all variations of the config file naming convention', () => { - let count = 0 - const fileMessages = [] - CONFIG_FILE_NAMES.forEach((file) => { - const fullPath = './test/fixtures/config/' + file - if (existsSync(fullPath)) { - count++ - } else { - fileMessages.push( - `Missing ${file} from ./test/fixtures/config directory` - ) - } - }) - - if (count === CONFIG_FILE_NAMES.length) { - assert.equal(count, CONFIG_FILE_NAMES.length) + it('config directory should contain all variations of the config file naming convention', () => { + let count = 0 + const fileMessages = [] + CONFIG_FILE_NAMES.forEach((file) => { + const fullPath = './test/fixtures/config/' + file + if (existsSync(fullPath)) { + count++ } else { - const msg = fileMessages.join(' \n ') - assert.equal(fileMessages.length, 0, msg) + fileMessages.push( + `Missing ${file} from ./test/fixtures/config directory` + ) } }) - describe('should be able to read config files with .json, .yml, .yaml, .js, .cjs extensions', () => { - const filePath = './fixtures/config/' - const testData = { - c8: { - description: 'c8 variations of config file', - fileNameLineNumberMap: { - '.c8rc.json': 101, - '.c8rc.yml': 69, - '.c8rc.yaml': 10, - 'c8.config.js': 47, - 'c8.config.cjs': 51, - '.c8rc.js': 22, - '.c8rc.cjs': 32, - '.c8.config.js': 47, - '.c8.config.cjs': 45 - } - }, - nyc: { - description: 'nyc variations of config file', - fileNameLineNumberMap: { - '.nycrc': 51, - '.nycrc.json': 96, - '.nycrc.yml': 99, - '.nycrc.yaml': 98, - 'nyc.config.js': 95, - 'nyc.config.cjs': 94, - '.nyc.config.js': 85, - '.nyc.config.cjs': 71 - } + if (count === CONFIG_FILE_NAMES.length) { + assert.equal(count, CONFIG_FILE_NAMES.length) + } else { + const msg = fileMessages.join(' \n ') + assert.equal(fileMessages.length, 0, msg) + } + }) + + describe('should be able to read config files with .json, .yml, .yaml, .js, .cjs extensions', () => { + const filePath = './fixtures/config/' + const testData = { + c8: { + description: 'c8 variations of config file', + fileNameLineNumberMap: { + '.c8rc.json': 101, + '.c8rc.yml': 69, + '.c8rc.yaml': 10, + 'c8.config.js': 47, + 'c8.config.cjs': 51, + '.c8rc.js': 22, + '.c8rc.cjs': 32, + '.c8.config.js': 47, + '.c8.config.cjs': 45 + } + }, + nyc: { + description: 'nyc variations of config file', + fileNameLineNumberMap: { + '.nycrc': 51, + '.nycrc.json': 96, + '.nycrc.yml': 99, + '.nycrc.yaml': 98, + 'nyc.config.js': 95, + 'nyc.config.cjs': 94, + '.nyc.config.js': 85, + '.nyc.config.cjs': 71 } } + } - Object.keys(testData).forEach((key) => { - const { description, fileNameLineNumberMap } = testData[key] - describe(description, function () { - testConfigFile( - filePath, - fileNameLineNumberMap, - function (fileName, expectedLines) { - beforeEach(() => beforeTestReadingConfigFile(fileName)) - it(`should be able to resolve config file ${fileName} with --config flag`, () => { - const configFilePath = join(filePath, fileName) - const argv = buildYargs().parse([ - 'node', - 'c8', - 'my-app', - '--config', - require.resolve(`./${configFilePath}`) - ]) - argv.lines.should.be.equal(expectedLines) - }) - - // skipping for the moment. Need another patch for this test to successfully run - it.skip(`should be able to resolve config file ${fileName} by detection`, function () { - // set the snapshot filename - chaiJestSnapshot.setTestName( - `should be able to resolve config file ${fileName} by detection` - ) - chaiJestSnapshot.setFilename( - './test/fixtures/config/snapshots/' + fileName + '.snap' - ) - - // Run V8 in the dir above - const { output } = spawnSync( - nodePath, - [ - c8Path, - '--temp-directory=tmp/normal', - '--all', - '--src=./test/fixtures/tmp-config-test', - nodePath, - require.resolve('./fixtures/tmp-config-test/normal.js') - ], - { cwd: './test/fixtures/tmp-config-test' } - ) - output.toString('utf8').should.matchSnapshot() - }) - after(afterTestReadingConfigFile) - } - ) - }) - }) - }) - - it('throws an error message if an invalid configuration file name is passed', function () { - const invalidConfig = './fixtures/config/.c8.config.py' - - expect(() => loadConfigFile(invalidConfig, baseDI)).to.throw( - UnsupportedFileTypeError - ) - }) - - describe('handles file extensions in a case insensitive manner', () => { - for (const ext of ['.jSoN', '.JSON']) { - it(`reads a JSON file with extension ${ext}`, () => { - expect( - loadConfigFile(`test${ext}`, { - ...baseDI, - readFile: () => '{"statements":12}' + Object.keys(testData).forEach((key) => { + const { description, fileNameLineNumberMap } = testData[key] + describe(description, function () { + testConfigFile( + filePath, + fileNameLineNumberMap, + function (fileName, expectedLines) { + beforeEach(() => beforeTestReadingConfigFile(fileName)) + it(`should be able to resolve config file ${fileName} with --config flag`, () => { + const configFilePath = join(filePath, fileName) + const argv = buildYargs().parse([ + 'node', + 'c8', + 'my-app', + '--config', + require.resolve(`./${configFilePath}`) + ]) + argv.lines.should.be.equal(expectedLines) }) - ).to.deep.equal({ statements: 12 }) - }) - } - for (const ext of ['.CJS', '.CJs', '.jS', '.JS']) { - it(`reads a Common JS file with extension ${ext}`, () => { - expect( - loadConfigFile(`test${ext}`, { - ...baseDI, - readJs: () => ({ - statements: 12 - }) - }) - ).to.deep.equal({ statements: 12 }) - }) - } + // skipping for the moment. Need another patch for this test to successfully run + it.skip(`should be able to resolve config file ${fileName} by detection`, function () { + // set the snapshot filename + chaiJestSnapshot.setTestName( + `should be able to resolve config file ${fileName} by detection` + ) + chaiJestSnapshot.setFilename( + './test/fixtures/config/snapshots/' + fileName + '.snap' + ) - for (const ext of ['.Yml', '.YAML']) { - it(`reads a YAML file with extension ${ext}`, () => { - expect( - loadConfigFile(`test${ext}`, { - ...baseDI, - readFile: () => 'statements: 12' + // Run V8 in the dir above + const { output } = spawnSync( + nodePath, + [ + c8Path, + '--temp-directory=tmp/normal', + '--all', + '--src=./test/fixtures/tmp-config-test', + nodePath, + require.resolve('./fixtures/tmp-config-test/normal.js') + ], + { cwd: './test/fixtures/tmp-config-test' } + ) + output.toString('utf8').should.matchSnapshot() }) - ).to.deep.equal({ statements: 12 }) - }) - } + after(afterTestReadingConfigFile) + } + ) + }) }) + }) - describe('.json', () => { - it('throws an error if the JSON file is empty', () => { - expect(() => - loadConfigFile('test.json', { + it('throws an error message if an invalid configuration file name is passed', function () { + const invalidConfig = './fixtures/config/.c8.config.py' + + expect(() => loadConfigFile(invalidConfig, baseDI)).to.throw( + UnsupportedFileTypeError + ) + }) + + describe('handles file extensions in a case insensitive manner', () => { + for (const ext of ['.jSoN', '.JSON']) { + it(`reads a JSON file with extension ${ext}`, () => { + expect( + loadConfigFile(`test${ext}`, { ...baseDI, - readFile: () => '' + readFile: () => '{"statements":12}' }) - ).to.throw(ConfigParsingError, /Unexpected end of JSON input/) + ).to.deep.equal({ statements: 12 }) }) + } - it('throws an error if the JSON file is invalid JSON', () => { - expect(() => - loadConfigFile('test.json', { + for (const ext of ['.CJS', '.CJs', '.jS', '.JS']) { + it(`reads a Common JS file with extension ${ext}`, () => { + expect( + loadConfigFile(`test${ext}`, { ...baseDI, - readFile: () => 'not valid json' + readJs: () => ({ + statements: 12 + }) }) - ).to.throw(ConfigParsingError, /Unexpected token /) + ).to.deep.equal({ statements: 12 }) }) + } - it('returns an empty config if the JSON file has an empty object', () => { + for (const ext of ['.Yml', '.YAML']) { + it(`reads a YAML file with extension ${ext}`, () => { expect( - loadConfigFile('test.json', { + loadConfigFile(`test${ext}`, { ...baseDI, - readFile: () => '{}' + readFile: () => 'statements: 12' }) - ).to.deep.equal({}) + ).to.deep.equal({ statements: 12 }) }) + } + }) - it('throws the expected error if the file reader throws', () => { - const expectedError = new Error('the expected error') + describe('.json', () => { + it('throws an error if the JSON file is empty', () => { + expect(() => + loadConfigFile('test.json', { + ...baseDI, + readFile: () => '' + }) + ).to.throw(ConfigParsingError, /Unexpected end of JSON input/) + }) - expect(() => - loadConfigFile('test.json', { - ...baseDI, - readFile: () => { - throw expectedError - } - }) - ).to.throw(Error, expectedError.message) - }) + it('throws an error if the JSON file is invalid JSON', () => { + expect(() => + loadConfigFile('test.json', { + ...baseDI, + readFile: () => 'not valid json' + }) + ).to.throw(ConfigParsingError, /Unexpected token /) }) - describe('.cjs', () => { - it('throws an error if the CJS file is empty', () => { - const { readJs: _readJs, ...di } = baseDI - // Note: allowing require to access the file will only work on one test in the entire test suite due to NodeJS global module caching. - expect(() => - loadConfigFile('../test/fixtures/config/blank.cjs', di) - ).to.throw( - ConfigParsingError, - /does not export a c8 configuration object/ - ) - }) + it('returns an empty config if the JSON file has an empty object', () => { + expect( + loadConfigFile('test.json', { + ...baseDI, + readFile: () => '{}' + }) + ).to.deep.equal({}) + }) + + it('throws the expected error if the file reader throws', () => { + const expectedError = new Error('the expected error') + + expect(() => + loadConfigFile('test.json', { + ...baseDI, + readFile: () => { + throw expectedError + } + }) + ).to.throw(Error, expectedError.message) + }) + }) + + describe('.cjs', () => { + it('throws an error if the CJS file is empty', () => { + const { readJs: _readJs, ...di } = baseDI + // Note: allowing require to access the file will only work on one test in the entire test suite due to NodeJS global module caching. + expect(() => + loadConfigFile('../test/fixtures/config/blank.cjs', di) + ).to.throw( + ConfigParsingError, + /does not export a c8 configuration object/ + ) + }) + + it('throws the error given by require if the CJS file is invalid Common JS', () => { + expect(() => + loadConfigFile('test.cjs', { + ...baseDI, + readJs: () => { + throw new Error('invalid CJS test') + } + }) + ).to.throw(Error, 'invalid CJS test') + }) - it('throws the error given by require if the CJS file is invalid Common JS', () => { + it('returns an empty config if the CJS file exports an empty object', () => { + expect( + loadConfigFile('test.cjs', { + ...baseDI, + readJs: () => ({}) // require('./test.cjs') returns {} when the file contains module.exports = {} + }) + ).to.deep.equal({}) + }) + }) + + describe('.js', () => { + it('returns an empty config if the JS file is empty', () => { + const { readJs: _readJs, ...di } = baseDI + // Note: allowing require to access the file will only work on one test in the entire test suite due to NodeJS global module caching. + expect(() => + loadConfigFile('../test/fixtures/config/blank.js', di) + ).to.throw( + ConfigParsingError, + /does not export a c8 configuration object/ + ) + }) + + it('throws the error given by require if the JS file is invalid Common JS', () => { + expect(() => + loadConfigFile('test.js', { + ...baseDI, + readJs: () => { + throw new Error('invalid JS test') + } + }) + ).to.throw(Error, 'invalid JS test') + }) + + it('returns an empty config if the JS file exports an empty object', () => { + expect( + loadConfigFile('test.js', { + ...baseDI, + readJs: () => ({}) // require('./test.js') returns {} when the file contains module.exports = {} + }) + ).to.deep.equal({}) + }) + }) + + for (const ext of ['.yaml', '.yml']) { + describe(ext, () => { + it('throws an error if the YAML file is empty', () => { expect(() => - loadConfigFile('test.cjs', { + loadConfigFile(`test${ext}`, { ...baseDI, - readJs: () => { - throw new Error('invalid CJS test') - } + readFile: () => '' }) - ).to.throw(Error, 'invalid CJS test') + ).to.throw(ConfigParsingError, /invalid configuration/i) }) - it('returns an empty config if the CJS file exports an empty object', () => { - expect( - loadConfigFile('test.cjs', { + it('throws an error if the YAML file is invalid', () => { + expect(() => + loadConfigFile(`test${ext}`, { ...baseDI, - readJs: () => ({}) // require('./test.cjs') returns {} when the file contains module.exports = {} + readFile: () => '%not valid' }) - ).to.deep.equal({}) - }) - }) - - describe('.js', () => { - it('returns an empty config if the JS file is empty', () => { - const { readJs: _readJs, ...di } = baseDI - // Note: allowing require to access the file will only work on one test in the entire test suite due to NodeJS global module caching. - expect(() => - loadConfigFile('../test/fixtures/config/blank.js', di) ).to.throw( ConfigParsingError, - /does not export a c8 configuration object/ + /must contain a valid c8 configuration object/i ) }) - it('throws the error given by require if the JS file is invalid Common JS', () => { + it('throws the expected error if the file reader throws', () => { + const expectedError = new Error('the expected error') + expect(() => - loadConfigFile('test.js', { + loadConfigFile(`test${ext}`, { ...baseDI, - readJs: () => { - throw new Error('invalid JS test') + readFile: () => { + throw expectedError } }) - ).to.throw(Error, 'invalid JS test') - }) - - it('returns an empty config if the JS file exports an empty object', () => { - expect( - loadConfigFile('test.js', { - ...baseDI, - readJs: () => ({}) // require('./test.js') returns {} when the file contains module.exports = {} - }) - ).to.deep.equal({}) + ).to.throw(Error, expectedError.message) }) }) - - for (const ext of ['.yaml', '.yml']) { - describe(ext, () => { - it('throws an error if the YAML file is empty', () => { - expect(() => - loadConfigFile(`test${ext}`, { - ...baseDI, - readFile: () => '' - }) - ).to.throw(ConfigParsingError, /invalid configuration/i) - }) - - it('throws an error if the YAML file is invalid', () => { - expect(() => - loadConfigFile(`test${ext}`, { - ...baseDI, - readFile: () => '%not valid' - }) - ).to.throw( - ConfigParsingError, - /must contain a valid c8 configuration object/i - ) - }) - - it('throws the expected error if the file reader throws', () => { - const expectedError = new Error('the expected error') - - expect(() => - loadConfigFile(`test${ext}`, { - ...baseDI, - readFile: () => { - throw expectedError - } - }) - ).to.throw(Error, expectedError.message) - }) - }) - } - }) + } }) From 6aa0eb285ffcb4b39f21baf62afe4151a783cb87 Mon Sep 17 00:00:00 2001 From: "Trevor D. McKeown" Date: Sun, 4 Feb 2024 00:36:54 -0500 Subject: [PATCH 5/6] refactor: added spawn and snapshot helpers refactor: var names and assignment in helpers refactor: removed some lines in helpers refactor: test/load-config.js to use only expect style syntax fix: removing path from dispaying on unit tests refactor: test case for config file detection refactor: removed dep on chai should fix: added a describe block to the error handling tests fix: unit test that was failing on ci due to difference in plaform json packages --- test/load-config-helper.js | 65 ++++++++++++--- test/load-config.js | 167 ++++++++++++++----------------------- 2 files changed, 116 insertions(+), 116 deletions(-) diff --git a/test/load-config-helper.js b/test/load-config-helper.js index 93b39d1a..e38495cc 100644 --- a/test/load-config-helper.js +++ b/test/load-config-helper.js @@ -1,17 +1,20 @@ +const { spawnSync } = require('child_process') const { mkdirSync, copyFileSync, rmSync, existsSync } = require('fs') const { join } = require('path') const testPath = './test/fixtures/tmp-config-test' -const testConfigFile = function (filePath, fileNameLineNumberMap, callback) { - Object.keys(fileNameLineNumberMap).forEach((fileName) => { - const expectedLines = fileNameLineNumberMap[fileName] - callback(fileName, expectedLines) +const iterateExtTestCase = function (testDataObj, callback) { + Object.keys(testDataObj).forEach((key) => { + Object.keys(testDataObj[key]).forEach((fileName) => { + const expectedLines = testDataObj[key][fileName] + callback(fileName, expectedLines) + }) }) } -const beforeTestReadingConfigFile = (configFileName) => { - afterTestReadingConfigFile() +const setUpConfigTest = (configFileName) => { + deleteConfigTest() // make the directory tmp-config-test mkdirSync(testPath) @@ -21,14 +24,56 @@ const beforeTestReadingConfigFile = (configFileName) => { copyFileSync('./test/fixtures/config/' + configFileName, join(testPath, configFileName)) } -const afterTestReadingConfigFile = () => { +const deleteConfigTest = () => { if (existsSync(testPath)) { rmSync(testPath, { recursive: true, force: true }) } } +const runc8 = (c8args, env = {}) => { + const c8Path = require.resolve('../bin/c8') + const nodePath = process.execPath + const c8argsv = [c8Path, ...c8args] + + const { output } = spawnSync(nodePath, c8argsv, env) + + return output +} + +const setUpSnapShot = (snapShotObj, testName, fileName) => { + snapShotObj.setTestName(testName) + snapShotObj.setFilename(fileName) +} + +const extLoadingTestData = Object.freeze({ + c8: { + '.c8rc.json': 101, + '.c8rc.yml': 69, + '.c8rc.yaml': 10, + 'c8.config.js': 47, + 'c8.config.cjs': 51, + '.c8rc.js': 22, + '.c8rc.cjs': 32, + '.c8.config.js': 47, + '.c8.config.cjs': 45 + }, + nyc: { + '.nycrc': 51, + '.nycrc.json': 96, + '.nycrc.yml': 99, + '.nycrc.yaml': 98, + 'nyc.config.js': 95, + 'nyc.config.cjs': 94, + '.nyc.config.js': 85, + '.nyc.config.cjs': 71 + } +}) + module.exports = { - testConfigFile: testConfigFile, - beforeTestReadingConfigFile: beforeTestReadingConfigFile, - afterTestReadingConfigFile: afterTestReadingConfigFile + extLoadingTestData, + runc8, + setUpSnapShot, + iterateExtTestCase, + setUpConfigTest, + deleteConfigTest } diff --git a/test/load-config.js b/test/load-config.js index 7b2f04ad..4a8473dd 100644 --- a/test/load-config.js +++ b/test/load-config.js @@ -1,14 +1,13 @@ /* global describe, it, beforeEach, after */ -const { assert, expect } = require('chai') +const { expect } = require('chai') const { existsSync } = require('fs') const { join } = require('path') const { buildYargs } = require('../lib/parse-args') const chaiJestSnapshot = require('chai-jest-snapshot') -const { spawnSync } = require('child_process') require('chai') - .should() + .use(chaiJestSnapshot) const { UnsupportedFileTypeError, @@ -17,14 +16,14 @@ const { const { CONFIG_FILE_NAMES, loadConfigFile } = require('../lib/load-config') const { - testConfigFile, - beforeTestReadingConfigFile, - afterTestReadingConfigFile + extLoadingTestData, + runc8, + setUpSnapShot, + iterateExtTestCase, + setUpConfigTest, + deleteConfigTest } = require('./load-config-helper.js') -const c8Path = require.resolve('../bin/c8') -const nodePath = process.execPath - describe(loadConfigFile.name, () => { const baseDI = { // These specify default functions for all the DI calls. @@ -39,107 +38,56 @@ describe(loadConfigFile.name, () => { } it('config directory should contain all variations of the config file naming convention', () => { - let count = 0 - const fileMessages = [] - CONFIG_FILE_NAMES.forEach((file) => { - const fullPath = './test/fixtures/config/' + file - if (existsSync(fullPath)) { - count++ - } else { - fileMessages.push( - `Missing ${file} from ./test/fixtures/config directory` - ) - } - }) + const pathPrefix = './test/fixtures/config/' + const accountedFor = CONFIG_FILE_NAMES + .map((configFile) => existsSync(pathPrefix + configFile)) + .reduce((prev, curr) => prev && curr) - if (count === CONFIG_FILE_NAMES.length) { - assert.equal(count, CONFIG_FILE_NAMES.length) - } else { - const msg = fileMessages.join(' \n ') - assert.equal(fileMessages.length, 0, msg) - } + expect(accountedFor).to.eql(true) }) - describe('should be able to read config files with .json, .yml, .yaml, .js, .cjs extensions', () => { - const filePath = './fixtures/config/' - const testData = { - c8: { - description: 'c8 variations of config file', - fileNameLineNumberMap: { - '.c8rc.json': 101, - '.c8rc.yml': 69, - '.c8rc.yaml': 10, - 'c8.config.js': 47, - 'c8.config.cjs': 51, - '.c8rc.js': 22, - '.c8rc.cjs': 32, - '.c8.config.js': 47, - '.c8.config.cjs': 45 - } - }, - nyc: { - description: 'nyc variations of config file', - fileNameLineNumberMap: { - '.nycrc': 51, - '.nycrc.json': 96, - '.nycrc.yml': 99, - '.nycrc.yaml': 98, - 'nyc.config.js': 95, - 'nyc.config.cjs': 94, - '.nyc.config.js': 85, - '.nyc.config.cjs': 71 - } - } - } + describe('read config files with .json, .yml, .yaml, .js, .cjs extensions', () => { + iterateExtTestCase(extLoadingTestData, function (fileName, expectedLines) { + describe(fileName, function () { + beforeEach(() => setUpConfigTest(fileName)) + + it('passed with -config flag', () => { + const filePath = './fixtures/config/' + const configFilePath = join(filePath, fileName) + const args = Object.freeze(['node', + 'c8', + 'my-app', + '--config', + require.resolve(`./${configFilePath}`) + ]) + const argv = buildYargs().parse(args) + expect(argv.lines).to.equal(expectedLines) + }) - Object.keys(testData).forEach((key) => { - const { description, fileNameLineNumberMap } = testData[key] - describe(description, function () { - testConfigFile( - filePath, - fileNameLineNumberMap, - function (fileName, expectedLines) { - beforeEach(() => beforeTestReadingConfigFile(fileName)) - it(`should be able to resolve config file ${fileName} with --config flag`, () => { - const configFilePath = join(filePath, fileName) - const argv = buildYargs().parse([ - 'node', - 'c8', - 'my-app', - '--config', - require.resolve(`./${configFilePath}`) - ]) - argv.lines.should.be.equal(expectedLines) - }) + // skipping for the moment. Need another patch for this test to thoroughly run + it.skip('by detection', function () { + // set the snapshot filename + const snapTestName = `should be able to resolve config file ${fileName} by detection` + const snapFileName = './test/fixtures/config/snapshots/' + fileName + '.snap' + + setUpSnapShot(chaiJestSnapshot, snapTestName, snapFileName) + + const nodePath = process.execPath + const env = Object.freeze({ cwd: './test/fixtures/tmp-config-test' }) + const args = Object.freeze([ + '--temp-directory=tmp/normal', + '--all', + '--src=./test/fixtures/tmp-config-test', + nodePath, + require.resolve('./fixtures/tmp-config-test/normal.js') + ]) + + // Run V8 in the dir above + const output = runc8(args, env) + expect(output.toString('utf8')).to.matchSnapshot() + }) - // skipping for the moment. Need another patch for this test to successfully run - it.skip(`should be able to resolve config file ${fileName} by detection`, function () { - // set the snapshot filename - chaiJestSnapshot.setTestName( - `should be able to resolve config file ${fileName} by detection` - ) - chaiJestSnapshot.setFilename( - './test/fixtures/config/snapshots/' + fileName + '.snap' - ) - - // Run V8 in the dir above - const { output } = spawnSync( - nodePath, - [ - c8Path, - '--temp-directory=tmp/normal', - '--all', - '--src=./test/fixtures/tmp-config-test', - nodePath, - require.resolve('./fixtures/tmp-config-test/normal.js') - ], - { cwd: './test/fixtures/tmp-config-test' } - ) - output.toString('utf8').should.matchSnapshot() - }) - after(afterTestReadingConfigFile) - } - ) + after(deleteConfigTest) }) }) }) @@ -200,12 +148,19 @@ describe(loadConfigFile.name, () => { }) it('throws an error if the JSON file is invalid JSON', () => { + const errorPattern = 'Error loading configuration from file ".+": ' + + 'must contain a valid c8 configuration object. Original error: .*' + const errorRegEx = new RegExp(errorPattern, 'g') + expect(() => loadConfigFile('test.json', { ...baseDI, readFile: () => 'not valid json' }) - ).to.throw(ConfigParsingError, /Unexpected token /) + ).to.throw( + ConfigParsingError, + errorRegEx + ) }) it('returns an empty config if the JSON file has an empty object', () => { From 434650f1690045914f9d9c6cadc6df6483474011 Mon Sep 17 00:00:00 2001 From: Trevor D McKeown Date: Thu, 1 Feb 2024 15:11:35 -0500 Subject: [PATCH 6/6] chore: circleci for matrix builds across win, mac, and linux for node v14 - v20 --- .circleci/config.yml | 124 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 .circleci/config.yml diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 00000000..ee52be24 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,124 @@ +--- +# +# This build use extensive use of parameters +# https://circleci.com/docs/reusing-config/#using-the-parameters-declaration +# +# And Conditional Steps +# https://circleci.com/docs/reusing-config/#defining-conditional-steps + +version: 2.1 + +orbs: + node: circleci/node@5.2.0 + +commands: + project-setup: + parameters: + working-dir: + type: string + node-version: + type: string + windows: + type: boolean + default: false + steps: + - checkout: + path: << parameters.working-dir >> + - when: + condition: + equal: [ true, << parameters.windows >> ] + steps: + - run: + name: nvm-install + command: choco install nvm -y + - run: + name: node-install + command: | + Start-Process powershell -verb runAs -Args "-start GeneralProfile" + nvm install << parameters.node-version >> + nvm use << parameters.node-version >> + - run: + name: npm-install + command: npm ci + - when: + condition: + equal: [ false, << parameters.windows >> ] + steps: + - node/install: + node-version: << parameters.node-version >> + install-yarn: false + - node/install-packages: + check-cache: always + pkg-manager: npm + with-cache: false + + lint-test: + steps: + - run: + command: npm run posttest + + unit-test: + steps: + - run: + command: npm run test + + +executors: + linux: # a Linux VM running Ubuntu 20.04 + docker: + - image: cimg/base:2024.01 + working_directory: /home/circleci/project/c8 + macos: # macos executor running Xcode + macos: + xcode: 14.2.0 + working_directory: /Users/distiller/project/c8 + win: + machine: + image: 'windows-server-2019-vs2019:2023.10.1' + resource_class: windows.medium + shell: powershell.exe -ExecutionPolicy Bypass + working_directory: C:\Users\circleci\project\c8 + +jobs: + # Refactor to a command + test: + parameters: + os: + type: string + node-version: + type: string + executor: << parameters.os >> + steps: + - when: + condition: + equal: [ linux, << parameters.os >> ] + steps: + - project-setup: + node-version: << parameters.node-version >> + working-dir: /home/circleci/project/c8 + - when: + condition: + equal: [ win , << parameters.os >> ] + steps: + - project-setup: + node-version: << parameters.node-version >> + working-dir: C:\Users\circleci\project\c8 + windows: true + - when: + condition: + equal: [ macos, << parameters.os >> ] + steps: + - project-setup: + node-version: << parameters.node-version >> + working-dir: /Users/distiller/project/c8 + - lint-test + - unit-test + +workflows: + matrix-test: + jobs: + - test: + matrix: + parameters: + os: [win, linux, macos] + node-version: ["14.21.3", "16.20.2", "18.19.0", "20.11.0"] \ No newline at end of file