Skip to content

Commit

Permalink
[fix] win32 mismatch of path casings causes duplicate entries
Browse files Browse the repository at this point in the history
  • Loading branch information
j03m committed Jan 25, 2020
1 parent 28a492c commit 61a9792
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 3 deletions.
25 changes: 25 additions & 0 deletions lib/path-set.js
Original file line number Diff line number Diff line change
@@ -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
5 changes: 3 additions & 2 deletions lib/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -144,7 +145,7 @@ class Report {
_getMergedProcessCov () {
const { mergeProcessCovs } = require('@bcoe/v8-coverage')
const v8ProcessCovs = []
const fileIndex = new Set() // Set<string>
const fileIndex = new PathSet() // PathSet<string>
for (const v8ProcessCov of this._loadReports()) {
if (this._isCoverageObject(v8ProcessCov)) {
if (v8ProcessCov['source-map-cache']) {
Expand Down Expand Up @@ -217,7 +218,7 @@ class Report {
'utf8'
))
} catch (err) {
console.warn(`${err.stack}`)
console.warn(`Failed to parse tmp file: ${f} with: ${err.stack}`)
}
})
}
Expand Down
74 changes: 74 additions & 0 deletions test/fixtures/all/mock-v8-output/bad-casing.json
Original file line number Diff line number Diff line change
@@ -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
}
]
}
]
}
44 changes: 43 additions & 1 deletion test/integration.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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()
})
}
})
})
14 changes: 14 additions & 0 deletions test/integration.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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
------------|---------|----------|---------|---------|-------------------
Expand Down
38 changes: 38 additions & 0 deletions test/path-set.js
Original file line number Diff line number Diff line change
@@ -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()))
})
})

0 comments on commit 61a9792

Please sign in to comment.