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

fix: string //# sourceMappingURL= breaking tests #726

Closed
zanona opened this issue Nov 19, 2017 · 6 comments
Closed

fix: string //# sourceMappingURL= breaking tests #726

zanona opened this issue Nov 19, 2017 · 6 comments
Labels

Comments

@zanona
Copy link

zanona commented Nov 19, 2017

This bug was initially reported on avajs/ava#1595, however, it was confirmed this happened with mocha as well so it might be something happening with nyc.

Expected Behavior

Files containing the string //# sourceMappingURL= to be tested without breaking.

Observed Behavior

Apparently, there's a bug where files containing a string with the initial part of a sourcemap instruction, i.e: //# sourceMappingURL=, will fail to generate code coverage as well as being tested.

Bonus Points! Code (or Repository) that Reproduces Issue

Repro: https://github.com/zanona/ava-sourcemap-fail

Forensic Information

Operating System: the operating system you observed the issue on.
Environment Information: information about your project's environment, see instructions below:

node -e "var os=require('os');console.log('Node.js ' + process.version + '\n' + os.platform() + ' ' + os.release())"
Node.js v8.7.0
darwin 16.7.0
ava --version > 0.23.0
npm --version > 5.5.1
@novemberborn
Copy link
Contributor

I think the source map util used by nyc does a string search for the comment pattern. It doesn't generate an AST for the file.

If it's possible try breaking up the string, e.g. '//# source' + 'MappingURL=' to avoid this problem.

@zanona
Copy link
Author

zanona commented Nov 20, 2017

thank @novemberborn. yep, that's why I have done so far to work around it, however, I believe the parser can be improved to only catch '//# sourceMappingURL=' on the beginning of lines, etc.

@novemberborn
Copy link
Contributor

This is an upstream issue though, see thlorenz/convert-source-map#60.

@zanona
Copy link
Author

zanona commented Nov 20, 2017

@novemberborn Haha, oh my that's NPM 😜 It feels like "The Twelve Tasks of Asterix".
Thanks for pointing that out.

@zanona
Copy link
Author

zanona commented Dec 14, 2017

it seems this has now been fixed at thlorenz/convert-source-map@46a6c17 which sits under 1.5.1, so as long as nyc updates their dependencies, it's all good here as well.

bcoe pushed a commit that referenced this issue Jan 2, 2018
@bcoe
Copy link
Member

bcoe commented Jan 2, 2018

@zanona thanks for tracking down this upstream bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment