Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: function to cleanup allocated resources after usage #161

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ converter.applyCoverage([
// output coverage information in a form that can
// be consumed by Istanbul.
console.info(JSON.stringify(converter.toIstanbul()))

// cleanup resources allocated in "load" (i.e. by the source-map dependency),
// the converter may not be used anymore afterwards
converter.destroy()
```

## Ignoring Uncovered Lines
Expand Down
1 change: 1 addition & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ declare type Sources =
}
declare class V8ToIstanbul {
load(): Promise<void>
destroy(): void
applyCoverage(blocks: ReadonlyArray<Profiler.FunctionCoverage>): void
toIstanbul(): CoverageMapData
}
Expand Down
7 changes: 7 additions & 0 deletions lib/v8-to-istanbul.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,13 @@ module.exports = class V8ToIstanbul {
}
}

destroy () {
if (this.sourceMap) {
this.sourceMap.destroy()
SimenB marked this conversation as resolved.
Show resolved Hide resolved
this.sourceMap = undefined
}
}

_resolveSource (rawSourceMap, sourcePath) {
if (sourcePath.startsWith('file://')) {
return fileURLToPath(sourcePath)
Expand Down
32 changes: 31 additions & 1 deletion test/v8-to-istanbul.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const sourcemap = require('source-map')
const assert = require('assert')

require('tap').mochaGlobals()
require('should')
const should = require('should')

describe('V8ToIstanbul', async () => {
describe('constructor', () => {
Expand All @@ -22,6 +22,8 @@ describe('V8ToIstanbul', async () => {
v8ToIstanbul.covSources[0].source.lines.length.should.equal(48)
v8ToIstanbul.covSources.length.should.equal(1)
v8ToIstanbul.wrapperLength.should.equal(0) // common-js header.

v8ToIstanbul.destroy()
})

it('handles ESM style paths', async () => {
Expand All @@ -33,6 +35,8 @@ describe('V8ToIstanbul', async () => {
v8ToIstanbul.covSources[0].source.lines.length.should.equal(48)
v8ToIstanbul.covSources.length.should.equal(1)
v8ToIstanbul.wrapperLength.should.equal(0) // ESM header.

v8ToIstanbul.destroy()
})

it('handles source maps with sourceRoot', async () => {
Expand Down Expand Up @@ -60,6 +64,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
await v8ToIstanbul.load()

v8ToIstanbul.path.should.equal(absoluteSourceFilePath)

v8ToIstanbul.destroy()
})

it('handles sourceContent', async () => {
Expand Down Expand Up @@ -91,6 +97,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
// if the source is transpiled and since we didn't inline the source map into the transpiled source file
// that means it was bale to access the content via the provided sources object
v8ToIstanbul.sourceTranspiled.should.not.be.undefined()

v8ToIstanbul.destroy()
})

it('should clamp line source column >= 0', async () => {
Expand Down Expand Up @@ -121,6 +129,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
endOffset: matchedNewLineChar + 10
}]
}])

v8ToIstanbul.destroy()
})

it('should exclude files when passing excludePath', async () => {
Expand All @@ -139,6 +149,8 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
}]
}])
Object.keys(v8ToIstanbul.toIstanbul()).should.eql(['/src/index.ts', '/src/utils.ts'].map(path.normalize))

v8ToIstanbul.destroy()
})
})

Expand All @@ -157,6 +169,7 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
0
)
await v8ToIstanbul.load()
v8ToIstanbul.destroy()
})

it('should handle relative sourceRoots correctly', async () => {
Expand All @@ -166,6 +179,7 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}
)
await v8ToIstanbul.load()
assert(v8ToIstanbul.path.includes(path.normalize('v8-to-istanbul/test/fixtures/one-up/relative-source-root.js')))
v8ToIstanbul.destroy()
})

it('should handles source maps with multiple sources', async () => {
Expand All @@ -177,9 +191,25 @@ ${'//'}${'#'} sourceMappingURL=data:application/json;base64,${base64Sourcemap}

v8ToIstanbul.covSources.length.should.equal(3)
Object.keys(v8ToIstanbul.toIstanbul()).should.eql(['/webpack/bootstrap', '/src/index.ts', '/src/utils.ts'].map(path.normalize))

v8ToIstanbul.destroy()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having a test that verifies that the source map has been destroyed would be good, perhaps you could follow @SimenB's recommendation of setting it equal to undefined, and then just assert this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a new test which covers this

})
})

it('destroy cleans up source map', async () => {
const v8ToIstanbul = new V8ToIstanbul(
pathToFileURL(require.resolve('./fixtures/scripts/empty.compiled.js')).href
)
await v8ToIstanbul.load()
// assertion only to check test data and setup - source map must be loaded,
// otherwise destroy would have no effect anyway
assert(v8ToIstanbul.sourceMap !== undefined, 'Test fixture must load a source map')

v8ToIstanbul.destroy()

should.not.exist(v8ToIstanbul.sourceMap)
})

// execute JavaScript files in fixtures directory; these
// files contain the raw v8 output along with a set of
// assertions. the original scripts can be found in the
Expand Down