Skip to content

Commit

Permalink
fix: Fix usage of excludeAfterRemap not to set the coverage always to…
Browse files Browse the repository at this point in the history 
… 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 bcoe#462
  • Loading branch information
@prantlf
prantlf committed Jun 18, 2023
1 parent 57bb94e commit 1aab44c
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 10 deletions.
29 changes: 23 additions & 6 deletions lib/report.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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
}
Expand Down
8 changes: 4 additions & 4 deletions test/integration.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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
Expand Down Expand Up @@ -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 |
Expand All @@ -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
Expand Down

0 comments on commit 1aab44c

Please sign in to comment.