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

--all not reporting coverage for all files. #333

Closed
kentcdodds opened this issue Jul 25, 2016 · 10 comments
Closed

--all not reporting coverage for all files. #333

kentcdodds opened this issue Jul 25, 2016 · 10 comments
Labels

Comments

@kentcdodds
Copy link
Member

Here's an example. I've dug through the nyc code locally for over an hour and can't quite figure out what's going on, but if I add a file called foo.js in the src directory, it doesn't appear in the report :-( Any ideas?

@JaKXz
Copy link
Member

JaKXz commented Jul 26, 2016

@kentcdodds just guessing, would using a glob in include be better? I usually do src/** but I'm not sure if it's actually different.

EDIT: it's not, my bad.

@bcoe
Copy link
Member

bcoe commented Jul 26, 2016

@kentcdodds --all isn't working with babel-plugin-istanbul currently:

istanbuljs/babel-plugin-istanbul#4

would love to figure out a good approach for this.

wKovacs64 added a commit to wKovacs64/hibp that referenced this issue Aug 12, 2016
Referenced to get nyc/istanbul reporting properly with tests transpiled inline:
https://github.com/react-bootstrap/react-prop-types (as of v0.4.0)

Note on source maps:
Source map support comes with `mocha --compilers js:babel-register`, which
seems to enable source maps to work in tests but not in the compiled output.
Adding `import 'source-map-support/register'` to the src enables source maps
to work in compiled code but break in tests. Including source-map-support in
src and requiring source-map-support before babel-register when running mocha
(instead of passing the compilers option) appears to get both.

Referenced to get source maps working in test and compiled output:
evanw/node-source-map-support#121

Notes on coverage:
nyc's "--all" option to include reports for files that are not required is
currently not working with babel-plugin-istanbul, but they're aware of the
matter. Also, including reporter settings in the nyc stanza of package.json
appear to trample command line options, so those were moved to the npm test
script instead.

Related issues:
istanbuljs/nyc#333
istanbuljs/babel-plugin-istanbul#4
wKovacs64 added a commit to wKovacs64/pwned that referenced this issue Aug 12, 2016
Referenced to get nyc/istanbul reporting properly with tests transpiled inline:
https://github.com/react-bootstrap/react-prop-types (as of v0.4.0)

Note on source maps:
Source map support comes with `mocha --compilers js:babel-register`, which
seems to enable source maps to work in tests but not in the compiled output.
Adding `import 'source-map-support/register'` to the src enables source maps
to work in compiled code but break in tests. Including source-map-support in
src and requiring source-map-support before babel-register when running mocha
(instead of passing the compilers option) appears to get both.

Referenced to get source maps working in test and compiled output:
evanw/node-source-map-support#121

Notes on coverage:
nyc's "--all" option to include reports for files that are not required is
currently not working with babel-plugin-istanbul, but they're aware of the
matter. Also, including reporter settings in the nyc stanza of package.json
appear to trample command line options, so those were moved to the npm test
script instead.

Related issues:
istanbuljs/nyc#333
istanbuljs/babel-plugin-istanbul#4
@JaKXz
Copy link
Member

JaKXz commented Sep 13, 2016

I think this has been fixed since at least v8+, if not v7 using the "all": true config, and #388 fixes the CLI args precedence, right @bcoe?

@danez
Copy link

danez commented Sep 13, 2016

AFAIK the all option only works if nyc is doing the instrumentation itself, not if the instrumentation is done outside of nyc, like the babel plugin.

@JaKXz
Copy link
Member

JaKXz commented Sep 13, 2016

@danez I believe that's been fixed too... unless I'm remembering wrong I've got it working in a couple projects. See the config section of the README. EDIT: & try npm cache clear && npm i -D nyc babel-plugin-istanbul with sourceMap and instrument set to false and your .babelrc configured as documented.

@danez
Copy link

danez commented Sep 13, 2016

@JaKXz I just tested with https://github.com/researchgate/webpack-watchman-plugin but only two files show up in the coverage report out of 5. The config is exactly like it is in the README, except the require: [] but that doesn't do anything.

@JaKXz JaKXz added the bug label Sep 13, 2016
@JaKXz
Copy link
Member

JaKXz commented Sep 13, 2016

@danez looks like you're right and I just had confirmation bias, my bad!

One thing I did notice in your repo is that you have NODE_ENV set after you call nyc and you'll want to do that before. I tried that locally and it didn't fix it (to my surprise) so I'm not sure what's wrong. @bcoe any ideas?

@bcoe
Copy link
Member

bcoe commented Sep 13, 2016

@JaKXz, @danez --all is not implemented in babel-plugin-istanbul:

istanbuljs/babel-plugin-istanbul#4

Long story short, since we skip the instrumentation step in nyc, we never perform the walk of all files that facilitates --all; would love help figuring out an implementation.

@JaKXz
Copy link
Member

JaKXz commented Sep 13, 2016

Right. Let's move the discussion there then to avoid more duplicating :)

@istanbuljs istanbuljs locked and limited conversation to collaborators Sep 13, 2016
@bcoe
Copy link
Member

bcoe commented Nov 10, 2016

@JaKXz @kentcdodds I could still use help with documenting the feature better, but we now have the core bits and pieces in place to support --all with the babel plugin, as discussed in:

istanbuljs/babel-plugin-istanbul#4

@bcoe bcoe closed this as completed Nov 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants