Skip to content

Commit

Permalink
feat: cache now turned on by default (#454)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: nyc's cache is now enabled by default
  • Loading branch information
bcoe committed Nov 22, 2016
1 parent c3c4d38 commit 0dd970c
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 6 deletions.
3 changes: 2 additions & 1 deletion index.js
@@ -1,5 +1,6 @@
/* global __coverage__ */
var arrify = require('arrify')
var debugLog = require('debug-log')('nyc')
var fs = require('fs')
var glob = require('glob')
var libCoverage = require('istanbul-lib-coverage')
Expand Down Expand Up @@ -270,7 +271,7 @@ NYC.prototype._transformFactory = function (cacheDir) {
instrumented = instrumenter.instrumentSync(code, filename, sourceMap)
} catch (e) {
// don't fail external tests due to instrumentation bugs.
console.warn('failed to instrument', filename, 'with error:', e.stack)
debugLog('failed to instrument ' + filename + 'with error: ' + e.stack)
instrumented = code
}

Expand Down
2 changes: 1 addition & 1 deletion lib/config-util.js
Expand Up @@ -133,7 +133,7 @@ Config.buildYargs = function (cwd) {
})
.option('cache', {
alias: 'c',
default: false,
default: true,
type: 'boolean',
describe: 'cache instrumentation results for improved performance'
})
Expand Down
7 changes: 4 additions & 3 deletions package.json
Expand Up @@ -95,7 +95,7 @@
"rimraf": "^2.5.4",
"signal-exit": "^3.0.1",
"spawn-wrap": "^1.2.4",
"test-exclude": "^3.2.2",
"test-exclude": "^3.3.0",
"yargs": "^6.4.0",
"yargs-parser": "^4.0.2"
},
Expand All @@ -104,6 +104,7 @@
"bundle-dependencies": "^1.0.2",
"chai": "^3.0.0",
"coveralls": "^2.11.11",
"debug-log": "^1.0.1",
"exists-sync": "0.0.4",
"forking-tap": "^0.1.1",
"is-windows": "^0.2.0",
Expand All @@ -112,8 +113,8 @@
"requirejs": "^2.3.0",
"sanitize-filename": "^1.5.3",
"sinon": "^1.15.3",
"split-lines": "^1.0.0",
"source-map-support": "^0.4.6",
"split-lines": "^1.0.0",
"standard": "^8.0.0",
"standard-version": "^3.0.0",
"tap": "^8.0.0",
Expand Down Expand Up @@ -157,4 +158,4 @@
"find-up"
]
}
}
}
2 changes: 1 addition & 1 deletion test/src/nyc-bin.js
Expand Up @@ -655,7 +655,7 @@ describe('the nyc cli', function () {
fs.readFileSync(file, 'utf-8')
))
})
Object.keys(coverage)[0].should.equal('./external-instrumenter.js')
Object.keys(coverage).should.include('./external-instrumenter.js')

// we should not have executed file, so all counts sould be 0.
var sum = 0
Expand Down

4 comments on commit 0dd970c

@tunnckoCore
Copy link

Choose a reason for hiding this comment

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

What's the breaking? I just can't get what can be broken just because some object - one cache.

@bcoe
Copy link
Member Author

@bcoe bcoe commented on 0dd970c Nov 22, 2016

Choose a reason for hiding this comment

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

@tunnckoCore I believe it simply uncovered a race condition where the order of files in the coverage information could vary; it was probably always bad to simply grab the 0th element.

@bcoe
Copy link
Member Author

@bcoe bcoe commented on 0dd970c Nov 22, 2016

Choose a reason for hiding this comment

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

@tunnckoCore with regards to why this is a breaking change; I turned on cache by default a year or so ago, and it broke several dependent projects that were looking at the contents of node_modules; It's enough of a performance gain that I want to enable the cache by default, but I think it needs a major.

@tunnckoCore
Copy link

Choose a reason for hiding this comment

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

@bcoe make sense, thanks.

Please sign in to comment.