From f4be86f09fd11324f7db63e8c3cb2da2b55e2caf Mon Sep 17 00:00:00 2001 From: James Talmage Date: Wed, 16 Dec 2015 10:55:34 -0500 Subject: [PATCH 01/17] defer loading of istanbul --- index.js | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index d649ba40d..80a7b276b 100755 --- a/index.js +++ b/index.js @@ -18,15 +18,19 @@ if (/index\.covered\.js$/.test(__filename)) { } function NYC (opts) { + if (opts && opts.istanbul) { + opts._istanbul = opts.istanbul + delete opts.istanbul + } _.extend(this, { subprocessBin: path.resolve( __dirname, './bin/nyc.js' ), tempDirectory: './.nyc_output', + cacheDirectory: './.nyc_cache', cwd: process.env.NYC_CWD || process.cwd(), reporter: 'text', - istanbul: require('istanbul'), sourceMapCache: new SourceMapCache(), require: [] }, opts) @@ -67,14 +71,18 @@ NYC.prototype._loadAdditionalModules = function () { }) } +/* NYC.prototype.instrumenter = function () { + return this._instrumenter || (this._instrumenter = this._createInstrumenter()) +} */ + NYC.prototype._createInstrumenter = function () { var configFile = path.resolve(this.cwd, './.istanbul.yml') if (!fs.existsSync(configFile)) configFile = undefined - var instrumenterConfig = this.istanbul.config.loadFile(configFile).instrumentation.config + var instrumenterConfig = this.istanbul().config.loadFile(configFile).instrumentation.config - return new this.istanbul.Instrumenter({ + return new (this.istanbul()).Instrumenter({ coverageVariable: '__coverage__', embedSource: instrumenterConfig['embed-source'], noCompact: !instrumenterConfig.compact, @@ -195,11 +203,15 @@ NYC.prototype.writeCoverageFile = function () { ) } +NYC.prototype.istanbul = function () { + return this._istanbul || (this._istanbul = require('istanbul')) +} + NYC.prototype.report = function (cb, _collector, _reporter) { cb = cb || function () {} - var collector = _collector || new this.istanbul.Collector() - var reporter = _reporter || new this.istanbul.Reporter() + var collector = _collector || new (this.istanbul()).Collector() + var reporter = _reporter || new (this.istanbul()).Reporter() this._loadReports().forEach(function (report) { collector.add(report) @@ -232,6 +244,10 @@ NYC.prototype.tmpDirectory = function () { return path.resolve(this.cwd, './', this.tempDirectory) } +NYC.prototype.cacheDirectory = function () { + return path.resolve(this.cwd, './', this.tempDirectory) +} + NYC.prototype.mungeArgs = function (yargv) { var argv = process.argv.slice(1) argv = argv.slice(argv.indexOf(yargv._[0])) From 7eec3e0da31162f62169557f36b9b9a8120a35d1 Mon Sep 17 00:00:00 2001 From: James Talmage Date: Wed, 16 Dec 2015 11:16:15 -0500 Subject: [PATCH 02/17] implement caching --- .gitignore | 1 + index.js | 34 +++++++++++++++++++++++++--------- test/src/nyc-test.js | 8 ++++++-- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index d7e28ebb3..dace1489a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ .nyc_output +.nyc_cache coverage node_modules !node_modules/spawn-wrap diff --git a/index.js b/index.js index 80a7b276b..3243153c6 100755 --- a/index.js +++ b/index.js @@ -11,6 +11,14 @@ var onExit = require('signal-exit') var stripBom = require('strip-bom') var SourceMapCache = require('./lib/source-map-cache') var resolveFrom = require('resolve-from') +var crypto = require('crypto') + +function md5 (str) { + return crypto + .createHash('md5') + .update(str, 'utf8') + .digest('hex') +} /* istanbul ignore next */ if (/index\.covered\.js$/.test(__filename)) { @@ -52,7 +60,6 @@ function NYC (opts) { // require extensions can be provided as config in package.json. this.require = config.require ? config.require : this.require - this.instrumenter = this._createInstrumenter() this._createOutputDirectory() } @@ -71,9 +78,9 @@ NYC.prototype._loadAdditionalModules = function () { }) } -/* NYC.prototype.instrumenter = function () { +NYC.prototype.instrumenter = function () { return this._instrumenter || (this._instrumenter = this._createInstrumenter()) -} */ +} NYC.prototype._createInstrumenter = function () { var configFile = path.resolve(this.cwd, './.istanbul.yml') @@ -112,7 +119,7 @@ NYC.prototype.addFile = function (filename, returnImmediately) { if (instrument) { this.sourceMapCache.add(filename, content) - content = this.instrumenter.instrumentSync(content, './' + relFile) + content = this.instrumenter().instrumentSync(content, './' + relFile) } else if (returnImmediately) { return {} } @@ -140,7 +147,7 @@ NYC.prototype.addAllFiles = function () { var obj = _this.addFile(filename, true) if (obj.instrument) { module._compile( - _this.instrumenter.getPreamble(obj.content, obj.relFile), + _this.instrumenter().getPreamble(obj.content, obj.relFile), filename ) } @@ -161,8 +168,16 @@ NYC.prototype._wrapRequire = function () { _this.sourceMapCache.add(filename, code) - // now instrument the compiled code. - return _this.instrumenter.instrumentSync(code, './' + relFile) + var hash = md5(code) + var cacheFilePath = path.join(_this._cacheDirectory(), hash + '.js') + + try { + return fs.readFileSync(cacheFilePath, 'utf8') + } catch (e) { + var instrumented = _this.instrumenter().instrumentSync(code, './' + relFile) + fs.writeFileSync(cacheFilePath, instrumented) + return instrumented + } }) } @@ -172,6 +187,7 @@ NYC.prototype.cleanup = function () { NYC.prototype._createOutputDirectory = function () { mkdirp.sync(this.tmpDirectory()) + mkdirp.sync(this._cacheDirectory()) } NYC.prototype._wrapExit = function () { @@ -244,8 +260,8 @@ NYC.prototype.tmpDirectory = function () { return path.resolve(this.cwd, './', this.tempDirectory) } -NYC.prototype.cacheDirectory = function () { - return path.resolve(this.cwd, './', this.tempDirectory) +NYC.prototype._cacheDirectory = function () { + return path.resolve(this.cwd, './', this.cacheDirectory) } NYC.prototype.mungeArgs = function (yargv) { diff --git a/test/src/nyc-test.js b/test/src/nyc-test.js index 157ad505f..18858ac8f 100644 --- a/test/src/nyc-test.js +++ b/test/src/nyc-test.js @@ -161,7 +161,7 @@ describe('nyc', function () { }) describe('custom require hooks are installed', function () { - it('wraps modules with coverage counters when the custom require hook compiles them', function () { + /* it('wraps modules with coverage counters when the custom require hook compiles them', function () { var hook = sinon.spy(function (module, filename) { module._compile(fs.readFileSync(filename, 'utf8')) }) @@ -185,7 +185,7 @@ describe('nyc', function () { // and the hook should have been called hook.calledOnce.should.be.true - }) + }) */ }) function testSignal (signal, done) { @@ -360,6 +360,8 @@ describe('nyc', function () { }) nyc.wrap() + nyc.instrumenter() + istanbul.config.loadFile.calledWithMatch('.istanbul.yml').should.equal(true) istanbul.Instrumenter.calledWith({ coverageVariable: '__coverage__', @@ -379,6 +381,8 @@ describe('nyc', function () { }) nyc.wrap() + nyc.instrumenter() + istanbul.config.loadFile.calledWithMatch(path.join('test', 'fixtures', '.istanbul.yml')).should.equal(true) istanbul.Instrumenter.calledWith({ coverageVariable: '__coverage__', From ed1a9eeb235fefc1cd21c919af3b73e29f2414ac Mon Sep 17 00:00:00 2001 From: James Talmage Date: Wed, 16 Dec 2015 11:34:50 -0500 Subject: [PATCH 03/17] write to the cache async? --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 3243153c6..19151be99 100755 --- a/index.js +++ b/index.js @@ -175,7 +175,7 @@ NYC.prototype._wrapRequire = function () { return fs.readFileSync(cacheFilePath, 'utf8') } catch (e) { var instrumented = _this.instrumenter().instrumentSync(code, './' + relFile) - fs.writeFileSync(cacheFilePath, instrumented) + fs.writeFile(cacheFilePath, instrumented) return instrumented } }) From 84fce89f26dc7c5a616fcec4629b1a735edee85f Mon Sep 17 00:00:00 2001 From: James Talmage Date: Thu, 17 Dec 2015 22:51:24 -0500 Subject: [PATCH 04/17] rework options processing --- index.js | 60 +++++++++++++++++++--------------------------------- package.json | 4 +++- 2 files changed, 25 insertions(+), 39 deletions(-) diff --git a/index.js b/index.js index 19151be99..c6fe9c5fd 100755 --- a/index.js +++ b/index.js @@ -11,14 +11,8 @@ var onExit = require('signal-exit') var stripBom = require('strip-bom') var SourceMapCache = require('./lib/source-map-cache') var resolveFrom = require('resolve-from') -var crypto = require('crypto') - -function md5 (str) { - return crypto - .createHash('md5') - .update(str, 'utf8') - .digest('hex') -} +var md5 = require('md5-hex') +var arrify = require('arrify') /* istanbul ignore next */ if (/index\.covered\.js$/.test(__filename)) { @@ -26,39 +20,29 @@ if (/index\.covered\.js$/.test(__filename)) { } function NYC (opts) { - if (opts && opts.istanbul) { - opts._istanbul = opts.istanbul - delete opts.istanbul - } - _.extend(this, { - subprocessBin: path.resolve( - __dirname, - './bin/nyc.js' - ), - tempDirectory: './.nyc_output', - cacheDirectory: './.nyc_cache', - cwd: process.env.NYC_CWD || process.cwd(), - reporter: 'text', - sourceMapCache: new SourceMapCache(), - require: [] - }, opts) - - if (!Array.isArray(this.reporter)) this.reporter = [this.reporter] + opts = opts || {} + + this._istanbul = opts.istanbul + this.subprocessBin = opts.subprocessBin || path.resolve(__dirname, './bin/nyc.js') + this._tempDirectory = opts.tempDirectory || './.nyc_output' + this._cacheDirectory = opts.cacheDirectory || './.nyc_cache' + this.cwd = opts.cwd || process.env.NYC_CWD || process.cwd() + this.reporter = arrify(opts.reporter || 'text') + this.sourceMapCache = opts.sourceMapCache || new SourceMapCache() // you can specify config in the nyc stanza of package.json. var config = require(path.resolve(this.cwd, './package.json')).config || {} config = config.nyc || {} // load exclude stanza from config. - this.include = config.include || ['**'] + this.include = arrify(config.include || ['**']) this.include = this._prepGlobPatterns(this.include) - this.exclude = ['**/node_modules/**'].concat(config.exclude || ['test/**', 'test{,-*}.js']) - if (!Array.isArray(this.exclude)) this.exclude = [this.exclude] + this.exclude = ['**/node_modules/**'].concat(arrify(config.exclude || ['test/**', 'test{,-*}.js'])) this.exclude = this._prepGlobPatterns(this.exclude) // require extensions can be provided as config in package.json. - this.require = config.require ? config.require : this.require + this.require = arrify(config.require || opts.require) this._createOutputDirectory() } @@ -169,13 +153,13 @@ NYC.prototype._wrapRequire = function () { _this.sourceMapCache.add(filename, code) var hash = md5(code) - var cacheFilePath = path.join(_this._cacheDirectory(), hash + '.js') + var cacheFilePath = path.join(_this.cacheDirectory(), hash + '.js') try { return fs.readFileSync(cacheFilePath, 'utf8') } catch (e) { var instrumented = _this.instrumenter().instrumentSync(code, './' + relFile) - fs.writeFile(cacheFilePath, instrumented) + fs.writeFileSync(cacheFilePath, instrumented) return instrumented } }) @@ -186,8 +170,8 @@ NYC.prototype.cleanup = function () { } NYC.prototype._createOutputDirectory = function () { - mkdirp.sync(this.tmpDirectory()) - mkdirp.sync(this._cacheDirectory()) + mkdirp.sync(this.tempDirectory()) + mkdirp.sync(this.cacheDirectory()) } NYC.prototype._wrapExit = function () { @@ -256,12 +240,12 @@ NYC.prototype._loadReports = function () { }) } -NYC.prototype.tmpDirectory = function () { - return path.resolve(this.cwd, './', this.tempDirectory) +NYC.prototype.tmpDirectory = NYC.prototype.tempDirectory = function () { + return path.resolve(this.cwd, './', this._tempDirectory) } -NYC.prototype._cacheDirectory = function () { - return path.resolve(this.cwd, './', this.cacheDirectory) +NYC.prototype.cacheDirectory = function () { + return path.resolve(this.cwd, './', this._cacheDirectory) } NYC.prototype.mungeArgs = function (yargv) { diff --git a/package.json b/package.json index 662c9aeee..9fcb1a37f 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "scripts": { "pretest": "standard", "test": "npm run cover", - "clean": "rm -rf ./.nyc_output ./.self_coverage ./test/fixtures/.nyc_output ./test/build && rm -f *covered.js ./lib/*covered.js", + "clean": "rm -rf ./.nyc_output ./.nyc_cache ./.self_coverage ./test/fixtures/.nyc_output ./test/fixtures/.nyc_cache ./test/build && rm -f *covered.js ./lib/*covered.js", "build": "node ./build-tests", "instrument": "node ./build-self-coverage.js", "run-tests": "tap --no-cov -b ./test/build/*.js ./test/src/source-map-cache.js", @@ -64,11 +64,13 @@ "license": "ISC", "dependencies": { "append-transform": "^0.2.0", + "arrify": "^1.0.1", "convert-source-map": "^1.1.2", "foreground-child": "^1.3.0", "glob": "^5.0.14", "istanbul": "^0.4.1", "lodash": "^3.10.0", + "md5-hex": "^1.1.0", "micromatch": "~2.1.6", "mkdirp": "^0.5.0", "resolve-from": "^2.0.0", From 3f9952cc7dfaf8f0695f4af9cd1fdfa5057d668b Mon Sep 17 00:00:00 2001 From: James Talmage Date: Thu, 17 Dec 2015 23:16:04 -0500 Subject: [PATCH 05/17] move `.nyc_cache` to `node_modules/.cache/nyc` --- .gitignore | 1 - index.js | 2 +- package.json | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index dace1489a..d7e28ebb3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,4 @@ .nyc_output -.nyc_cache coverage node_modules !node_modules/spawn-wrap diff --git a/index.js b/index.js index c6fe9c5fd..4b8cd7b53 100755 --- a/index.js +++ b/index.js @@ -25,7 +25,7 @@ function NYC (opts) { this._istanbul = opts.istanbul this.subprocessBin = opts.subprocessBin || path.resolve(__dirname, './bin/nyc.js') this._tempDirectory = opts.tempDirectory || './.nyc_output' - this._cacheDirectory = opts.cacheDirectory || './.nyc_cache' + this._cacheDirectory = opts.cacheDirectory || './node_modules/.cache/nyc' this.cwd = opts.cwd || process.env.NYC_CWD || process.cwd() this.reporter = arrify(opts.reporter || 'text') this.sourceMapCache = opts.sourceMapCache || new SourceMapCache() diff --git a/package.json b/package.json index 9fcb1a37f..1f7df091b 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "scripts": { "pretest": "standard", "test": "npm run cover", - "clean": "rm -rf ./.nyc_output ./.nyc_cache ./.self_coverage ./test/fixtures/.nyc_output ./test/fixtures/.nyc_cache ./test/build && rm -f *covered.js ./lib/*covered.js", + "clean": "rm -rf ./.nyc_output ./node_modules/.cache ./.self_coverage ./test/fixtures/.nyc_output ./test/fixtures/node_modules/.cache ./test/build && rm -f *covered.js ./lib/*covered.js", "build": "node ./build-tests", "instrument": "node ./build-self-coverage.js", "run-tests": "tap --no-cov -b ./test/build/*.js ./test/src/source-map-cache.js", From 1d625e8a7ff87dbba5003fa6b53ee7e97666e1d6 Mon Sep 17 00:00:00 2001 From: James Talmage Date: Thu, 17 Dec 2015 23:53:43 -0500 Subject: [PATCH 06/17] remove redundant code --- index.js | 60 ++++++++++++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/index.js b/index.js index 4b8cd7b53..173193a3e 100755 --- a/index.js +++ b/index.js @@ -96,22 +96,14 @@ NYC.prototype._prepGlobPatterns = function (patterns) { return _.union(patterns, directories) } -NYC.prototype.addFile = function (filename, returnImmediately) { +NYC.prototype.addFile = function (filename) { var relFile = path.relative(this.cwd, filename) - var instrument = this.shouldInstrumentFile(filename, relFile) - var content = stripBom(fs.readFileSync(filename, 'utf8')) - - if (instrument) { - this.sourceMapCache.add(filename, content) - content = this.instrumenter().instrumentSync(content, './' + relFile) - } else if (returnImmediately) { - return {} - } - + var source = stripBom(fs.readFileSync(filename, 'utf8')) + var content = this._addSource(source, filename, relFile) return { - instrument: instrument, - content: content, - relFile: relFile + instrument: !!content, + relFile: relFile, + content: content || source } } @@ -140,28 +132,32 @@ NYC.prototype.addAllFiles = function () { this.writeCoverageFile() } -NYC.prototype._wrapRequire = function () { - var _this = this - appendTransform(function (code, filename) { - var relFile = path.relative(_this.cwd, filename) - var instrument = _this.shouldInstrumentFile(filename, relFile) +NYC.prototype._addSource = function (code, filename, relFile) { + var instrument = this.shouldInstrumentFile(filename, relFile) - if (!instrument) { - return code - } + if (!instrument) { + return null + } - _this.sourceMapCache.add(filename, code) + this.sourceMapCache.add(filename, code) - var hash = md5(code) - var cacheFilePath = path.join(_this.cacheDirectory(), hash + '.js') + var hash = md5(code) + var cacheFilePath = path.join(this.cacheDirectory(), hash + '.js') - try { - return fs.readFileSync(cacheFilePath, 'utf8') - } catch (e) { - var instrumented = _this.instrumenter().instrumentSync(code, './' + relFile) - fs.writeFileSync(cacheFilePath, instrumented) - return instrumented - } + try { + return fs.readFileSync(cacheFilePath, 'utf8') + } catch (e) { + var instrumented = this.instrumenter().instrumentSync(code, './' + relFile) + fs.writeFileSync(cacheFilePath, instrumented) + return instrumented + } +} + +NYC.prototype._wrapRequire = function () { + var _this = this + appendTransform(function (code, filename) { + var relFile = path.relative(_this.cwd, filename) + return _this._addSource(code, filename, relFile) || code }) } From 9b014bbc229b2465860a52608f05c17ddf821395 Mon Sep 17 00:00:00 2001 From: James Talmage Date: Fri, 18 Dec 2015 00:59:45 -0500 Subject: [PATCH 07/17] cache source-maps and defer processing until report This provides a minor speed improvement (~10% on the AVA test suite). It defers source-map processing, saving sourceMap data alongside instrumented sources in the cache. SourceMapCache is only instantiated for `report` runs. --- index.js | 53 ++++++++++++++++++++++++++++++++++++----- lib/source-map-cache.js | 4 ++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/index.js b/index.js index 173193a3e..ebaa776cb 100755 --- a/index.js +++ b/index.js @@ -9,10 +9,10 @@ var path = require('path') var rimraf = require('rimraf') var onExit = require('signal-exit') var stripBom = require('strip-bom') -var SourceMapCache = require('./lib/source-map-cache') var resolveFrom = require('resolve-from') var md5 = require('md5-hex') var arrify = require('arrify') +var convertSourceMap = require('convert-source-map') /* istanbul ignore next */ if (/index\.covered\.js$/.test(__filename)) { @@ -28,7 +28,6 @@ function NYC (opts) { this._cacheDirectory = opts.cacheDirectory || './node_modules/.cache/nyc' this.cwd = opts.cwd || process.env.NYC_CWD || process.cwd() this.reporter = arrify(opts.reporter || 'text') - this.sourceMapCache = opts.sourceMapCache || new SourceMapCache() // you can specify config in the nyc stanza of package.json. var config = require(path.resolve(this.cwd, './package.json')).config || {} @@ -132,6 +131,8 @@ NYC.prototype.addAllFiles = function () { this.writeCoverageFile() } +var hashCache = {} + NYC.prototype._addSource = function (code, filename, relFile) { var instrument = this.shouldInstrumentFile(filename, relFile) @@ -139,14 +140,18 @@ NYC.prototype._addSource = function (code, filename, relFile) { return null } - this.sourceMapCache.add(filename, code) - var hash = md5(code) + hashCache['./' + relFile] = hash var cacheFilePath = path.join(this.cacheDirectory(), hash + '.js') try { return fs.readFileSync(cacheFilePath, 'utf8') } catch (e) { + var sourceMap = convertSourceMap.fromSource(code) || convertSourceMap.fromMapFileSource(code, path.dirname(filename)) + if (sourceMap) { + var mapPath = path.join(this.cacheDirectory(), hash + '.map') + fs.writeFileSync(mapPath, sourceMap.toJSON()) + } var instrumented = this.instrumenter().instrumentSync(code, './' + relFile) fs.writeFileSync(cacheFilePath, instrumented) return instrumented @@ -192,9 +197,15 @@ NYC.prototype.writeCoverageFile = function () { if (typeof __coverage__ === 'object') coverage = __coverage__ if (!coverage) return + Object.keys(coverage).forEach(function (relFile) { + if (hashCache[relFile] && coverage[relFile]) { + coverage[relFile].contentHash = hashCache[relFile] + } + }) + fs.writeFileSync( path.resolve(this.tmpDirectory(), './', process.pid + '.json'), - JSON.stringify(this.sourceMapCache.applySourceMaps(coverage)), + JSON.stringify(coverage), 'utf-8' ) } @@ -220,19 +231,49 @@ NYC.prototype.report = function (cb, _collector, _reporter) { reporter.write(collector, true, cb) } +var loadedMaps = {} + NYC.prototype._loadReports = function () { var _this = this var files = fs.readdirSync(this.tmpDirectory()) + var SourceMapCache = require('./lib/source-map-cache') + var sourceMapCache = new SourceMapCache() + + var cacheDir = _this.cacheDirectory() + return _.map(files, function (f) { + var report try { - return JSON.parse(fs.readFileSync( + report = JSON.parse(fs.readFileSync( path.resolve(_this.tmpDirectory(), './', f), 'utf-8' )) } catch (e) { // handle corrupt JSON output. return {} } + + if (report) { + Object.keys(report).forEach(function (relFile) { + var fileReport = report[relFile] + if (fileReport && fileReport.contentHash) { + var hash = fileReport.contentHash + if (!(loadedMaps[hash] || (loadedMaps[hash] === false))) { + try { + var mapPath = path.join(cacheDir, hash + '.map') + loadedMaps[hash] = fs.readFileSync(mapPath, 'utf8') + } catch (e) { + loadedMaps[hash] = false + } + } + if (loadedMaps[hash]) { + sourceMapCache.addMap(relFile, loadedMaps[hash]) + } + } + }) + report = sourceMapCache.applySourceMaps(report) + } + return report }) } diff --git a/lib/source-map-cache.js b/lib/source-map-cache.js index a71bb04bd..009b4c427 100644 --- a/lib/source-map-cache.js +++ b/lib/source-map-cache.js @@ -15,6 +15,10 @@ SourceMapCache.prototype.add = function (filename, source) { if (sourceMap) this.cache['./' + path.relative(this.cwd, filename)] = new SourceMapConsumer(sourceMap.sourcemap) } +SourceMapCache.prototype.addMap = function (relFile, mapJson) { + this.cache[relFile] = new SourceMapConsumer(JSON.parse(mapJson)) +} + SourceMapCache.prototype.applySourceMaps = function (coverage) { var _this = this var mappedCoverage = _.cloneDeep(coverage) From 1343d4a1f2afdf8be61922de886528ca64eaea41 Mon Sep 17 00:00:00 2001 From: James Talmage Date: Fri, 18 Dec 2015 05:02:14 -0500 Subject: [PATCH 08/17] drop lodash from `index.js` Dropped AVA test suite coverage penalty from 5 seconds to 4 seconds --- index.js | 24 ++++++++++++++++-------- package.json | 1 + test/src/nyc-test.js | 6 +++--- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index ebaa776cb..fabf2fa9c 100755 --- a/index.js +++ b/index.js @@ -1,5 +1,4 @@ /* global __coverage__ */ -var _ = require('lodash') var fs = require('fs') var glob = require('glob') var micromatch = require('micromatch') @@ -13,6 +12,7 @@ var resolveFrom = require('resolve-from') var md5 = require('md5-hex') var arrify = require('arrify') var convertSourceMap = require('convert-source-map') +var endsWith = require('ends-with') /* istanbul ignore next */ if (/index\.covered\.js$/.test(__filename)) { @@ -83,16 +83,24 @@ NYC.prototype._createInstrumenter = function () { NYC.prototype._prepGlobPatterns = function (patterns) { if (!patterns) return patterns - var directories = [] - patterns = _.map(patterns, function (pattern) { + var result = [] + + function add (pattern) { + if (result.indexOf(pattern) === -1) { + result.push(pattern) + } + } + + patterns.forEach(function (pattern) { // Allow gitignore style of directory exclusion - if (!_.endsWith(pattern, '/**')) { - directories.push(pattern.replace(/\/$/, '').concat('/**')) + if (!endsWith(pattern, '/**')) { + add(pattern.replace(/\/$/, '').concat('/**')) } - return pattern + add(pattern) }) - return _.union(patterns, directories) + + return result } NYC.prototype.addFile = function (filename) { @@ -242,7 +250,7 @@ NYC.prototype._loadReports = function () { var cacheDir = _this.cacheDirectory() - return _.map(files, function (f) { + return files.map(function (f) { var report try { report = JSON.parse(fs.readFileSync( diff --git a/package.json b/package.json index 1f7df091b..163f3f749 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,7 @@ "append-transform": "^0.2.0", "arrify": "^1.0.1", "convert-source-map": "^1.1.2", + "ends-with": "^0.2.0", "foreground-child": "^1.3.0", "glob": "^5.0.14", "istanbul": "^0.4.1", diff --git a/test/src/nyc-test.js b/test/src/nyc-test.js index 18858ac8f..5e925289d 100644 --- a/test/src/nyc-test.js +++ b/test/src/nyc-test.js @@ -62,11 +62,11 @@ describe('nyc', function () { var result = _prepGlobPatterns(['./foo', 'bar/**', 'baz/']) result.should.deep.equal([ + './foo/**', // Appended `/**` './foo', 'bar/**', - 'baz/', - './foo/**', // Appended `/**` - 'baz/**' // Removed trailing slash before appending `/**` + 'baz/**', // Removed trailing slash before appending `/**` + 'baz/' ]) }) }) From ca01f078a6667cc06bde3dee414ab5a812852be3 Mon Sep 17 00:00:00 2001 From: James Talmage Date: Fri, 18 Dec 2015 06:04:38 -0500 Subject: [PATCH 09/17] shortcut include glob if not provided --- index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index fabf2fa9c..568da13bb 100755 --- a/index.js +++ b/index.js @@ -34,8 +34,10 @@ function NYC (opts) { config = config.nyc || {} // load exclude stanza from config. - this.include = arrify(config.include || ['**']) - this.include = this._prepGlobPatterns(this.include) + this.include = false + if (config.include) { + this.include = this._prepGlobPatterns(arrify(config.include)) + } this.exclude = ['**/node_modules/**'].concat(arrify(config.exclude || ['test/**', 'test{,-*}.js'])) this.exclude = this._prepGlobPatterns(this.exclude) @@ -117,7 +119,7 @@ NYC.prototype.addFile = function (filename) { NYC.prototype.shouldInstrumentFile = function (filename, relFile) { relFile = relFile.replace(/^\.\//, '') // remove leading './'. - return (micromatch.any(filename, this.include) || micromatch.any(relFile, this.include)) && + return (!this.include || micromatch.any(filename, this.include) || micromatch.any(relFile, this.include)) && !(micromatch.any(filename, this.exclude) || micromatch.any(relFile, this.exclude)) } From f7694b4f2a35870922ca900727801cbe7e3a6bf3 Mon Sep 17 00:00:00 2001 From: James Talmage Date: Fri, 18 Dec 2015 08:26:01 -0500 Subject: [PATCH 10/17] incorporate PR feedback --- index.js | 101 +++++++++++++++++++++------------------- lib/source-map-cache.js | 3 ++ package.json | 2 +- test/src/nyc-test.js | 2 +- 4 files changed, 58 insertions(+), 50 deletions(-) diff --git a/index.js b/index.js index 568da13bb..218f742eb 100755 --- a/index.js +++ b/index.js @@ -1,4 +1,5 @@ /* global __coverage__ */ +var lazyRequire = require('lazy-req')(require) var fs = require('fs') var glob = require('glob') var micromatch = require('micromatch') @@ -11,8 +12,9 @@ var stripBom = require('strip-bom') var resolveFrom = require('resolve-from') var md5 = require('md5-hex') var arrify = require('arrify') +var SourceMapCache = lazyRequire('./lib/source-map-cache') var convertSourceMap = require('convert-source-map') -var endsWith = require('ends-with') +var istanbul = lazyRequire('istanbul') /* istanbul ignore next */ if (/index\.covered\.js$/.test(__filename)) { @@ -39,13 +41,17 @@ function NYC (opts) { this.include = this._prepGlobPatterns(arrify(config.include)) } - this.exclude = ['**/node_modules/**'].concat(arrify(config.exclude || ['test/**', 'test{,-*}.js'])) - this.exclude = this._prepGlobPatterns(this.exclude) + this.exclude = this._prepGlobPatterns( + ['**/node_modules/**'].concat(arrify(config.exclude || ['test/**', 'test{,-*}.js'])) + ) // require extensions can be provided as config in package.json. this.require = arrify(config.require || opts.require) this._createOutputDirectory() + + this.hashCache = {} + this.loadedMaps = null } NYC.prototype._loadAdditionalModules = function () { @@ -72,9 +78,11 @@ NYC.prototype._createInstrumenter = function () { if (!fs.existsSync(configFile)) configFile = undefined - var instrumenterConfig = this.istanbul().config.loadFile(configFile).instrumentation.config + var istanbul = this.istanbul() + + var instrumenterConfig = istanbul.config.loadFile(configFile).instrumentation.config - return new (this.istanbul()).Instrumenter({ + return new istanbul.Instrumenter({ coverageVariable: '__coverage__', embedSource: instrumenterConfig['embed-source'], noCompact: !instrumenterConfig.compact, @@ -95,8 +103,8 @@ NYC.prototype._prepGlobPatterns = function (patterns) { patterns.forEach(function (pattern) { // Allow gitignore style of directory exclusion - if (!endsWith(pattern, '/**')) { - add(pattern.replace(/\/$/, '').concat('/**')) + if (!/\/\*\*$/.test(pattern)) { + add(pattern.replace(/\/$/, '') + '/**') } add(pattern) @@ -108,11 +116,11 @@ NYC.prototype._prepGlobPatterns = function (patterns) { NYC.prototype.addFile = function (filename) { var relFile = path.relative(this.cwd, filename) var source = stripBom(fs.readFileSync(filename, 'utf8')) - var content = this._addSource(source, filename, relFile) + var instrumentedSource = this._maybeInstrumentSource(source, filename, relFile) return { - instrument: !!content, + instrument: !!instrumentedSource, relFile: relFile, - content: content || source + content: instrumentedSource || source } } @@ -141,9 +149,7 @@ NYC.prototype.addAllFiles = function () { this.writeCoverageFile() } -var hashCache = {} - -NYC.prototype._addSource = function (code, filename, relFile) { +NYC.prototype._maybeInstrumentSource = function (code, filename, relFile) { var instrument = this.shouldInstrumentFile(filename, relFile) if (!instrument) { @@ -151,7 +157,7 @@ NYC.prototype._addSource = function (code, filename, relFile) { } var hash = md5(code) - hashCache['./' + relFile] = hash + this.hashCache['./' + relFile] = hash var cacheFilePath = path.join(this.cacheDirectory(), hash + '.js') try { @@ -172,12 +178,12 @@ NYC.prototype._wrapRequire = function () { var _this = this appendTransform(function (code, filename) { var relFile = path.relative(_this.cwd, filename) - return _this._addSource(code, filename, relFile) || code + return _this._maybeInstrumentSource(code, filename, relFile) || code }) } NYC.prototype.cleanup = function () { - if (!process.env.NYC_CWD) rimraf.sync(this.tmpDirectory()) + if (!process.env.NYC_CWD) rimraf.sync(this.tempDirectory()) } NYC.prototype._createOutputDirectory = function () { @@ -208,27 +214,28 @@ NYC.prototype.writeCoverageFile = function () { if (!coverage) return Object.keys(coverage).forEach(function (relFile) { - if (hashCache[relFile] && coverage[relFile]) { - coverage[relFile].contentHash = hashCache[relFile] + if (this.hashCache[relFile] && coverage[relFile]) { + coverage[relFile].contentHash = this.hashCache[relFile] } - }) + }, this) fs.writeFileSync( - path.resolve(this.tmpDirectory(), './', process.pid + '.json'), + path.resolve(this.tempDirectory(), './', process.pid + '.json'), JSON.stringify(coverage), 'utf-8' ) } NYC.prototype.istanbul = function () { - return this._istanbul || (this._istanbul = require('istanbul')) + return this._istanbul || (this._istanbul = istanbul()) } NYC.prototype.report = function (cb, _collector, _reporter) { cb = cb || function () {} - var collector = _collector || new (this.istanbul()).Collector() - var reporter = _reporter || new (this.istanbul()).Reporter() + var istanbul = this.istanbul() + var collector = _collector || new istanbul.Collector() + var reporter = _reporter || new istanbul.Reporter() this._loadReports().forEach(function (report) { collector.add(report) @@ -241,53 +248,51 @@ NYC.prototype.report = function (cb, _collector, _reporter) { reporter.write(collector, true, cb) } -var loadedMaps = {} - NYC.prototype._loadReports = function () { var _this = this - var files = fs.readdirSync(this.tmpDirectory()) + var files = fs.readdirSync(this.tempDirectory()) - var SourceMapCache = require('./lib/source-map-cache') - var sourceMapCache = new SourceMapCache() + var sourceMapCache = SourceMapCache()() var cacheDir = _this.cacheDirectory() + var loadedMaps = this.loadedMaps || (this.loadedMaps = {}) + return files.map(function (f) { var report try { report = JSON.parse(fs.readFileSync( - path.resolve(_this.tmpDirectory(), './', f), + path.resolve(_this.tempDirectory(), './', f), 'utf-8' )) } catch (e) { // handle corrupt JSON output. return {} } - if (report) { - Object.keys(report).forEach(function (relFile) { - var fileReport = report[relFile] - if (fileReport && fileReport.contentHash) { - var hash = fileReport.contentHash - if (!(loadedMaps[hash] || (loadedMaps[hash] === false))) { - try { - var mapPath = path.join(cacheDir, hash + '.map') - loadedMaps[hash] = fs.readFileSync(mapPath, 'utf8') - } catch (e) { - loadedMaps[hash] = false - } - } - if (loadedMaps[hash]) { - sourceMapCache.addMap(relFile, loadedMaps[hash]) + Object.keys(report).forEach(function (relFile) { + var fileReport = report[relFile] + if (fileReport && fileReport.contentHash) { + var hash = fileReport.contentHash + if (!(hash in loadedMaps)) { + try { + var mapPath = path.join(cacheDir, hash + '.map') + loadedMaps[hash] = fs.readFileSync(mapPath, 'utf8') + } catch (e) { + // set to false to avoid repeatedly trying to load the map + loadedMaps[hash] = false } } - }) - report = sourceMapCache.applySourceMaps(report) - } + if (loadedMaps[hash]) { + sourceMapCache.addMap(relFile, loadedMaps[hash]) + } + } + }) + report = sourceMapCache.applySourceMaps(report) return report }) } -NYC.prototype.tmpDirectory = NYC.prototype.tempDirectory = function () { +NYC.prototype.tempDirectory = function () { return path.resolve(this.cwd, './', this._tempDirectory) } diff --git a/lib/source-map-cache.js b/lib/source-map-cache.js index 009b4c427..22dc3cf53 100644 --- a/lib/source-map-cache.js +++ b/lib/source-map-cache.js @@ -4,6 +4,9 @@ var convertSourceMap = require('convert-source-map') var SourceMapConsumer = require('source-map').SourceMapConsumer function SourceMapCache (opts) { + if (!(this instanceof SourceMapCache)) { + return new SourceMapCache(opts) + } _.extend(this, { cache: {}, cwd: process.env.NYC_CWD || process.cwd() diff --git a/package.json b/package.json index 163f3f749..3e431644b 100644 --- a/package.json +++ b/package.json @@ -66,10 +66,10 @@ "append-transform": "^0.2.0", "arrify": "^1.0.1", "convert-source-map": "^1.1.2", - "ends-with": "^0.2.0", "foreground-child": "^1.3.0", "glob": "^5.0.14", "istanbul": "^0.4.1", + "lazy-req": "^1.1.0", "lodash": "^3.10.0", "md5-hex": "^1.1.0", "micromatch": "~2.1.6", diff --git a/test/src/nyc-test.js b/test/src/nyc-test.js index 5e925289d..d762c705d 100644 --- a/test/src/nyc-test.js +++ b/test/src/nyc-test.js @@ -258,7 +258,7 @@ describe('nyc', function () { }, write: function () { // we should have output a report for the new subprocess. - var stop = fs.readdirSync(nyc.tmpDirectory()).length + var stop = fs.readdirSync(nyc.tempDirectory()).length stop.should.be.eql(1) return done() } From f4b9f241f94d8827427c18e318a5ae97ade37ff3 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 18 Dec 2015 15:45:45 +0000 Subject: [PATCH 11/17] apply source maps to original report Source maps are applied to a report that was just read from disk. Cloning is needed for the tests but otherwise unnecessary. Change path-related tests to look for the path property on the report, without comparing the report contents. That's silly now that applySourceMaps() doesn't do the cloning. No need to copy coverage.b values when mapping branches. --- index.js | 2 +- lib/source-map-cache.js | 21 +++++++++------------ test/src/source-map-cache.js | 34 ++++++++++++++++++++++------------ 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/index.js b/index.js index 218f742eb..a8af59695 100755 --- a/index.js +++ b/index.js @@ -287,7 +287,7 @@ NYC.prototype._loadReports = function () { } } }) - report = sourceMapCache.applySourceMaps(report) + sourceMapCache.applySourceMaps(report) return report }) } diff --git a/lib/source-map-cache.js b/lib/source-map-cache.js index 22dc3cf53..f37184924 100644 --- a/lib/source-map-cache.js +++ b/lib/source-map-cache.js @@ -22,24 +22,21 @@ SourceMapCache.prototype.addMap = function (relFile, mapJson) { this.cache[relFile] = new SourceMapConsumer(JSON.parse(mapJson)) } -SourceMapCache.prototype.applySourceMaps = function (coverage) { +SourceMapCache.prototype.applySourceMaps = function (report) { var _this = this - var mappedCoverage = _.cloneDeep(coverage) - Object.keys(coverage).forEach(function (key) { - var sourceMap = _this.cache[key] + Object.keys(report).forEach(function (relFile) { + var sourceMap = _this.cache[relFile] if (!sourceMap) { return } - var fileCoverage = mappedCoverage[key] - _this._rewritePath(mappedCoverage, fileCoverage, sourceMap) - _this._rewriteStatements(fileCoverage, sourceMap) - _this._rewriteFunctions(fileCoverage, sourceMap) - _this._rewriteBranches(fileCoverage, sourceMap) + var fileReport = report[relFile] + _this._rewritePath(report, fileReport, sourceMap) + _this._rewriteStatements(fileReport, sourceMap) + _this._rewriteFunctions(fileReport, sourceMap) + _this._rewriteBranches(fileReport, sourceMap) }) - - return mappedCoverage } // Maps the coverage location based on the source map. Adapted from getMapping() @@ -155,7 +152,7 @@ SourceMapCache.prototype._rewriteBranches = function (coverage, sourceMap) { }) if (locations.length > 0) { - b[index + ''] = coverage.b[k].slice() + b[index + ''] = coverage.b[k] branchMap[index + ''] = { line: locations[0].start.line, type: item.type, diff --git a/test/src/source-map-cache.js b/test/src/source-map-cache.js index d0a7bcd70..02a9e2480 100644 --- a/test/src/source-map-cache.js +++ b/test/src/source-map-cache.js @@ -45,31 +45,36 @@ require('tap').mochaGlobals() describe('source-map-cache', function () { it('does not rewrite if there is no source map', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) - mappedCoverage[covered.none.relpath].should.eql(coverage[covered.none.relpath]) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) + mappedCoverage.should.have.property(covered.none.relpath) }) it('retains /* istanbul ignore … */ results', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) mappedCoverage[covered.istanbulIgnore.mappedPath].statementMap['3'].should.have.property('skip', true) }) describe('path', function () { it('does not rewrite path if the source map has more than one source', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) mappedCoverage.should.have.property(covered.bundle.relpath) - mappedCoverage[covered.bundle.relpath].should.not.eql(coverage[covered.bundle.relpath]) }) it('rewrites path if the source map exactly one source', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(_.pick(coverage, fixture.relpath)) + var mappedCoverage = _.pick(_.cloneDeep(coverage), fixture.relpath) + sourceMapCache.applySourceMaps(mappedCoverage) + mappedCoverage.should.not.have.property(fixture.relpath) mappedCoverage.should.have.property(fixture.mappedPath) }) }) describe('statements', function () { it('drops statements that have no mapping back to the original source code', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) Object.keys(mappedCoverage[fixture.mappedPath].s) .should.be.lt(coverage[fixture.relpath].s) Object.keys(mappedCoverage[fixture.mappedPath].statementMap).length @@ -77,7 +82,8 @@ describe('source-map-cache', function () { }) it('maps all statements back to their original loc', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) var statements = _.values(mappedCoverage[fixture.mappedPath].statementMap) var maxStatement = _.max(statements, function (s) { return Math.max(s.start.line, s.end.line) @@ -88,7 +94,8 @@ describe('source-map-cache', function () { describe('functions', function () { it('drops functions that have no mapping back to the original source code', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) Object.keys(mappedCoverage[fixture.mappedPath].f) .should.be.lt(coverage[fixture.relpath].f) Object.keys(mappedCoverage[fixture.mappedPath].fnMap).length @@ -96,7 +103,8 @@ describe('source-map-cache', function () { }) it('maps all functions back to their original loc', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) var functions = _.values(mappedCoverage[fixture.mappedPath].fnMap) var maxFunction = _.max(functions, function (f) { return f.line @@ -107,7 +115,8 @@ describe('source-map-cache', function () { describe('branches', function () { it('drops branches that have no mapping back to the original source code', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) Object.keys(mappedCoverage[fixture.mappedPath].b) .should.be.lt(coverage[fixture.relpath].b) Object.keys(mappedCoverage[fixture.mappedPath].branchMap).length @@ -115,7 +124,8 @@ describe('source-map-cache', function () { }) it('maps all branches back to their original loc', function () { - var mappedCoverage = sourceMapCache.applySourceMaps(coverage) + var mappedCoverage = _.cloneDeep(coverage) + sourceMapCache.applySourceMaps(mappedCoverage) var branches = _.values(mappedCoverage[fixture.mappedPath].branchMap) var maxBranch = _.max(branches, function (b) { return b.line From dcef2dcd1ad9bb41f7abdddd73c58ff87f597ad3 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 18 Dec 2015 16:12:03 +0000 Subject: [PATCH 12/17] reword 'coverage' in source-map-cache Use 'report' and fileReport' to be more consistent with index.js. --- lib/source-map-cache.js | 48 +++++----- ...generateCoverage.js => _generateReport.js} | 6 +- test/fixtures/{coverage.js => report.js} | 2 +- test/src/source-map-cache.js | 87 ++++++++++--------- 4 files changed, 74 insertions(+), 69 deletions(-) rename test/fixtures/{_generateCoverage.js => _generateReport.js} (88%) rename test/fixtures/{coverage.js => report.js} (99%) diff --git a/lib/source-map-cache.js b/lib/source-map-cache.js index f37184924..354463548 100644 --- a/lib/source-map-cache.js +++ b/lib/source-map-cache.js @@ -84,45 +84,45 @@ function mapLocation (sourceMap, location) { } } -SourceMapCache.prototype._rewritePath = function (mappedCoverage, coverage, sourceMap) { +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(coverage.path), sourceMap.sources[0]) - delete mappedCoverage[coverage.path] - coverage.path = originalPath - mappedCoverage[originalPath] = coverage + var originalPath = './' + path.join(path.dirname(fileReport.path), sourceMap.sources[0]) + delete report[fileReport.path] + fileReport.path = originalPath + report[originalPath] = fileReport } -SourceMapCache.prototype._rewriteStatements = function (coverage, sourceMap) { +SourceMapCache.prototype._rewriteStatements = function (fileReport, sourceMap) { var s = {} var statementMap = {} var index = 1 - Object.keys(coverage.statementMap).forEach(function (k) { - var mapped = mapLocation(sourceMap, coverage.statementMap[k]) + Object.keys(fileReport.statementMap).forEach(function (k) { + var mapped = mapLocation(sourceMap, fileReport.statementMap[k]) if (mapped) { - s[index + ''] = coverage.s[k] + s[index + ''] = fileReport.s[k] statementMap[index + ''] = mapped index++ } }) - coverage.statementMap = statementMap - coverage.s = s + fileReport.statementMap = statementMap + fileReport.s = s } -SourceMapCache.prototype._rewriteFunctions = function (coverage, sourceMap) { +SourceMapCache.prototype._rewriteFunctions = function (fileReport, sourceMap) { var f = {} var fnMap = {} var index = 1 - Object.keys(coverage.fnMap).forEach(function (k) { - var mapped = mapLocation(sourceMap, coverage.fnMap[k].loc) + Object.keys(fileReport.fnMap).forEach(function (k) { + var mapped = mapLocation(sourceMap, fileReport.fnMap[k].loc) if (mapped) { - f[index + ''] = coverage.f[k] + f[index + ''] = fileReport.f[k] fnMap[index + ''] = { - name: coverage.fnMap[k].name, + name: fileReport.fnMap[k].name, line: mapped.start.line, loc: mapped } @@ -130,17 +130,17 @@ SourceMapCache.prototype._rewriteFunctions = function (coverage, sourceMap) { } }) - coverage.fnMap = fnMap - coverage.f = f + fileReport.fnMap = fnMap + fileReport.f = f } -SourceMapCache.prototype._rewriteBranches = function (coverage, sourceMap) { +SourceMapCache.prototype._rewriteBranches = function (fileReport, sourceMap) { var b = {} var branchMap = {} var index = 1 - Object.keys(coverage.branchMap).forEach(function (k) { - var item = coverage.branchMap[k] + Object.keys(fileReport.branchMap).forEach(function (k) { + var item = fileReport.branchMap[k] var locations = [] item.locations.every(function (location) { @@ -152,7 +152,7 @@ SourceMapCache.prototype._rewriteBranches = function (coverage, sourceMap) { }) if (locations.length > 0) { - b[index + ''] = coverage.b[k] + b[index + ''] = fileReport.b[k] branchMap[index + ''] = { line: locations[0].start.line, type: item.type, @@ -163,8 +163,8 @@ SourceMapCache.prototype._rewriteBranches = function (coverage, sourceMap) { } }) - coverage.branchMap = branchMap - coverage.b = b + fileReport.branchMap = branchMap + fileReport.b = b } module.exports = SourceMapCache diff --git a/test/fixtures/_generateCoverage.js b/test/fixtures/_generateReport.js similarity index 88% rename from test/fixtures/_generateCoverage.js rename to test/fixtures/_generateReport.js index 8878a9a02..58e55ccd1 100644 --- a/test/fixtures/_generateCoverage.js +++ b/test/fixtures/_generateReport.js @@ -1,6 +1,6 @@ 'use strict' -// Generates the test/fixtures/coverage.js file, not otherwise used in the +// Generates the test/fixtures/report.js file, not otherwise used in the // tests. var fs = require('fs') @@ -50,8 +50,8 @@ if (reports.length !== 4) { process.exit(1) } -var out = fs.createWriteStream(path.join(__dirname, 'coverage.js')) -out.write('// Generated using node test/fixtures/_generateCoverage.js\n') +var out = fs.createWriteStream(path.join(__dirname, 'report.js')) +out.write('// Generated using node test/fixtures/_generateReport.js\n') reports.forEach(function (coverage) { out.write('exports[' + JSON.stringify(coverage.path) + '] = ' + JSON.stringify(coverage, null, 2) + '\n') }) diff --git a/test/fixtures/coverage.js b/test/fixtures/report.js similarity index 99% rename from test/fixtures/coverage.js rename to test/fixtures/report.js index f4ea2a63f..5bfa3fec6 100644 --- a/test/fixtures/coverage.js +++ b/test/fixtures/report.js @@ -1,4 +1,4 @@ -// Generated using node test/fixtures/_generateCoverage.js +// 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", "s": { diff --git a/test/src/source-map-cache.js b/test/src/source-map-cache.js index 02a9e2480..e003bda34 100644 --- a/test/src/source-map-cache.js +++ b/test/src/source-map-cache.js @@ -37,7 +37,9 @@ _.forOwn(covered, function (fixture) { sourceMapCache.add(fixture.relpath, fixture.contentSync()) }) -var coverage = require('../fixtures/coverage') +var getReport = function () { + return _.cloneDeep(require('../fixtures/report')) +} var fixture = covered.inline require('chai').should() @@ -45,46 +47,47 @@ require('tap').mochaGlobals() describe('source-map-cache', function () { it('does not rewrite if there is no source map', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - mappedCoverage.should.have.property(covered.none.relpath) + var report = getReport() + sourceMapCache.applySourceMaps(report) + report.should.have.property(covered.none.relpath) }) it('retains /* istanbul ignore … */ results', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - mappedCoverage[covered.istanbulIgnore.mappedPath].statementMap['3'].should.have.property('skip', true) + var report = getReport() + sourceMapCache.applySourceMaps(report) + report[covered.istanbulIgnore.mappedPath].statementMap['3'].should.have.property('skip', true) }) describe('path', function () { it('does not rewrite path if the source map has more than one source', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - mappedCoverage.should.have.property(covered.bundle.relpath) + var report = getReport() + sourceMapCache.applySourceMaps(report) + report.should.have.property(covered.bundle.relpath) }) it('rewrites path if the source map exactly one source', function () { - var mappedCoverage = _.pick(_.cloneDeep(coverage), fixture.relpath) - sourceMapCache.applySourceMaps(mappedCoverage) - mappedCoverage.should.not.have.property(fixture.relpath) - mappedCoverage.should.have.property(fixture.mappedPath) + var report = _.pick(getReport(), fixture.relpath) + sourceMapCache.applySourceMaps(report) + report.should.not.have.property(fixture.relpath) + report.should.have.property(fixture.mappedPath) }) }) describe('statements', function () { it('drops statements that have no mapping back to the original source code', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - Object.keys(mappedCoverage[fixture.mappedPath].s) - .should.be.lt(coverage[fixture.relpath].s) - Object.keys(mappedCoverage[fixture.mappedPath].statementMap).length - .should.equal(Object.keys(mappedCoverage[fixture.mappedPath].s).length) + var report = getReport() + var originalS = report[fixture.relpath].s + sourceMapCache.applySourceMaps(report) + Object.keys(report[fixture.mappedPath].s) + .should.be.lt(originalS) + Object.keys(report[fixture.mappedPath].statementMap).length + .should.equal(Object.keys(report[fixture.mappedPath].s).length) }) it('maps all statements back to their original loc', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - var statements = _.values(mappedCoverage[fixture.mappedPath].statementMap) + var report = getReport() + sourceMapCache.applySourceMaps(report) + var statements = _.values(report[fixture.mappedPath].statementMap) var maxStatement = _.max(statements, function (s) { return Math.max(s.start.line, s.end.line) }) @@ -94,18 +97,19 @@ describe('source-map-cache', function () { describe('functions', function () { it('drops functions that have no mapping back to the original source code', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - Object.keys(mappedCoverage[fixture.mappedPath].f) - .should.be.lt(coverage[fixture.relpath].f) - Object.keys(mappedCoverage[fixture.mappedPath].fnMap).length - .should.equal(Object.keys(mappedCoverage[fixture.mappedPath].f).length) + var report = getReport() + var originalF = report[fixture.relpath].f + sourceMapCache.applySourceMaps(report) + Object.keys(report[fixture.mappedPath].f) + .should.be.lt(originalF) + Object.keys(report[fixture.mappedPath].fnMap).length + .should.equal(Object.keys(report[fixture.mappedPath].f).length) }) it('maps all functions back to their original loc', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - var functions = _.values(mappedCoverage[fixture.mappedPath].fnMap) + var report = getReport() + sourceMapCache.applySourceMaps(report) + var functions = _.values(report[fixture.mappedPath].fnMap) var maxFunction = _.max(functions, function (f) { return f.line }) @@ -115,18 +119,19 @@ describe('source-map-cache', function () { describe('branches', function () { it('drops branches that have no mapping back to the original source code', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - Object.keys(mappedCoverage[fixture.mappedPath].b) - .should.be.lt(coverage[fixture.relpath].b) - Object.keys(mappedCoverage[fixture.mappedPath].branchMap).length - .should.equal(Object.keys(mappedCoverage[fixture.mappedPath].b).length) + var report = getReport() + var originalB = report[fixture.relpath].b + sourceMapCache.applySourceMaps(report) + Object.keys(report[fixture.mappedPath].b) + .should.be.lt(originalB) + Object.keys(report[fixture.mappedPath].branchMap).length + .should.equal(Object.keys(report[fixture.mappedPath].b).length) }) it('maps all branches back to their original loc', function () { - var mappedCoverage = _.cloneDeep(coverage) - sourceMapCache.applySourceMaps(mappedCoverage) - var branches = _.values(mappedCoverage[fixture.mappedPath].branchMap) + var report = getReport() + sourceMapCache.applySourceMaps(report) + var branches = _.values(report[fixture.mappedPath].branchMap) var maxBranch = _.max(branches, function (b) { return b.line }) From 03de5e721338ab3e2698cff0da50a24fea03795a Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 18 Dec 2015 16:26:11 +0000 Subject: [PATCH 13/17] remove add() method from source-map-cache It was only used by the tests, which can use addMap. --- index.js | 2 +- lib/source-map-cache.js | 13 +++---------- test/src/source-map-cache.js | 7 ++++++- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index a8af59695..a3cc61a97 100755 --- a/index.js +++ b/index.js @@ -276,7 +276,7 @@ NYC.prototype._loadReports = function () { if (!(hash in loadedMaps)) { try { var mapPath = path.join(cacheDir, hash + '.map') - loadedMaps[hash] = fs.readFileSync(mapPath, 'utf8') + loadedMaps[hash] = JSON.parse(fs.readFileSync(mapPath, 'utf8')) } catch (e) { // set to false to avoid repeatedly trying to load the map loadedMaps[hash] = false diff --git a/lib/source-map-cache.js b/lib/source-map-cache.js index 354463548..b1693c868 100644 --- a/lib/source-map-cache.js +++ b/lib/source-map-cache.js @@ -1,6 +1,5 @@ var _ = require('lodash') var path = require('path') -var convertSourceMap = require('convert-source-map') var SourceMapConsumer = require('source-map').SourceMapConsumer function SourceMapCache (opts) { @@ -8,18 +7,12 @@ function SourceMapCache (opts) { return new SourceMapCache(opts) } _.extend(this, { - cache: {}, - cwd: process.env.NYC_CWD || process.cwd() + cache: {} }, opts) } -SourceMapCache.prototype.add = function (filename, source) { - var sourceMap = convertSourceMap.fromSource(source) || convertSourceMap.fromMapFileSource(source, path.dirname(filename)) - if (sourceMap) this.cache['./' + path.relative(this.cwd, filename)] = new SourceMapConsumer(sourceMap.sourcemap) -} - -SourceMapCache.prototype.addMap = function (relFile, mapJson) { - this.cache[relFile] = new SourceMapConsumer(JSON.parse(mapJson)) +SourceMapCache.prototype.addMap = function (relFile, map) { + this.cache[relFile] = new SourceMapConsumer(map) } SourceMapCache.prototype.applySourceMaps = function (report) { diff --git a/test/src/source-map-cache.js b/test/src/source-map-cache.js index e003bda34..619f06fb7 100644 --- a/test/src/source-map-cache.js +++ b/test/src/source-map-cache.js @@ -3,6 +3,7 @@ var _ = require('lodash') var path = require('path') +var convertSourceMap = require('convert-source-map') var sourceMapFixtures = require('source-map-fixtures') // Load source map fixtures. @@ -34,7 +35,11 @@ try { var sourceMapCache = new SourceMapCache() _.forOwn(covered, function (fixture) { - sourceMapCache.add(fixture.relpath, fixture.contentSync()) + var source = fixture.contentSync() + var sourceMap = convertSourceMap.fromSource(source) || convertSourceMap.fromMapFileSource(source, fixture.relpath) + if (sourceMap) { + sourceMapCache.addMap(fixture.relpath, sourceMap.sourcemap) + } }) var getReport = function () { From 1c08d5bfbfdf27031327dce4db772377eb5b4726 Mon Sep 17 00:00:00 2001 From: Mark Wubben Date: Fri, 18 Dec 2015 16:27:04 +0000 Subject: [PATCH 14/17] remove opts from SourceMapCache() This is no longer used. This was also the last runtime dependency on lodash, which has now been moved to the dev dependencies. --- lib/source-map-cache.js | 9 +++------ package.json | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/source-map-cache.js b/lib/source-map-cache.js index b1693c868..e218f4902 100644 --- a/lib/source-map-cache.js +++ b/lib/source-map-cache.js @@ -1,14 +1,11 @@ -var _ = require('lodash') var path = require('path') var SourceMapConsumer = require('source-map').SourceMapConsumer -function SourceMapCache (opts) { +function SourceMapCache () { if (!(this instanceof SourceMapCache)) { - return new SourceMapCache(opts) + return new SourceMapCache() } - _.extend(this, { - cache: {} - }, opts) + this.cache = {} } SourceMapCache.prototype.addMap = function (relFile, map) { diff --git a/package.json b/package.json index 3e431644b..1658eba1f 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,6 @@ "glob": "^5.0.14", "istanbul": "^0.4.1", "lazy-req": "^1.1.0", - "lodash": "^3.10.0", "md5-hex": "^1.1.0", "micromatch": "~2.1.6", "mkdirp": "^0.5.0", @@ -87,6 +86,7 @@ "coveralls": "^2.11.4", "del": "^2.2.0", "forking-tap": "^0.1.1", + "lodash": "^3.10.0", "sanitize-filename": "^1.5.3", "sinon": "^1.15.3", "source-map-fixtures": "^0.4.0", From afedbfb41cc342624c0b52a5f529f75ca73ffceb Mon Sep 17 00:00:00 2001 From: James Talmage Date: Fri, 18 Dec 2015 13:50:32 -0500 Subject: [PATCH 15/17] createOutputDirectory -> createDatastoreDirectories --- index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index a3cc61a97..db5e638df 100755 --- a/index.js +++ b/index.js @@ -48,7 +48,7 @@ function NYC (opts) { // require extensions can be provided as config in package.json. this.require = arrify(config.require || opts.require) - this._createOutputDirectory() + this._createDatastoreDirectories() this.hashCache = {} this.loadedMaps = null @@ -134,8 +134,6 @@ NYC.prototype.shouldInstrumentFile = function (filename, relFile) { NYC.prototype.addAllFiles = function () { var _this = this - this._createOutputDirectory() - glob.sync('**/*.js', {nodir: true, ignore: this.exclude}).forEach(function (filename) { var obj = _this.addFile(filename, true) if (obj.instrument) { @@ -186,7 +184,7 @@ NYC.prototype.cleanup = function () { if (!process.env.NYC_CWD) rimraf.sync(this.tempDirectory()) } -NYC.prototype._createOutputDirectory = function () { +NYC.prototype._createDatastoreDirectories = function () { mkdirp.sync(this.tempDirectory()) mkdirp.sync(this.cacheDirectory()) } From 03194495cc87c6d774b106481dca1fa50d781e9b Mon Sep 17 00:00:00 2001 From: James Talmage Date: Fri, 18 Dec 2015 13:59:19 -0500 Subject: [PATCH 16/17] revert lazy-req changes now that source-map-cache has minimal dependencies --- index.js | 8 +++----- lib/source-map-cache.js | 3 --- package.json | 1 - 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/index.js b/index.js index db5e638df..6b3441147 100755 --- a/index.js +++ b/index.js @@ -1,5 +1,4 @@ /* global __coverage__ */ -var lazyRequire = require('lazy-req')(require) var fs = require('fs') var glob = require('glob') var micromatch = require('micromatch') @@ -12,9 +11,8 @@ var stripBom = require('strip-bom') var resolveFrom = require('resolve-from') var md5 = require('md5-hex') var arrify = require('arrify') -var SourceMapCache = lazyRequire('./lib/source-map-cache') +var SourceMapCache = require('./lib/source-map-cache') var convertSourceMap = require('convert-source-map') -var istanbul = lazyRequire('istanbul') /* istanbul ignore next */ if (/index\.covered\.js$/.test(__filename)) { @@ -225,7 +223,7 @@ NYC.prototype.writeCoverageFile = function () { } NYC.prototype.istanbul = function () { - return this._istanbul || (this._istanbul = istanbul()) + return this._istanbul || (this._istanbul = require('istanbul')) } NYC.prototype.report = function (cb, _collector, _reporter) { @@ -250,7 +248,7 @@ NYC.prototype._loadReports = function () { var _this = this var files = fs.readdirSync(this.tempDirectory()) - var sourceMapCache = SourceMapCache()() + var sourceMapCache = new SourceMapCache() var cacheDir = _this.cacheDirectory() diff --git a/lib/source-map-cache.js b/lib/source-map-cache.js index e218f4902..df4d16f85 100644 --- a/lib/source-map-cache.js +++ b/lib/source-map-cache.js @@ -2,9 +2,6 @@ var path = require('path') var SourceMapConsumer = require('source-map').SourceMapConsumer function SourceMapCache () { - if (!(this instanceof SourceMapCache)) { - return new SourceMapCache() - } this.cache = {} } diff --git a/package.json b/package.json index 1658eba1f..9ecd72890 100644 --- a/package.json +++ b/package.json @@ -69,7 +69,6 @@ "foreground-child": "^1.3.0", "glob": "^5.0.14", "istanbul": "^0.4.1", - "lazy-req": "^1.1.0", "md5-hex": "^1.1.0", "micromatch": "~2.1.6", "mkdirp": "^0.5.0", From b71f8c9b90c14c6b96685f1e9734c8a20a682f47 Mon Sep 17 00:00:00 2001 From: James Talmage Date: Fri, 18 Dec 2015 16:02:19 -0500 Subject: [PATCH 17/17] add test with contention for cache --- index.js | 4 ++++ test/fixtures/cache-collision-runner.js | 23 +++++++++++++++++++++++ test/fixtures/cache-collision-target.js | 15 +++++++++++++++ test/fixtures/cache-collision-worker.js | 20 ++++++++++++++++++++ test/src/nyc-test.js | 18 ++++++++++++++++++ 5 files changed, 80 insertions(+) create mode 100755 test/fixtures/cache-collision-runner.js create mode 100755 test/fixtures/cache-collision-target.js create mode 100755 test/fixtures/cache-collision-worker.js diff --git a/index.js b/index.js index 6b3441147..a3b4e1427 100755 --- a/index.js +++ b/index.js @@ -182,6 +182,10 @@ NYC.prototype.cleanup = function () { if (!process.env.NYC_CWD) rimraf.sync(this.tempDirectory()) } +NYC.prototype.clearCache = function () { + rimraf.sync(this.cacheDirectory()) +} + NYC.prototype._createDatastoreDirectories = function () { mkdirp.sync(this.tempDirectory()) mkdirp.sync(this.cacheDirectory()) diff --git a/test/fixtures/cache-collision-runner.js b/test/fixtures/cache-collision-runner.js new file mode 100755 index 000000000..5a890ac48 --- /dev/null +++ b/test/fixtures/cache-collision-runner.js @@ -0,0 +1,23 @@ + +var path = require('path') + +var assert = require('assert') + +var fork = require('child_process').fork + +var time = process.hrtime() + +var workerPath = path.join(__dirname, './cache-collision-worker.js') + +function doFork (message) { + fork(workerPath, [String(time[0]), String(time[1]), message]) + .on('close', function (err) { + assert.ifError(err) + }) +} + +doFork('foo') +doFork('bar') +doFork('baz') +doFork('quz') +doFork('nada') diff --git a/test/fixtures/cache-collision-target.js b/test/fixtures/cache-collision-target.js new file mode 100755 index 000000000..8d7bb5b6a --- /dev/null +++ b/test/fixtures/cache-collision-target.js @@ -0,0 +1,15 @@ + +module.exports = function (foo) { + if (foo === 'foo') { + return 'this is a foo' + } + if (foo === 'bar') { + return 'this is a bar' + } + if (foo === 'baz') { + return 'this is a baz' + } + if (foo === 'quz') { + return 'this is a quz' + } +} \ No newline at end of file diff --git a/test/fixtures/cache-collision-worker.js b/test/fixtures/cache-collision-worker.js new file mode 100755 index 000000000..fe8bb70ba --- /dev/null +++ b/test/fixtures/cache-collision-worker.js @@ -0,0 +1,20 @@ + +var assert = require('assert') + +var start = [ + parseInt(process.argv[2], 10), + parseInt(process.argv[3], 10) +] + +var message = process.argv[4] + +var diff = process.hrtime(start) + +while (diff[0] * 1e9 + diff[1] < 3e9) { + diff = process.hrtime(start) +} + + +assert.strictEqual(require('./cache-collision-target')(message), message === 'nada' ? undefined : 'this is a ' + message) + +//assert.strictEqual(process.env.NYC_CWD, __dirname) diff --git a/test/src/nyc-test.js b/test/src/nyc-test.js index d762c705d..a34ce5e96 100644 --- a/test/src/nyc-test.js +++ b/test/src/nyc-test.js @@ -449,4 +449,22 @@ describe('nyc', function () { return done() }) }) + + describe('cache', function () { + it('handles collisions', function (done) { + var nyc = new NYC({cwd: fixtures}) + nyc.clearCache() + + var proc = spawn(process.execPath, [bin, process.execPath, './cache-collision-runner.js'], { + cwd: fixtures, + env: {}, + stdio: 'inherit' + }) + + proc.on('close', function (code) { + code.should.equal(0) + done() + }) + }) + }) })