From 61a97927e5e5e1a0eac2a0f95a1cbd31e6ea2cbd Mon Sep 17 00:00:00 2001 From: Joe Mordetsky Date: Thu, 23 Jan 2020 15:22:04 -0500 Subject: [PATCH 1/2] [fix] win32 mismatch of path casings causes duplicate entries --- lib/path-set.js | 25 +++++++ lib/report.js | 5 +- .../all/mock-v8-output/bad-casing.json | 74 +++++++++++++++++++ test/integration.js | 44 ++++++++++- test/integration.js.snap | 14 ++++ test/path-set.js | 38 ++++++++++ 6 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 lib/path-set.js create mode 100644 test/fixtures/all/mock-v8-output/bad-casing.json create mode 100644 test/path-set.js diff --git a/lib/path-set.js b/lib/path-set.js new file mode 100644 index 00000000..20494f7d --- /dev/null +++ b/lib/path-set.js @@ -0,0 +1,25 @@ +/** + * A unique set of paths. Case insensitive on win32 + */ +class PathSet extends Set { + constructor () { + super() + this._win32 = process.platform === 'win32' + } + + add (value) { + if (this._win32) { + value = value.toLowerCase() + } + super.add(value) + } + + has (value) { + if (this._win32) { + value = value.toLowerCase() + } + return super.has(value) + } +} + +module.exports = PathSet diff --git a/lib/report.js b/lib/report.js index 50ba9528..75fa6d05 100644 --- a/lib/report.js +++ b/lib/report.js @@ -3,6 +3,7 @@ const furi = require('furi') const libCoverage = require('istanbul-lib-coverage') const libReport = require('istanbul-lib-report') const reports = require('istanbul-reports') +const PathSet = require('./path-set') const { readdirSync, readFileSync, statSync } = require('fs') const { isAbsolute, resolve, extname } = require('path') const getSourceMapFromFile = require('./source-map-from-file') @@ -144,7 +145,7 @@ class Report { _getMergedProcessCov () { const { mergeProcessCovs } = require('@bcoe/v8-coverage') const v8ProcessCovs = [] - const fileIndex = new Set() // Set + const fileIndex = new PathSet() // PathSet for (const v8ProcessCov of this._loadReports()) { if (this._isCoverageObject(v8ProcessCov)) { if (v8ProcessCov['source-map-cache']) { @@ -217,7 +218,7 @@ class Report { 'utf8' )) } catch (err) { - console.warn(`${err.stack}`) + console.warn(`Failed to parse tmp file: ${f} with: ${err.stack}`) } }) } diff --git a/test/fixtures/all/mock-v8-output/bad-casing.json b/test/fixtures/all/mock-v8-output/bad-casing.json new file mode 100644 index 00000000..6e748bb2 --- /dev/null +++ b/test/fixtures/all/mock-v8-output/bad-casing.json @@ -0,0 +1,74 @@ +{ + "result": [ + { + "scriptId": "51", + "url": "file:///TEST_CWD/test/fixtures/all/vanilla/MAIN.js", + "functions": [ + { + "functionName": "", + "ranges": [ + { + "startOffset": 0, + "endOffset": 111, + "count": 1 + } + ], + "isBlockCoverage": true + } + ] + }, + { + "scriptId": "52", + "url": "file:///TEST_CWD/test/fixtures/all/vanilla/LOADED.js", + "functions": [ + { + "functionName": "", + "ranges": [ + { + "startOffset": 0, + "endOffset": 309, + "count": 1 + } + ], + "isBlockCoverage": true + }, + { + "functionName": "getString", + "ranges": [ + { + "startOffset": 17, + "endOffset": 309, + "count": 3 + }, + { + "startOffset": 89, + "endOffset": 117, + "count": 0 + }, + { + "startOffset": 140, + "endOffset": 169, + "count": 1 + }, + { + "startOffset": 169, + "endOffset": 267, + "count": 2 + }, + { + "startOffset": 190, + "endOffset": 267, + "count": 1 + }, + { + "startOffset": 272, + "endOffset": 306, + "count": 0 + } + ], + "isBlockCoverage": true + } + ] + } + ] +} diff --git a/test/integration.js b/test/integration.js index 839766e0..f2c1eb90 100644 --- a/test/integration.js +++ b/test/integration.js @@ -1,7 +1,15 @@ /* global describe, before, beforeEach, it */ const { spawnSync } = require('child_process') -const { statSync } = require('fs') + +const { + statSync, + readFileSync, + writeFileSync, + existsSync, + mkdirSync +} = require('fs') + const c8Path = require.resolve('../bin/c8') const nodePath = process.execPath const tsNodePath = './node_modules/.bin/ts-node' @@ -415,5 +423,39 @@ describe('c8', () => { ]) output.toString('utf8').should.matchSnapshot() }) + + if (process.platform === 'win32') { + // issue: https://github.com/bcoe/c8/issues/191 + it('should, given path casing mismatches between path.resolve and v8 output, NOT contain duplicate report entries', async () => { + // make a fake v8 blob using a template from fixtures that + // purposely contains case mismatches with nodes path.resolve on windows + const cwd = process.cwd() + const mockOutput = readFileSync('./test/fixtures/all/mock-v8-output/bad-casing.json') + .toString('utf8') + .replace(/TEST_CWD/g, process.cwd().replace(/\\/g, '/')) + const tmpDir = `${cwd}/tmp/mockcwd/` + if (!existsSync(tmpDir)) { + mkdirSync(tmpDir, { recursive: true }) + } + writeFileSync(`${tmpDir}/v8.json`, mockOutput, 'utf-8') + + // run c8 + const { output } = spawnSync(nodePath, [ + c8Path, + 'report', + `--temp-directory=${tmpDir}`, + '--clean=true', + '--all=true', + '--include=test/fixtures/all/vanilla/**/*.js', + '--exclude=**/*.ts', // add an exclude to avoid default excludes of test/** + nodePath, + require.resolve('./fixtures/all/vanilla/main') + ]) + + // prior to issue 191 all would provide an extra 0 coverage record + // for each mismatched file + output.toString('utf8').should.matchSnapshot() + }) + } }) }) diff --git a/test/integration.js.snap b/test/integration.js.snap index f6936bee..19fba957 100644 --- a/test/integration.js.snap +++ b/test/integration.js.snap @@ -88,6 +88,20 @@ exports[`c8 --all should allow for --all to be used with the check-coverage comm " `; +exports[`c8 --all should, given path casing mismatches between path.resolve and v8 output, NOT contain duplicate report entries 1`] = ` +",--------------|---------|----------|---------|---------|------------------- +File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s +--------------|---------|----------|---------|---------|------------------- +All files | 64.29 | 66.67 | 50 | 64.29 | + vanilla | 78.26 | 75 | 100 | 78.26 | + LOADED.js | 73.68 | 71.43 | 100 | 73.68 | 4,5,16-18 + MAIN.js | 100 | 100 | 100 | 100 | + vanilla/dir | 0 | 0 | 0 | 0 | + unloaded.js | 0 | 0 | 0 | 0 | 1-5 +--------------|---------|----------|---------|---------|------------------- +," +`; + exports[`c8 ESM Modules collects coverage for ESM modules 1`] = ` ",bar foo ------------|---------|----------|---------|---------|------------------- diff --git a/test/path-set.js b/test/path-set.js new file mode 100644 index 00000000..92a9b42b --- /dev/null +++ b/test/path-set.js @@ -0,0 +1,38 @@ +/* global describe, it */ +const PathSet = require('../lib/path-set') +const assert = require('assert') +describe('path-set', () => { + it('should have only one entry when adding strings with different cases on win32', () => { + const pathSet = new PathSet() + pathSet._win32 = true + const file = 'foo.js' + pathSet.add(file) + pathSet.add(file.toUpperCase()) + assert.strictEqual(pathSet.size, 1) + }) + + it('should have multiple entries when adding strings with different cases on non-win32', () => { + const pathSet = new PathSet() + pathSet._win32 = false + const file = 'foo.js' + pathSet.add(file) + pathSet.add(file.toUpperCase()) + assert.strictEqual(pathSet.size, 2) + }) + + it('has should return true for different cases on win32', () => { + const pathSet = new PathSet() + pathSet._win32 = true + const file = 'foo.js' + pathSet.add(file) + assert(pathSet.has(file.toUpperCase())) + }) + + it('has should NOT return true for different cases non-win32', () => { + const pathSet = new PathSet() + pathSet._win32 = false + const file = 'foo.js' + pathSet.add(file) + assert(!pathSet.has(file.toUpperCase())) + }) +}) From 5c80da9a15899c4ce0d83b212d63697243a829ce Mon Sep 17 00:00:00 2001 From: Joe Mordetsky Date: Mon, 27 Jan 2020 09:25:00 -0500 Subject: [PATCH 2/2] minor: Set.add should return itself per spec --- lib/path-set.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/path-set.js b/lib/path-set.js index 20494f7d..c4cbaf8d 100644 --- a/lib/path-set.js +++ b/lib/path-set.js @@ -12,6 +12,7 @@ class PathSet extends Set { value = value.toLowerCase() } super.add(value) + return this } has (value) {