Skip to content

Commit

Permalink
feat(experimental): Support using --all with node.js ESM (#1320)
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
coreyfarrell committed May 24, 2020
1 parent 086fd20 commit 992359a
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 11 deletions.
38 changes: 27 additions & 11 deletions index.js
Expand Up @@ -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')

Expand Down Expand Up @@ -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) {
Expand All @@ -187,7 +210,7 @@ class NYC {
all: true
}
}
})
}
this.fakeRequire = false

this.writeCoverageFile()
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions tap-snapshots/test-nyc-integration.js-TAP.test.js
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/all-type-module/extra.mjs
@@ -0,0 +1,3 @@
export default function () {
return 'es module';
}
3 changes: 3 additions & 0 deletions test/fixtures/all-type-module/index.js
@@ -0,0 +1,3 @@
export default function () {
return 'es module';
}
10 changes: 10 additions & 0 deletions 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
}
}
5 changes: 5 additions & 0 deletions 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);
10 changes: 10 additions & 0 deletions test/nyc-integration.js
Expand Up @@ -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',
Expand Down Expand Up @@ -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: [
Expand Down

0 comments on commit 992359a

Please sign in to comment.