From 1aab44cd25068f0501ca91c1c8f860ac1929acba Mon Sep 17 00:00:00 2001 From: Ferdinand Prantl Date: Sun, 2 Apr 2023 16:53:19 +0200 Subject: [PATCH] fix: Fix usage of excludeAfterRemap not to set the coverage always to 100 Do not use the `excludePath` callback. Remove the excluded sources at the end, after all coverage parts have been merged. This might be a problem in `v8-to-istanbul`, because `istanbul-lib-coverage` doesn't offer a method to filter the coverage data at the end. Attempts to fix #462 --- lib/report.js | 29 +++++++++++++++++++++++------ test/integration.js.snap | 8 ++++---- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/report.js b/lib/report.js index e1671ca0..7a9c7e32 100644 --- a/lib/report.js +++ b/lib/report.js @@ -46,11 +46,11 @@ class Report { this.watermarks = watermarks this.resolve = resolvePaths this.exclude = new Exclude({ - exclude: exclude, - include: include, - extension: extension, + exclude, + include, + extension, relativePath: !allowExternal, - excludeNodeModules: excludeNodeModules + excludeNodeModules }) this.excludeAfterRemap = excludeAfterRemap this.shouldInstrumentCache = new Map() @@ -112,11 +112,14 @@ class Report { try { const sources = this._getSourceMap(v8ScriptCov) const path = resolve(this.resolve, v8ScriptCov.url) - const converter = v8toIstanbul(path, this.wrapperLength, sources, (path) => { + const converter = v8toIstanbul(path, this.wrapperLength, sources + // v8toIstanbul sets all coverage stats of the remaining files + // to 100, if some files are excluded here. See below. + /*, (path) => { if (this.excludeAfterRemap) { return !this._shouldInstrument(path) } - }) + } */) await converter.load() if (resultCountPerPath.has(path)) { @@ -145,6 +148,20 @@ class Report { map.merge(converter.toIstanbul()) } } + + // Workaround for too early applied excludeAfterRemap above - + // v8toIstanbul sets all coverage stats of the remaining files + // to 100, if some files are excluded by the excludePath callback. + if (this.excludeAfterRemap) { + const { data } = map + map.data = Object.keys(data).reduce((result, key) => { + if (this._shouldInstrument(key)) { + result[key] = data[key] + } + return result + }, {}) + } + this._allCoverageFiles = map return this._allCoverageFiles } diff --git a/test/integration.js.snap b/test/integration.js.snap index 821d9907..7a5321cb 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 | 1.81 | 12.24 | 6.38 | 1.81 | +All files | 1.79 | 12.24 | 6.38 | 1.79 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -168,7 +168,7 @@ All files | 1.81 | 12.24 | 6.38 | 1.81 c8/lib | 0 | 0 | 0 | 0 | is-cjs-esm-bridge.js | 0 | 0 | 0 | 0 | 1-10 parse-args.js | 0 | 0 | 0 | 0 | 1-224 - report.js | 0 | 0 | 0 | 0 | 1-417 + report.js | 0 | 0 | 0 | 0 | 1-434 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 c8/lib/commands | 0 | 0 | 0 | 0 | check-coverage.js | 0 | 0 | 0 | 0 | 1-70 @@ -522,7 +522,7 @@ hey ---------------------------------------|---------|----------|---------|---------|------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------------------------|---------|----------|---------|---------|------------------- -All files | 1.81 | 12.24 | 6.38 | 1.81 | +All files | 1.79 | 12.24 | 6.38 | 1.79 | c8 | 0 | 0 | 0 | 0 | index.js | 0 | 0 | 0 | 0 | 1 c8/bin | 0 | 0 | 0 | 0 | @@ -534,7 +534,7 @@ All files | 1.81 | 12.24 | 6.38 | 1.81 c8/lib | 0 | 0 | 0 | 0 | is-cjs-esm-bridge.js | 0 | 0 | 0 | 0 | 1-10 parse-args.js | 0 | 0 | 0 | 0 | 1-224 - report.js | 0 | 0 | 0 | 0 | 1-417 + report.js | 0 | 0 | 0 | 0 | 1-434 source-map-from-file.js | 0 | 0 | 0 | 0 | 1-100 c8/lib/commands | 0 | 0 | 0 | 0 | check-coverage.js | 0 | 0 | 0 | 0 | 1-70