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

fix: Update caching-transform options. #873

Merged
merged 1 commit into from Jun 29, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 3 additions & 5 deletions index.js
Expand Up @@ -91,13 +91,11 @@ function NYC (config) {
}

NYC.prototype._createTransform = function (ext) {
var _this = this
var opts = {
salt: Hash.salt,
hash: function (code, metadata, salt) {
var hash = Hash(code, metadata.filename)
_this.hashCache[metadata.filename] = hash
return hash
hashData: (input, metadata) => [metadata.filename],
onHash: (input, metadata, hash) => {
this.hashCache[metadata.filename] = hash
},
cacheDir: this.cacheDirectory,
// when running --all we should not load source-file from
Expand Down
18 changes: 6 additions & 12 deletions lib/hash.js
@@ -1,14 +1,8 @@
const CACHE_VERSION = require('../package.json').version
const md5hex = require('md5-hex')
const salt = JSON.stringify({
istanbul: require('istanbul-lib-coverage/package.json').version,
nyc: CACHE_VERSION
})
'use strict'

function Hash (code, filename) {
return md5hex([code, filename, salt]) + '_' + CACHE_VERSION
module.exports = {
salt: JSON.stringify({
istanbul: require('istanbul-lib-coverage/package.json').version,
nyc: require('../package.json').version
})
}

Hash.salt = salt

module.exports = Hash
14 changes: 14 additions & 0 deletions test/fixtures/identical-file-runner.js
@@ -0,0 +1,14 @@
const path = require('path')
const assert = require('assert')
const file1 = require('./identical-file1.js')
const file2 = require('./identical-file2.js')

assert.equal(file1(), file2())

const cov = (new Function('return this.__coverage__'))()
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm open to better ways to test that the coverage data includes the 3 files.

Copy link
Member

Choose a reason for hiding this comment

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

@coreyfarrell this assertion seems reasonable to me but, out of curiosity, what would the behavior have been before your fixes to the caching transform, would we have only had one of the two files listed in the coverage object?

mind testing running this test prior to your fix and confirming that it breaks as expected?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did test it, without the fixes to caching-transform this test would not record results for identical-file2.js. This is because both identical files have the same hash value so caching-transform gave the instrumented code from file1 to file2. This meant file2 recorded it's results to file1.


assert.deepEqual(Object.keys(cov).sort(), [
__filename,
path.resolve('identical-file1.js'),
path.resolve('identical-file2.js')
])
5 changes: 5 additions & 0 deletions test/fixtures/identical-file1.js
@@ -0,0 +1,5 @@
function identical() {
return 'identical'
}

module.exports = identical
5 changes: 5 additions & 0 deletions test/fixtures/identical-file2.js
@@ -0,0 +1,5 @@
function identical() {
return 'identical'
}

module.exports = identical
17 changes: 17 additions & 0 deletions test/src/nyc-tap.js
Expand Up @@ -457,5 +457,22 @@ describe('nyc', function () {
done()
})
})

it('handles identical files', function (done) {
var nyc = new NYC(configUtil.buildYargs(fixtures).parse())
nyc.clearCache()

var args = [bin, process.execPath, './identical-file-runner.js']

var proc = spawn(process.execPath, args, {
cwd: fixtures,
env: {}
})

proc.on('close', function (code) {
code.should.equal(0)
done()
})
})
})
})