Improve source maps

[novemberborn]

First of, thanks for adding source map support! I've been trying to use nyc@4 with the project that lead to my code coverage article from last week. I got it working but had to make some changes. This is the second PR which covers the source map handling.

---

I'm now caching a source map if a file is added, even if the dependency wasn't loaded via a custom require hook. This helps with cases where the file was compiled using say a watcher.

nyc now also loads source maps that are merely referenced by the source file.

Finally, if the covered file came from a single source (according to its source map), the path is rewritten to the original source. Subsequent reports that include the file content, like the HTML report, will now show the correct content.

Without rewriting the path the HTML report will show compiled output highlighted using the remapped coverage information.

If there are two or more sources the compiled output will be shown, with the wrong coverage information. I'm not sure how to split coverage over multiple source files. Hopefully that's an edge-case.

It's getting late in the day in my timezone so I haven't had a chance to update the tests to cover this new behavior. The coverage report for my own project looks the same as it did previously with my more custom setup so that's encouraging.

[bcoe]

@novemberborn these changes are looking great \o/

I've added you as a collaborator to the project, it's great to have another set of eyes from someone who's dived so deep into the codebase.

I'll code-review your work more thoroughly after the work day. It's all looking good, but we need to be careful to test nyc in a bunch of hostile environments: babel, no babel, subprocess, no subprocess, Windows, not Windows:

I'd also like to have this issue in mind as we work on sourcemap support:

avajs/ava#185

[bcoe]

Great catch, I bet this was slowing down test runs a ton.

[novemberborn]

Nah your original code still had the guard, but around the rewrite calls. Since I was adding more I figured I'd return early and use variables rather than repeat lookups.

[bcoe]

I tested this on several of my projects and it works wonderfully, great job ⭐

Before we merge, would you mind adding tests for _rewritePath() 👍 otherwise, it's looking great.

[novemberborn]

I've added you as a collaborator to the project, it's great to have another set of eyes from someone who's dived so deep into the codebase.

Thanks!

I'll code-review your work more thoroughly after the work day. It's all looking good, but we need to be careful to test nyc in a bunch of hostile environments: babel, no babel, subprocess, no subprocess, Windows, not Windows:

I wonder if Travis can help with that?

Before we merge, would you mind adding tests for _rewritePath() otherwise, it's looking great.

Sure. Looks like I need to add a fixture file with a source map that doesn't have sources, and another that has more than one source. For which I then need to generate coverage. Could you tell me how test/fixtures/coverage-to-map.json is generated?

[novemberborn]

Could you tell me how test/fixtures/coverage-to-map.json is generated?

Is this how?

rm -rf .nyc_output test/fixtures/.nyc_output
rm -rf coverage
npm test
bin/nyc.js report --reporter=json
cp coverage/coverage-final.json test/fixtures/coverage-to-map.json
# And now prettify the coverage-to-map.json file

The diff is a little different though, and the source-map-cache > branches > maps all branches back to their original loc test now fails. The branchMap object ends up empty.

I'm not quite sure about the relationship between code-with-map.js and es6-not-loaded.js. The latter is quite a lot shorter! Where did the extra code in code-with-map.js go?

It seems like maybe the fixtures in c6a23b9 happen to work but aren't actually reproducible?

Please let me know how to regenerate the coverage map so I can make the current tests pass again with a new map, and then I can add the three new source-map permutations I need to complete test coverage.

(Edit: bin/nyc.js not $(npm bin)/nyc because the tap dependency installs an older nyc version…)

[bcoe]

@novemberborn my secret is out, I generated the fixtures against code that is no longer checked into the codebase -- the approach I used to generate the fixtures was a little bit wonky too, rather than taking the files output in /.nyc_output, I hacked in a couple lines of code to output the fixtures and ran the code with these hacks once. Adding a line here:

https://github.com/bcoe/nyc/blob/master/lib/source-map-cache.js#L14

if (sourceMap) require('fs').writeFileSync('./test/fixtures/my-awesome-fixture.js', source, 'utf-8')

And here:

https://github.com/bcoe/nyc/blob/master/lib/source-map-cache.js#L21

require('fs').writeFileSync('./test/fixtures/my-awesome-fixture.json', JSON.stringify(coverage, null, 2), 'utf-8').

I then created a little testing file that required the specific modules I wanted to create fixtures out of, and ran it with ./bin/nyc -- sorry about how manual and painful this is.

[bcoe]

@novemberborn great work. I'm going to publish this in a minor relese, which will be followed shortly with a major bump when we pull in the in @Lalem001's exclude improvements.

[cades]

I encounter an issue with _rewritePath. The original path src/lib/app.ls become src/lib/src/lib/app.ls after _rewritePath.
And when I run nyc report --reporter=lcov in terminal, it emits an error: Error: ENOENT: no such file or directory, open './src/lib/src/lib/app.ls'

[novemberborn]

@cades could you open a new issue for this? Any sample code allowing us to reproduce the error locally would be appreciated. Thanks.