Skip to content

Commit

Permalink
use absolute file paths in coverage
Browse files Browse the repository at this point in the history
Pass an absolute path to the instrumenter. This results in the coverage
containing absolute paths.

Adjust caching and source map logic to use absolute paths. Rename variables.

The change to the source map logic should let it handle source map sources that
have absolute paths. Before these were joined to the file path, now the new path
is resolved instead.
  • Loading branch information
novemberborn committed Feb 9, 2016
1 parent 89098db commit 1d53f10
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 52 deletions.
20 changes: 9 additions & 11 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ NYC.prototype._createTransform = function () {
}),
hash: function (code, metadata, salt) {
var hash = md5hex([code, metadata.filename, salt])
_this.hashCache['./' + metadata.relFile] = hash
_this.hashCache[metadata.filename] = hash
return hash
},
factory: this._transformFactory.bind(this),
Expand Down Expand Up @@ -221,20 +221,18 @@ NYC.prototype._transformFactory = function (cacheDir) {

return function (code, metadata, hash) {
var filename = metadata.filename
var relFile = './' + metadata.relFile

var sourceMap = convertSourceMap.fromSource(code) || convertSourceMap.fromMapFileSource(code, path.dirname(filename))

if (sourceMap) {
if (hash) {
var mapPath = path.join(cacheDir, hash + '.map')
fs.writeFileSync(mapPath, sourceMap.toJSON())
} else {
_this.sourceMapCache.addMap(relFile, sourceMap.toJSON())
_this.sourceMapCache.addMap(filename, sourceMap.toJSON())
}
}

return instrumenter.instrumentSync(code, relFile)
return instrumenter.instrumentSync(code, filename)
}
}

Expand Down Expand Up @@ -288,9 +286,9 @@ NYC.prototype.writeCoverageFile = function () {
if (!coverage) return

if (this.enableCache) {
Object.keys(coverage).forEach(function (relFile) {
if (this.hashCache[relFile] && coverage[relFile]) {
coverage[relFile].contentHash = this.hashCache[relFile]
Object.keys(coverage).forEach(function (absFile) {
if (this.hashCache[absFile] && coverage[absFile]) {
coverage[absFile].contentHash = this.hashCache[absFile]
}
}, this)
} else {
Expand Down Expand Up @@ -345,8 +343,8 @@ NYC.prototype._loadReports = function () {
return {}
}

Object.keys(report).forEach(function (relFile) {
var fileReport = report[relFile]
Object.keys(report).forEach(function (absFile) {
var fileReport = report[absFile]
if (fileReport && fileReport.contentHash) {
var hash = fileReport.contentHash
if (!(hash in loadedMaps)) {
Expand All @@ -359,7 +357,7 @@ NYC.prototype._loadReports = function () {
}
}
if (loadedMaps[hash]) {
_this.sourceMapCache.addMap(relFile, loadedMaps[hash])
_this.sourceMapCache.addMap(absFile, loadedMaps[hash])
}
}
})
Expand Down
13 changes: 6 additions & 7 deletions lib/source-map-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@ function SourceMapCache () {
this.cache = {}
}

SourceMapCache.prototype.addMap = function (relFile, map) {
this.cache[relFile] = new SourceMapConsumer(map)
SourceMapCache.prototype.addMap = function (absFile, map) {
this.cache[absFile] = new SourceMapConsumer(map)
}

SourceMapCache.prototype.applySourceMaps = function (report) {
var _this = this

Object.keys(report).forEach(function (relFile) {
var sourceMap = _this.cache[relFile]
Object.keys(report).forEach(function (absFile) {
var sourceMap = _this.cache[absFile]
if (!sourceMap) {
return
}

var fileReport = report[relFile]
var fileReport = report[absFile]
_this._rewritePath(report, fileReport, sourceMap)
_this._rewriteStatements(fileReport, sourceMap)
_this._rewriteFunctions(fileReport, sourceMap)
Expand Down Expand Up @@ -95,8 +95,7 @@ SourceMapCache.prototype._rewritePath = function (report, fileReport, sourceMap)
// only rewrite the path if the file comes from a single source
if (sourceMap.sources.length !== 1) return

var originalPath = './' + path.join(path.dirname(fileReport.path), sourceMap.sources[0])

var originalPath = path.resolve(path.dirname(fileReport.path), sourceMap.sources[0])
report[fileReport.path] = undefined // Hack for Windows tests, until we can normalize paths.
delete report[fileReport.path]

Expand Down
1 change: 1 addition & 0 deletions test/fixtures/_generateReport.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ if (reports.length !== 5) {
var out = fs.createWriteStream(path.join(__dirname, 'report.js'))
out.write('// Generated using node test/fixtures/_generateReport.js\n')
reports.forEach(function (coverage) {
coverage.path = path.relative(nyc.cwd, coverage.path)
out.write('exports[' + JSON.stringify(coverage.path) + '] = ' + JSON.stringify(coverage, null, 2) + '\n')
})
out.end()
Expand Down
20 changes: 10 additions & 10 deletions test/fixtures/report.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Generated using node test/fixtures/_generateReport.js
exports["./node_modules/source-map-fixtures/fixtures/bundle-inline.js"] = {
"path": "./node_modules/source-map-fixtures/fixtures/bundle-inline.js",
exports["node_modules/source-map-fixtures/fixtures/bundle-inline.js"] = {
"path": "node_modules/source-map-fixtures/fixtures/bundle-inline.js",
"s": {
"1": 1,
"2": 1,
Expand Down Expand Up @@ -301,8 +301,8 @@ exports["./node_modules/source-map-fixtures/fixtures/bundle-inline.js"] = {
}
}
}
exports["./node_modules/source-map-fixtures/fixtures/branching-inline.js"] = {
"path": "./node_modules/source-map-fixtures/fixtures/branching-inline.js",
exports["node_modules/source-map-fixtures/fixtures/branching-inline.js"] = {
"path": "node_modules/source-map-fixtures/fixtures/branching-inline.js",
"s": {
"1": 1,
"2": 1,
Expand Down Expand Up @@ -416,8 +416,8 @@ exports["./node_modules/source-map-fixtures/fixtures/branching-inline.js"] = {
}
}
}
exports["./node_modules/source-map-fixtures/fixtures/istanbul-ignore-inline.js"] = {
"path": "./node_modules/source-map-fixtures/fixtures/istanbul-ignore-inline.js",
exports["node_modules/source-map-fixtures/fixtures/istanbul-ignore-inline.js"] = {
"path": "node_modules/source-map-fixtures/fixtures/istanbul-ignore-inline.js",
"s": {
"1": 1,
"2": 1,
Expand Down Expand Up @@ -533,8 +533,8 @@ exports["./node_modules/source-map-fixtures/fixtures/istanbul-ignore-inline.js"]
}
}
}
exports["./node_modules/source-map-fixtures/fixtures/istanbul-ignore-fn-inline.js"] = {
"path": "./node_modules/source-map-fixtures/fixtures/istanbul-ignore-fn-inline.js",
exports["node_modules/source-map-fixtures/fixtures/istanbul-ignore-fn-inline.js"] = {
"path": "node_modules/source-map-fixtures/fixtures/istanbul-ignore-fn-inline.js",
"s": {
"1": 1,
"2": 1,
Expand Down Expand Up @@ -676,8 +676,8 @@ exports["./node_modules/source-map-fixtures/fixtures/istanbul-ignore-fn-inline.j
}
}
}
exports["./node_modules/source-map-fixtures/fixtures/branching-none.js"] = {
"path": "./node_modules/source-map-fixtures/fixtures/branching-none.js",
exports["node_modules/source-map-fixtures/fixtures/branching-none.js"] = {
"path": "node_modules/source-map-fixtures/fixtures/branching-none.js",
"s": {
"1": 1,
"2": 1,
Expand Down
30 changes: 21 additions & 9 deletions test/src/nyc-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ describe('nyc', function () {

proc.on('close', function () {
var reports = _.filter(nyc._loadReports(), function (report) {
return report['./' + signal + '.js']
return report[path.join(fixtures, signal + '.js')]
})
reports.length.should.equal(1)
return done()
Expand Down Expand Up @@ -296,7 +296,14 @@ describe('nyc', function () {
add: function (report) {
// the subprocess we ran should output reports
// for files in the fixtures directory.
Object.keys(report).should.match(/.\/(spawn|child-1|child-2)\.js/)
var expected = [
'spawn.js',
'child-1.js',
'child-2.js'
].map(function (relFile) {
return path.join(fixtures, relFile)
})
expected.should.include.members(Object.keys(report))
}
},
{
Expand Down Expand Up @@ -489,10 +496,11 @@ describe('nyc', function () {
nyc.reset()
nyc.addAllFiles()

var notLoadedPath = path.join(fixtures, './not-loaded.js')
var reports = _.filter(nyc._loadReports(), function (report) {
return ap(report)['./not-loaded.js']
return ap(report)[notLoadedPath]
})
var report = reports[0]['./not-loaded.js']
var report = reports[0][notLoadedPath]

reports.length.should.equal(1)
report.s['1'].should.equal(0)
Expand All @@ -510,10 +518,12 @@ describe('nyc', function () {
require('../fixtures/not-loaded')

nyc.writeCoverageFile()

var notLoadedPath = path.join(fixtures, './not-loaded.js')
var reports = _.filter(nyc._loadReports(), function (report) {
return report['./not-loaded.js']
return report[notLoadedPath]
})
var report = reports[0]['./not-loaded.js']
var report = reports[0][notLoadedPath]

reports.length.should.equal(1)
report.s['1'].should.equal(1)
Expand All @@ -530,21 +540,23 @@ describe('nyc', function () {
)

var nyc = (new NYC({
cwd: fixtures,
require: './test/fixtures/transpile-hook'
}))

nyc.reset()
nyc.addAllFiles()

var needsTranspilePath = path.join(fixtures, './needs-transpile.js')
var reports = _.filter(nyc._loadReports(), function (report) {
return ap(report)['./test/fixtures/needs-transpile.js']
return ap(report)[needsTranspilePath]
})
var report = reports[0]['./test/fixtures/needs-transpile.js']
var report = reports[0][needsTranspilePath]

reports.length.should.equal(1)
report.s['1'].should.equal(0)

fs.unlinkSync('./test/fixtures/needs-transpile.js')
fs.unlinkSync(needsTranspilePath)
return done()
})
})
Expand Down
41 changes: 26 additions & 15 deletions test/src/source-map-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ var path = require('path')
var convertSourceMap = require('convert-source-map')
var sourceMapFixtures = require('source-map-fixtures')

var nycDir = path.join(__dirname, '../..')

// Load source map fixtures.
var covered = _.mapValues({
bundle: sourceMapFixtures.inline('bundle'),
Expand All @@ -19,9 +21,9 @@ var covered = _.mapValues({
return _.assign({
// Coverage for the fixture is stored relative to the root directory. Here
// compute the path to the fixture file relative to the root directory.
relpath: './' + path.relative(path.join(__dirname, '../..'), fixture.file),
absPath: path.resolve(nycDir, fixture.file),
// the sourcemap itself remaps the path.
mappedPath: './' + path.relative(path.join(__dirname, '../..'), fixture.sourceFile),
mappedPath: path.resolve(nycDir, fixture.sourceFile),
// Compute the number of lines in the original source, excluding any line
// break at the end of the file.
maxLine: fixture.sourceContentSync().trimRight().split(/\r?\n/).length
Expand All @@ -39,15 +41,24 @@ try {
var sourceMapCache = new SourceMapCache()
_.forOwn(covered, function (fixture) {
var source = fixture.contentSync()
var sourceMap = convertSourceMap.fromSource(source) || convertSourceMap.fromMapFileSource(source, fixture.relpath)
var sourceMap = convertSourceMap.fromSource(source) || convertSourceMap.fromMapFileSource(source, fixture.absPath)
if (sourceMap) {
sourceMapCache.addMap(fixture.relpath, sourceMap.sourcemap)
sourceMapCache.addMap(fixture.absPath, sourceMap.sourcemap)
}
})

var getReport = function () {
return ap(_.cloneDeep(require('../fixtures/report')))
}
var getReport = (function () {
var report = _.reduce(require('../fixtures/report'), function (result, fileReport, relpath) {
var absPath = path.resolve(nycDir, relpath)
fileReport.path = absPath
result[absPath] = fileReport
return result
}, {})

return function () {
return ap(_.cloneDeep(report))
}
})()
var fixture = covered.inline

require('chai').should()
Expand All @@ -57,7 +68,7 @@ describe('source-map-cache', function () {
it('does not rewrite if there is no source map', function () {
var report = getReport()
sourceMapCache.applySourceMaps(report)
report.should.have.property(covered.none.relpath)
report.should.have.property(covered.none.absPath)
})

it('retains /* istanbul ignore … */ results', function () {
Expand All @@ -72,7 +83,7 @@ describe('source-map-cache', function () {
sourceMapCache.applySourceMaps(report)

var busted = getReport()
var start = busted[covered.inline.relpath].statementMap['1'].start
var start = busted[covered.inline.absPath].statementMap['1'].start
start.line = 0
start.column = -2
sourceMapCache.applySourceMaps(busted)
Expand All @@ -84,21 +95,21 @@ describe('source-map-cache', function () {
it('does not rewrite path if the source map has more than one source', function () {
var report = getReport()
sourceMapCache.applySourceMaps(report)
expect(report[covered.bundle.relpath]).to.not.equal(undefined)
expect(report[covered.bundle.absPath]).to.not.equal(undefined)
})

it('rewrites path if the source map has exactly one source', function () {
var report = ap(_.pick(getReport(), fixture.relpath))
var report = ap(_.pick(getReport(), fixture.absPath))
sourceMapCache.applySourceMaps(report)
expect(report[fixture.relpath]).to.equal(undefined)
expect(report[fixture.absPath]).to.equal(undefined)
report.should.have.property(fixture.mappedPath)
})
})

describe('statements', function () {
it('drops statements that have no mapping back to the original source code', function () {
var report = getReport()
var originalS = report[fixture.relpath].s
var originalS = report[fixture.absPath].s
sourceMapCache.applySourceMaps(report)
Object.keys(report[fixture.mappedPath].s)
.should.be.lt(originalS)
Expand All @@ -120,7 +131,7 @@ describe('source-map-cache', function () {
describe('functions', function () {
it('drops functions that have no mapping back to the original source code', function () {
var report = getReport()
var originalF = report[fixture.relpath].f
var originalF = report[fixture.absPath].f
sourceMapCache.applySourceMaps(report)
Object.keys(report[fixture.mappedPath].f)
.should.be.lt(originalF)
Expand All @@ -142,7 +153,7 @@ describe('source-map-cache', function () {
describe('branches', function () {
it('drops branches that have no mapping back to the original source code', function () {
var report = getReport()
var originalB = report[fixture.relpath].b
var originalB = report[fixture.absPath].b
sourceMapCache.applySourceMaps(report)
Object.keys(report[fixture.mappedPath].b)
.should.be.lt(originalB)
Expand Down

0 comments on commit 1d53f10

Please sign in to comment.