Include input source map in file coverage #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄 |
|
CLA is valid! |
MichaReiser
pushed a commit
to MichaReiser/istanbul-lib-source-maps
that referenced
this pull request
Sep 5, 2016
Part of istanbuljs-archived-repos/istanbul-lib-instrument#23 Alternative could be to move it to remap-istanbul instead and implement there a custom source map resolution strategy
MichaReiser
pushed a commit
to MichaReiser/istanbul-instrumenter-loader
that referenced
this pull request
Sep 6, 2016
Pass the intermediate source map to the instrumenter that can be used to remap the coverage report to the original source. Requires istanbuljs-archived-repos/istanbul-lib-instrument#23. Fixes webpack-contrib#22, webpack-contrib#7, karma-runner/karma-coverage#109,
MichaReiser
pushed a commit
to MichaReiser/remap-istanbul
that referenced
this pull request
Sep 6, 2016
When locating the source map, use the source map included in the file coverage whenever possible. Closes SitePen#2 but requires istanbuljs-archived-repos/istanbul-lib-instrument#23
MichaReiser
force-pushed
the
include-source-map
branch
from
4c82ee2
to
b521abe
Compare
bcoe
requested changes
Nov 7, 2016
bcoe
reviewed
Nov 7, 2016
| this.varName = genVar(sourceFilePath); | ||
| this.attrs = {}; | ||
| this.nextIgnore = null; | ||
| this.cov = new SourceCoverage(sourceFilePath); | ||
| this.cov.inputSourceMap(inputSourceMap); |
There was a problem hiding this comment.
Perhaps let's only assign this.data.inputSourceMap if inputSourceMap is undefined? that way, for anyone not using the new API feature, the shape of the coverage object stays identical 👍
MichaReiser
force-pushed
the
include-source-map
branch
from
b521abe
to
8d70de9
Compare
|
Added a simple unit test that verifies if the inputSourceMap is included
Thats right. If the option is not provided (and with your suggested change), the coverage object should be exactly equal to the one before. One question is left. Should inputSourceMap be exposed (no need for .data.inputSourceMap)? Would be more consistent, however, if the coverage object is converted to json that the new property is always included.... |
MichaReiser
pushed a commit
to MichaReiser/istanbul-lib-source-maps
that referenced
this pull request
Nov 7, 2016
Part of istanbuljs-archived-repos/istanbul-lib-instrument#23 Alternative could be to move it to remap-istanbul instead and implement there a custom source map resolution strategy
Add an option to include an input source map. The input source map is the source map that maps the code before its instumentation back it's original form. This is needed to support more complex setups, e.g. when bundlers are involved. If a bundler like webpack, browserify - or you name it - is used, then there are several, intermediate, source maps involved. The main issue is, that the instrumented code is bundled into a single file, and therefore a new source map is created. But the source map of the bundled file cannot be used to remap the istanbul coverage. Therefore, these intermediate source maps need to be stored to be available when creating the report or remaping. This solution is kind of hacky and should start a discussion how to solve it best. Anyway, it fixes variouis issues that, up to now, required hacky solutions. refs SitePen/remap-istanbul#2
MichaReiser
force-pushed
the
include-source-map
branch
from
8d70de9
to
11df1cd
Compare
MichaReiser
pushed a commit
to MichaReiser/istanbul-lib-source-maps
that referenced
this pull request
Nov 7, 2016
Part of istanbuljs-archived-repos/istanbul-lib-instrument#23 Alternative could be to move it to remap-istanbul instead and implement there a custom source map resolution strategy
MichaReiser
pushed a commit
to MichaReiser/remap-istanbul
that referenced
this pull request
Nov 7, 2016
When locating the source map, use the source map included in the file coverage whenever possible. Closes SitePen#2 but requires istanbuljs-archived-repos/istanbul-lib-instrument#23
bcoe
approved these changes
Nov 10, 2016
bcoe
reviewed
Nov 10, 2016
| this.varName = genVar(sourceFilePath); | ||
| this.attrs = {}; | ||
| this.nextIgnore = null; | ||
| this.cov = new SourceCoverage(sourceFilePath); | ||
|
|
||
| if (typeof (inputSourceMap) !== "undefined") { |
deepsweet
pushed a commit
to webpack-contrib/istanbul-instrumenter-loader
that referenced
this pull request
Nov 27, 2016
* Upgrade to istanbul-lib-instrument * Pass intermediate source map to instrumenter Pass the intermediate source map to the instrumenter that can be used to remap the coverage report to the original source. Requires istanbuljs-archived-repos/istanbul-lib-instrument#23. Fixes #22, #7, karma-runner/karma-coverage#109,
|
Solid work @bcoe @MichaReiser Integrating this now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add an option to include an input source map. The input source map
is the source map that maps the code before its instumentation back it's
original form. This is needed to support more complex setups, e.g. when
bundlers are involved. If a bundler like webpack, browserify - or you
name it - is used, then there are several, intermediate, source maps
involved. The main issue is, that the instrumented code is bundled
into a single file, and therefore a new source map is created. But the
source map of the bundled file cannot be used to remap the istanbul coverage.
Therefore, these intermediate source maps need to be stored to be available
when creating the report or remaping.
This solution is kind of hacky and should start a discussion how to solve
it best. Anyway, it fixes variouis issues that, up to now, required hacky
solutions.
refs SitePen/remap-istanbul#2