Skip to content

Commit

Permalink
fix(refactor): remove stale check for createDynamicModule
Browse files Browse the repository at this point in the history 
The logic meant to correct coverage for createDynamicModule, such that it detected the wrapper inserted here https://github.com/nodejs/node/blob/2e81415d9a21756b7516cd3826482a152d1f8b02/lib/internal/modules/esm/create_dynamic_module.js#L51C7-L51C26 appears to have not been working.

Removing for now. We can consider re-adding similar logic in the future with tests if appropriate.
  • Loading branch information
@mcknasty
mcknasty authored Jan 4, 2024
1 parent 128bee2 commit 5e18365
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 33 deletions.
10 changes: 0 additions & 10 deletions lib/is-cjs-esm-bridge.js
View file

This file was deleted.

19 changes: 2 additions & 17 deletions lib/report.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const { pathToFileURL, fileURLToPath } = require('url')
const getSourceMapFromFile = require('./source-map-from-file')
// TODO: switch back to @c88/v8-coverage once patch is landed.
const v8toIstanbul = require('v8-to-istanbul')
const isCjsEsmBridgeCov = require('./is-cjs-esm-bridge')
const util = require('util')
const debuglog = util.debuglog('c8')

Expand Down Expand Up @@ -106,7 +105,6 @@ class Report {
v8ProcessCov = this._getMergedProcessCov()
}
const resultCountPerPath = new Map()
const possibleCjsEsmBridges = new Map()

for (const v8ScriptCov of v8ProcessCov.result) {
try {
Expand All @@ -125,26 +123,13 @@ class Report {
resultCountPerPath.set(path, 0)
}

if (isCjsEsmBridgeCov(v8ScriptCov)) {
possibleCjsEsmBridges.set(converter, {
path,
functions: v8ScriptCov.functions
})
} else {
converter.applyCoverage(v8ScriptCov.functions)
map.merge(converter.toIstanbul())
}
converter.applyCoverage(v8ScriptCov.functions)
map.merge(converter.toIstanbul())
} catch (err) {
debuglog(`file: ${v8ScriptCov.url} error: ${err.stack}`)
}
}

for (const [converter, { path, functions }] of possibleCjsEsmBridges) {
if (resultCountPerPath.get(path) <= 1) {
converter.applyCoverage(functions)
map.merge(converter.toIstanbul())
}
}
this._allCoverageFiles = map
return this._allCoverageFiles
}
Expand Down
10 changes: 4 additions & 6 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 | 3.47 | 12.24 | 6.38 | 3.47 |
All files | 3.52 | 12.5 | 6.52 | 3.52 |
c8 | 0 | 0 | 0 | 0 |
index.js | 0 | 0 | 0 | 0 | 1
c8/bin | 0 | 0 | 0 | 0 |
Expand All @@ -166,9 +166,8 @@ All files | 3.47 | 12.24 | 6.38 | 3.47
prettify.js | 0 | 0 | 0 | 0 | 1-2
sorter.js | 0 | 0 | 0 | 0 | 1-196
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-402
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 +521,7 @@ hey
---------------------------------------|---------|----------|---------|---------|------------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
---------------------------------------|---------|----------|---------|---------|------------------------
All files | 3.47 | 12.24 | 6.38 | 3.47 |
All files | 3.52 | 12.5 | 6.52 | 3.52 |
c8 | 0 | 0 | 0 | 0 |
index.js | 0 | 0 | 0 | 0 | 1
c8/bin | 0 | 0 | 0 | 0 |
Expand All @@ -532,9 +531,8 @@ All files | 3.47 | 12.24 | 6.38 | 3.47
prettify.js | 0 | 0 | 0 | 0 | 1-2
sorter.js | 0 | 0 | 0 | 0 | 1-196
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-402
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 5e18365

Please sign in to comment.