From 992359a1e2108906e28324058008f28aa1052dc1 Mon Sep 17 00:00:00 2001 From: Corey Farrell Date: Sun, 24 May 2020 07:10:42 -0400 Subject: [PATCH] feat(experimental): Support using `--all` with node.js ESM (#1320) This allows `--all` to collect initial coverage from fils which node.js recognizes as ES modules (`.mjs` files and conditionally `.js` files). Previously this would cause node.js to produce an `ERR_REQUIRE_ESM` error. This does not enable collection of coverage during actual tests, for that you must use the experimental `@istanbuljs/esm-loader-hook`. --- index.js | 38 +++++++++++++------ package-lock.json | 5 +++ package.json | 1 + .../test-nyc-integration.js-TAP.test.js | 12 ++++++ test/fixtures/all-type-module/extra.mjs | 3 ++ test/fixtures/all-type-module/index.js | 3 ++ test/fixtures/all-type-module/package.json | 10 +++++ test/fixtures/all-type-module/script.cjs | 5 +++ test/nyc-integration.js | 10 +++++ 9 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/all-type-module/extra.mjs create mode 100644 test/fixtures/all-type-module/index.js create mode 100644 test/fixtures/all-type-module/package.json create mode 100755 test/fixtures/all-type-module/script.cjs diff --git a/index.js b/index.js index e2ae1c24c..3164b1d0b 100755 --- a/index.js +++ b/index.js @@ -20,6 +20,7 @@ const rimraf = promisify(require('rimraf')) const SourceMaps = require('./lib/source-maps') const TestExclude = require('test-exclude') const pMap = require('p-map') +const getPackageType = require('get-package-type') const debugLog = debuglog('nyc') @@ -170,14 +171,36 @@ class NYC { return source } + _getSourceMap (code, filename, hash) { + const sourceMap = {} + if (this._sourceMap) { + sourceMap.sourceMap = this.sourceMaps.extract(code, filename) + sourceMap.registerMap = () => this.sourceMaps.registerMap(filename, hash, sourceMap.sourceMap) + } else { + sourceMap.registerMap = () => {} + } + + return sourceMap + } + async addAllFiles () { this._loadAdditionalModules() this.fakeRequire = true const files = await this.exclude.glob(this.cwd) - files.forEach(relFile => { + for (const relFile of files) { const filename = path.resolve(this.cwd, relFile) - this.addFile(filename) + const ext = path.extname(filename) + if (ext === '.mjs' || (ext === '.js' && await getPackageType(filename) === 'module')) { + const source = await fs.readFile(filename, 'utf8') + this.instrumenter().instrumentSync( + source, + filename, + this._getSourceMap(source, filename) + ) + } else { + this.addFile(filename) + } const coverage = coverageFinder() const lastCoverage = this.instrumenter().lastFileCoverage() if (lastCoverage) { @@ -187,7 +210,7 @@ class NYC { all: true } } - }) + } this.fakeRequire = false this.writeCoverageFile() @@ -276,14 +299,7 @@ class NYC { return (code, metadata, hash) => { const filename = metadata.filename - const sourceMap = {} - - if (this._sourceMap) { - sourceMap.sourceMap = this.sourceMaps.extract(code, filename) - sourceMap.registerMap = () => this.sourceMaps.registerMap(filename, hash, sourceMap.sourceMap) - } else { - sourceMap.registerMap = () => {} - } + const sourceMap = this._getSourceMap(code, filename, hash) try { instrumented = instrumenter.instrumentSync(code, filename, sourceMap) diff --git a/package-lock.json b/package-lock.json index 822b5eab4..e24af98bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2248,6 +2248,11 @@ "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", "integrity": "sha512-DyFP3BM/3YHTQOCUL/w0OZHR0lpKeGrxotcHWcqNEdnltqFwXVfhEBQ94eIo34AfQpo0rGki4cyIiftY06h2Fg==" }, + "get-package-type": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/get-package-type/-/get-package-type-0.1.0.tgz", + "integrity": "sha512-pjzuKtY64GYfWizNAJ0fr9VqttZkNiK2iS430LtIHzjBEr6bX8Am2zm4sW4Ro5wjWW5cAlRL1qAMTcXbjNAO2Q==" + }, "get-pkg-repo": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/get-pkg-repo/-/get-pkg-repo-1.4.0.tgz", diff --git a/package.json b/package.json index c1816a51f..8871d4cbd 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "find-cache-dir": "^3.2.0", "find-up": "^4.1.0", "foreground-child": "^2.0.0", + "get-package-type": "^0.1.0", "glob": "^7.1.6", "istanbul-lib-coverage": "^3.0.0", "istanbul-lib-hook": "^3.0.0", diff --git a/tap-snapshots/test-nyc-integration.js-TAP.test.js b/tap-snapshots/test-nyc-integration.js-TAP.test.js index fed0650a2..f6563687b 100644 --- a/tap-snapshots/test-nyc-integration.js-TAP.test.js +++ b/tap-snapshots/test-nyc-integration.js-TAP.test.js @@ -5,6 +5,18 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' +exports[`test/nyc-integration.js TAP --all does not fail on ERR_REQUIRE_ESM > stdout 1`] = ` +------------|---------|----------|---------|---------|------------------- +File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s +------------|---------|----------|---------|---------|------------------- +All files | 33.33 | 100 | 0 | 33.33 | + extra.mjs | 0 | 100 | 0 | 0 | 2 + index.js | 0 | 100 | 0 | 0 | 2 + script.cjs | 100 | 100 | 100 | 100 | +------------|---------|----------|---------|---------|------------------- + +` + exports[`test/nyc-integration.js TAP --all includes files with both .map files and inline source-maps > stdout 1`] = ` ----------|---------|----------|---------|---------|------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s diff --git a/test/fixtures/all-type-module/extra.mjs b/test/fixtures/all-type-module/extra.mjs new file mode 100644 index 000000000..7c8308e6d --- /dev/null +++ b/test/fixtures/all-type-module/extra.mjs @@ -0,0 +1,3 @@ +export default function () { + return 'es module'; +} diff --git a/test/fixtures/all-type-module/index.js b/test/fixtures/all-type-module/index.js new file mode 100644 index 000000000..7c8308e6d --- /dev/null +++ b/test/fixtures/all-type-module/index.js @@ -0,0 +1,3 @@ +export default function () { + return 'es module'; +} diff --git a/test/fixtures/all-type-module/package.json b/test/fixtures/all-type-module/package.json new file mode 100644 index 000000000..5a3060c41 --- /dev/null +++ b/test/fixtures/all-type-module/package.json @@ -0,0 +1,10 @@ +{ + "name": "all-type-module", + "version": "0.1.0", + "description": "", + "main": "index.js", + "type": "module", + "nyc": { + "all": true + } +} diff --git a/test/fixtures/all-type-module/script.cjs b/test/fixtures/all-type-module/script.cjs new file mode 100755 index 000000000..55501a20c --- /dev/null +++ b/test/fixtures/all-type-module/script.cjs @@ -0,0 +1,5 @@ +#!/usr/bin/env node +'use strict'; + +// Not doing anything, we just want something to run with `nyc --all` +process.exit(0); diff --git a/test/nyc-integration.js b/test/nyc-integration.js index 35ee19293..4d8e5fdd7 100644 --- a/test/nyc-integration.js +++ b/test/nyc-integration.js @@ -15,6 +15,7 @@ const nycConfigJS = path.resolve(fixturesCLI, 'nyc-config-js') const nycrcDir = path.resolve(fixturesCLI, 'nycrc') const fixturesSourceMaps = path.resolve(fixturesCLI, '../source-maps') const fixturesENM = path.resolve(fixturesCLI, '../exclude-node-modules') +const fixturesAllTypeModule = path.resolve(fixturesCLI, '../all-type-module') const executeNodeModulesArgs = [ '--all=true', @@ -436,6 +437,15 @@ t.test('--all uses source-maps to exclude original sources from reports', t => t cwd: fixturesSourceMaps })) +t.test('--all does not fail on ERR_REQUIRE_ESM', t => testSuccess(t, { + args: [ + '--all', + process.execPath, + 'script.cjs' + ], + cwd: fixturesAllTypeModule +})) + t.test('caches source-maps from .map files', async t => { await testSuccess(t, { args: [