From a71e432c2c3c0a064cb40f2ac1068603f716b42c Mon Sep 17 00:00:00 2001 From: Charles Samborski Date: Tue, 9 Oct 2018 08:57:12 +0200 Subject: [PATCH] fix: process coverage merging This commit uses the [new merge algorithm][merge] to handle sub-processes. It fixes the issues reported in bcoe/v8-coverage-merge#7 and improves performance. merge: https://github.com/demurgos/v8-coverage/blob/master/docs/merge.md --- lib/report.js | 77 ++++++++++++++++++++++++++++------------ package-lock.json | 24 ++++++------- package.json | 2 +- test/integration.js.snap | 10 +++--- 4 files changed, 72 insertions(+), 41 deletions(-) diff --git a/lib/report.js b/lib/report.js index 9fb532947..184f2a2e8 100644 --- a/lib/report.js +++ b/lib/report.js @@ -4,7 +4,7 @@ const libReport = require('istanbul-lib-report') const reports = require('istanbul-reports') const { readdirSync, readFileSync } = require('fs') const { resolve, isAbsolute } = require('path') -const v8CoverageMerge = require('v8-coverage-merge') +const { mergeProcessCovs } = require('@c88/v8-coverage') const v8toIstanbul = require('v8-to-istanbul') class Report { @@ -41,38 +41,47 @@ class Report { }) } _getCoverageMapFromAllCoverageFiles () { + const v8ProcessCov = this._getMergedProcessCov() + const map = libCoverage.createCoverageMap({}) - const mergedResults = {} - this._loadReports().forEach((report) => { - report.result.forEach((result) => { - if (this.exclude.shouldInstrument(result.url) && - (!this.omitRelative || isAbsolute(result.url))) { - if (mergedResults[result.url]) { - mergedResults[result.url] = v8CoverageMerge( - mergedResults[result.url], - result - ) - } else { - mergedResults[result.url] = result - } - } - }) - }) - Object.keys(mergedResults).forEach((url) => { + for (const v8ScriptCov of v8ProcessCov.result) { try { - const result = mergedResults[url] - const path = resolve(this.resolve, result.url) + const path = resolve(this.resolve, v8ScriptCov.url) const script = v8toIstanbul(path) - script.applyCoverage(result.functions) + script.applyCoverage(v8ScriptCov.functions) map.merge(script.toIstanbul()) } catch (err) { - console.warn(`file: ${url} error: ${err.stack}`) + console.warn(`file: ${v8ScriptCov.url} error: ${err.stack}`) } - }) + } return map } + + /** + * Returns the merged V8 process coverage. + * + * The result is computed from the individual process coverages generated + * by Node. It represents the sum of their counts. + * + * @return {ProcessCov} Merged V8 process coverage. + * @private + */ + _getMergedProcessCov () { + const v8ProcessCovs = [] + for (const v8ProcessCov of this._loadReports()) { + v8ProcessCovs.push(this._filterProcessCov(v8ProcessCov)) + } + return mergeProcessCovs(v8ProcessCovs) + } + + /** + * Returns the list of V8 process coverages generated by Node. + * + * @return {ProcessCov[]} Process coverages generated by Node. + * @private + */ _loadReports () { const files = readdirSync(this.tempDirectory) @@ -87,6 +96,28 @@ class Report { } }) } + + /** + * Returns a filtered process coverage. + * + * The result is a copy of the input, with script coverages filtered based + * on their `url` and the current inclusion rules. + * There is no deep cloning. + * + * @param v8ProcessCov V8 process coverage to filter. + * @return {v8ProcessCov} Filtered V8 process coverage. + * @private + */ + _filterProcessCov (v8ProcessCov) { + const result = [] + for (const v8ScriptCov of v8ProcessCov.result) { + if (this.exclude.shouldInstrument(v8ScriptCov.url) && + (!this.omitRelative || isAbsolute(v8ScriptCov.url))) { + result.push(v8ScriptCov) + } + } + return { result } + } } module.exports = function (opts) { diff --git a/package-lock.json b/package-lock.json index 25f84c0b5..9a0836083 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4,6 +4,11 @@ "lockfileVersion": 1, "requires": true, "dependencies": { + "@c88/v8-coverage": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/@c88/v8-coverage/-/v8-coverage-0.1.0.tgz", + "integrity": "sha512-Y3NzP6KPpYlHx0FDOY9E7f1TgtorGuSKwUslgIgOOIsS4WP5UsJOckCW5WtAXB5GP8VLyJoXwJhdIXQVAfY1ew==" + }, "JSONStream": { "version": "1.3.4", "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.4.tgz", @@ -187,7 +192,7 @@ "dependencies": { "chalk": { "version": "1.1.3", - "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "dev": true, "requires": { @@ -438,7 +443,7 @@ }, "commander": { "version": "2.15.1", - "resolved": "http://registry.npmjs.org/commander/-/commander-2.15.1.tgz", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.15.1.tgz", "integrity": "sha512-VlfT9F3V0v+jr4yxPc5gg9s62/fIVWsd2Bk2iD435um1NlGMYdVCq+MjcXnhYq2icNOizHr1kK+5TI6H0Hy0ag==", "dev": true }, @@ -825,7 +830,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true }, @@ -1545,7 +1550,7 @@ }, "external-editor": { "version": "2.2.0", - "resolved": "http://registry.npmjs.org/external-editor/-/external-editor-2.2.0.tgz", + "resolved": "https://registry.npmjs.org/external-editor/-/external-editor-2.2.0.tgz", "integrity": "sha512-bSn6gvGxKt+b7+6TKEv1ZycHleA7aHhRHyAqJyp5pbUFuYYNIzpZnQDk7AsYckyWdEnTeAnay0aCy2aV6iTk9A==", "dev": true, "requires": { @@ -1779,7 +1784,7 @@ }, "minimist": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.0.tgz", "integrity": "sha1-o1AIsg9BOD7sH7kU9M1d95omQoQ=", "dev": true }, @@ -2812,7 +2817,7 @@ }, "mkdirp": { "version": "0.5.1", - "resolved": "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "requires": { "minimist": "0.0.8" @@ -3740,7 +3745,7 @@ }, "chalk": { "version": "1.1.3", - "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "dev": true, "requires": { @@ -4157,11 +4162,6 @@ "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.3.2.tgz", "integrity": "sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA==" }, - "v8-coverage-merge": { - "version": "1.1.2", - "resolved": "https://registry.npmjs.org/v8-coverage-merge/-/v8-coverage-merge-1.1.2.tgz", - "integrity": "sha512-ZgRnN6rS7F5KL0y3UWte70J9JL7yRSiz5B2SG1EYu4lg+pNgSDP7+aIIkzWc6469ucmVlT6HLxeZs3iTrEebaw==" - }, "v8-to-istanbul": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/v8-to-istanbul/-/v8-to-istanbul-1.2.0.tgz", diff --git a/package.json b/package.json index 76dca9b73..4b19697bf 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "rimraf": "^2.6.2", "test-exclude": "^5.0.0", "uuid": "^3.3.2", - "v8-coverage-merge": "^1.1.2", + "@c88/v8-coverage": "^0.1.0", "v8-to-istanbul": "^1.2.0", "yargs": "^12.0.2", "yargs-parser": "^10.1.0" diff --git a/test/integration.js.snap b/test/integration.js.snap index 92ca6c630..9e35f3147 100644 --- a/test/integration.js.snap +++ b/test/integration.js.snap @@ -8,17 +8,17 @@ second --------------------|----------|----------|----------|----------|-------------------| File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s | --------------------|----------|----------|----------|----------|-------------------| -All files | 92.5 | 69.23 | 0 | 92.5 | | +All files | 94.53 | 71.7 | 0 | 94.53 | | bin | 83.72 | 57.14 | 100 | 83.72 | | c8.js | 83.72 | 57.14 | 100 | 83.72 |... 22,40,41,42,43 | - lib | 93.71 | 62.96 | 100 | 93.71 | | + lib | 96.6 | 62.07 | 100 | 96.6 | | parse-args.js | 97.47 | 44.44 | 100 | 97.47 | 55,56 | - report.js | 90.63 | 72.22 | 100 | 90.63 |... 70,71,85,86,87 | - test/fixtures | 95.16 | 83.33 | 0 | 95.16 | | + report.js | 96.06 | 70 | 100 | 96.06 | 55,56,94,95,96 | + test/fixtures | 95.16 | 94.12 | 0 | 95.16 | | async.js | 100 | 100 | 100 | 100 | | multiple-spawn.js | 100 | 100 | 100 | 100 | | normal.js | 85.71 | 75 | 0 | 85.71 | 14,15,16 | - subprocess.js | 100 | 71.43 | 100 | 100 | 9,13 | + subprocess.js | 100 | 100 | 100 | 100 | | --------------------|----------|----------|----------|----------|-------------------| ," `;