From 31488f5d8d69cb9445dbb008a99f8692adc31af3 Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Wed, 9 Nov 2016 22:13:32 -0800 Subject: [PATCH] feat: adds support for source-map production (#439) --- README.md | 7 +++++++ index.js | 15 ++++++++------ lib/config-util.js | 5 +++++ lib/instrumenters/istanbul.js | 38 +++++++++++++++++++++++++++++++---- package.json | 6 ++++-- test/fixtures/stack-trace.js | 16 +++++++++++++++ test/src/nyc-test.js | 24 +++++++++++++++++++++- 7 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 test/fixtures/stack-trace.js diff --git a/README.md b/README.md index 1c1289cff..43bfcb923 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,13 @@ and a `text-lcov` coverage report. nyc --reporter=lcov --reporter=text-lcov npm test ``` +## Accurate stack traces using source maps + +When `produce-source-map` is set to true, then the instrumented source files will +include inline source maps for the instrumenter transform. When combined with +[source-map-support](https://github.com/evanw/node-source-map-support), +stack traces for instrumented code will reflect their original lines. + ## Support for custom require hooks (babel, webpack, etc.) nyc supports custom require hooks like diff --git a/index.js b/index.js index 8f7e5ef92..2428efb23 100755 --- a/index.js +++ b/index.js @@ -39,6 +39,7 @@ if (/index\.covered\.js$/.test(__filename)) { function NYC (config) { config = config || {} + this.config = config this.subprocessBin = config.subprocessBin || path.resolve(__dirname, './bin/nyc.js') this._tempDirectory = config.tempDirectory || './.nyc_output' @@ -84,8 +85,6 @@ function NYC (config) { this.processInfo = new ProcessInfo(config && config._processInfo) this.rootId = this.processInfo.root || this.generateUniqueID() - this.instrument = config.instrument - this.all = config.all } NYC.prototype._createTransform = function (ext) { @@ -128,7 +127,9 @@ NYC.prototype.instrumenter = function () { } NYC.prototype._createInstrumenter = function () { - return this._instrumenterLib(this.cwd) + return this._instrumenterLib(this.cwd, { + produceSourceMap: this.config.produceSourceMap + }) } NYC.prototype.addFile = function (filename) { @@ -261,14 +262,15 @@ NYC.prototype._transformFactory = function (cacheDir) { return function (code, metadata, hash) { var filename = metadata.filename + var sourceMap = null - if (_this._sourceMap) _this._handleSourceMap(cacheDir, code, hash, filename) + if (_this._sourceMap) sourceMap = _this._handleSourceMap(cacheDir, code, hash, filename) try { - instrumented = instrumenter.instrumentSync(code, filename) + 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.message) + console.warn('failed to instrument', filename, 'with error:', e.stack) instrumented = code } @@ -290,6 +292,7 @@ NYC.prototype._handleSourceMap = function (cacheDir, code, hash, filename) { this.sourceMapCache.registerMap(filename, sourceMap.sourcemap) } } + return sourceMap } NYC.prototype._handleJs = function (code, filename) { diff --git a/lib/config-util.js b/lib/config-util.js index 729c472d3..756cf3333 100644 --- a/lib/config-util.js +++ b/lib/config-util.js @@ -168,6 +168,11 @@ Config.buildYargs = function (cwd) { type: 'boolean', description: 'should nyc detect and handle source maps?' }) + .option('produce-source-map', { + default: false, + type: 'boolean', + description: "should nyc's instrumenter produce source maps?" + }) .option('instrument', { default: true, type: 'boolean', diff --git a/lib/instrumenters/istanbul.js b/lib/instrumenters/istanbul.js index 48555ddfb..c1d4b1338 100644 --- a/lib/instrumenters/istanbul.js +++ b/lib/instrumenters/istanbul.js @@ -1,13 +1,43 @@ -function InstrumenterIstanbul (cwd) { - var istanbul = InstrumenterIstanbul.istanbul() +'use strict' + +var convertSourceMap = require('convert-source-map') +var mergeSourceMap = require('merge-source-map') - return istanbul.createInstrumenter({ +function InstrumenterIstanbul (cwd, options) { + var istanbul = InstrumenterIstanbul.istanbul() + var instrumenter = istanbul.createInstrumenter({ autoWrap: true, coverageVariable: '__coverage__', embedSource: true, noCompact: false, - preserveComments: true + preserveComments: true, + produceSourceMap: options.produceSourceMap }) + + return { + instrumentSync: function (code, filename, sourceMap) { + var instrumented = instrumenter.instrumentSync(code, filename) + // the instrumenter can optionally produce source maps, + // this is useful for features like remapping stack-traces. + // TODO: test source-map merging logic. + if (options.produceSourceMap) { + var lastSourceMap = instrumenter.lastSourceMap() + if (lastSourceMap) { + if (sourceMap) { + lastSourceMap = mergeSourceMap( + sourceMap.toObject(), + lastSourceMap + ) + } + instrumented += '\n' + convertSourceMap.fromObject(lastSourceMap).toComment() + } + } + return instrumented + }, + lastFileCoverage: function () { + return instrumenter.lastFileCoverage() + } + } } InstrumenterIstanbul.istanbul = function () { diff --git a/package.json b/package.json index aacc13829..f3387e0ec 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,7 @@ "istanbul-lib-source-maps": "^1.0.2", "istanbul-reports": "^1.0.0", "md5-hex": "^1.2.0", + "merge-source-map": "^1.0.2", "micromatch": "^2.3.11", "mkdirp": "^0.5.0", "resolve-from": "^2.0.0", @@ -111,8 +112,8 @@ "requirejs": "^2.3.0", "sanitize-filename": "^1.5.3", "sinon": "^1.15.3", - "source-map-support": "^0.4.2", "split-lines": "^1.0.0", + "source-map-support": "^0.4.6", "standard": "^8.0.0", "standard-version": "^3.0.0", "tap": "^8.0.0", @@ -140,6 +141,7 @@ "istanbul-lib-source-maps", "istanbul-reports", "md5-hex", + "merge-source-map", "micromatch", "mkdirp", "resolve-from", @@ -155,4 +157,4 @@ "find-up" ] } -} +} \ No newline at end of file diff --git a/test/fixtures/stack-trace.js b/test/fixtures/stack-trace.js new file mode 100644 index 000000000..b356fb6c5 --- /dev/null +++ b/test/fixtures/stack-trace.js @@ -0,0 +1,16 @@ +'use strict'; + +function blah() { + throw new Error('Blarrh') +} + +var stack; +try { + blah(); +} catch(err) { + stack = err.stack; +} + +module.exports = function() { + return stack; +} diff --git a/test/src/nyc-test.js b/test/src/nyc-test.js index d43b57f19..184e83945 100644 --- a/test/src/nyc-test.js +++ b/test/src/nyc-test.js @@ -1,6 +1,6 @@ /* global describe, it */ -require('source-map-support').install() +require('source-map-support').install({hookRequire: true}) var _ = require('lodash') var ap = require('any-path') var configUtil = require('../../lib/config-util') @@ -202,6 +202,28 @@ describe('nyc', function () { }) }) + describe('produce source map', function () { + it('handles stack traces', function () { + var nyc = new NYC(configUtil.loadConfig('--produce-source-map')) + nyc.reset() + nyc.wrap() + + var check = require('../fixtures/stack-trace') + check().should.match(/stack-trace.js:4:/) + }) + + it('does not handle stack traces when disabled', function () { + var nyc = new NYC(configUtil.loadConfig()) + nyc.reset() + nyc.wrap() + + var check = require('../fixtures/stack-trace') + check().should.match(/stack-trace.js:1:/) + }) + + // TODO: add test for merge source-map logic. + }) + describe('compile handlers for custom extensions are assigned', function () { it('assigns a function to custom extensions', function () { var nyc = new NYC(configUtil.loadConfig([],