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

feat: adds support for source-map production #439

Merged
merged 7 commits into from
Nov 10, 2016
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 9 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

👍

})
}

NYC.prototype.addFile = function (filename) {
Expand Down Expand Up @@ -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
}

Expand All @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions lib/config-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
38 changes: 34 additions & 4 deletions lib/instrumenters/istanbul.js
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -140,6 +141,7 @@
"istanbul-lib-source-maps",
"istanbul-reports",
"md5-hex",
"merge-source-map",
"micromatch",
"mkdirp",
"resolve-from",
Expand All @@ -155,4 +157,4 @@
"find-up"
]
}
}
}
16 changes: 16 additions & 0 deletions test/fixtures/stack-trace.js
Original file line number Diff line number Diff line change
@@ -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;
}
24 changes: 23 additions & 1 deletion test/src/nyc-test.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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([],
Expand Down