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

Empty coverage output #157

Closed
micnic opened this issue Jan 29, 2016 · 11 comments
Closed

Empty coverage output #157

micnic opened this issue Jan 29, 2016 · 11 comments

Comments

@micnic
Copy link

micnic commented Jan 29, 2016

I created a fake module https://gist.github.com/micnic/f815c0bee18e99805318 and added a test for it, I am using tap which uses nyc for code coverage. As you can see, If I do not add any additional argument I get an empty output, in case I add --all argument the output adds the index.js file but it has 0% coverage which is false as the exported function was executed inside the test file.

Does nyc avoid somehow my index.js file or it is required in the way nyc does not expect the file to be required or I am missing something?

@jamestalmage
Copy link
Member

Try this:

tap --coverage index.test.js

This issue should really be raised with tap, since you aren't using nyc directly.

@micnic
Copy link
Author

micnic commented Jan 29, 2016

@jamestalmage actually the tap commands that I used are equivalent to:

nyc check-coverage tap test/index.test.js
nyc report

that's why I posted here, it gives the same output as before.

I think the problem is somehow related to the nyc's way of tracing required modules or maybe it's something related to istanbul module.

@bcoe
Copy link
Member

bcoe commented Jan 30, 2016

Rather than require('fake') I would try require('../) to require the index file.

@bcoe
Copy link
Member

bcoe commented Feb 1, 2016

@micnic did you get a chance to give this a shot?

@micnic
Copy link
Author

micnic commented Feb 1, 2016

@bcoe yes, I did the change, did not help, the same output, I also tried to run my test using nyc node test/index.test.js but without any good news. Then I moved my fake folder away from the node_modules folder and it worked! It seems that somehow the modules that are inside the node_modules are not checked at all by nyc, can you, please, confirm this?

@novemberborn
Copy link
Contributor

It seems that somehow the modules that are inside the node_modules are not checked at all by nyc, can you, please, confirm this?

This is correct, see https://github.com/bcoe/nyc/#excluding-files, but I think there's a bug. This is supposed to exclude any dependencies from the coverage report. However if you are testing a dependency its working directory would naturally be inside a node_modules directory. We should only be excluding nested and sibling dependencies.

I think behavior should be like this:

path cwd is dir/ cwd is dir/node_modules/foo/
dir/node_modules/foo excluded covered
dir/node_modules/bar excluded excluded
dir/node_modules/foo/node_modules/baz excluded excluded

novemberborn added a commit to novemberborn/nyc that referenced this issue Feb 1, 2016
Files outside of the current working directory (meaning higher up in the file
hierarchy) should not be instrumented.

Fixes istanbuljs#157 where nyc is run from inside a node_modules directory.

Fixes istanbuljs#154 where 'npm link' is used to fulfill dependencies.

This removes matching the include/exclude patterns to the absolute filename, the
patterns are now applied only to the relative portion within the current working
directory.

addAllFiles() has been changed to explicitly add files in the current working
directory only.
novemberborn added a commit to novemberborn/nyc that referenced this issue Feb 1, 2016
Files outside of the current working directory (meaning higher up in the file
hierarchy) should not be instrumented.

Fixes istanbuljs#157 where nyc is run from inside a node_modules directory.

Fixes istanbuljs#154 where 'npm link' is used to fulfill dependencies.

This removes matching the include/exclude patterns to the absolute filename, the
patterns are now applied only to the relative portion within the current working
directory.

addAllFiles() has been changed to explicitly add files in the current working
directory only.
@novemberborn
Copy link
Contributor

@micnic could you try with #160?

@micnic
Copy link
Author

micnic commented Feb 1, 2016

@novemberborn it worked for the fake module, will try a more complex module and will write you when it is ready

@micnic
Copy link
Author

micnic commented Feb 1, 2016

good news, the tests for the real module have the expected output (with about 28 files).
#160 LGTM!

novemberborn added a commit to novemberborn/nyc that referenced this issue Feb 3, 2016
Files outside of the current working directory (meaning higher up in the file
hierarchy) should not be instrumented.

Fixes istanbuljs#157 where nyc is run from inside a node_modules directory.

Fixes istanbuljs#154 where 'npm link' is used to fulfill dependencies.

This removes matching the include/exclude patterns to the absolute filename, the
patterns are now applied only to the relative portion within the current working
directory.

addAllFiles() has been changed to explicitly add files in the current working
directory only.
@bcoe bcoe closed this as completed in #160 Feb 4, 2016
@bcoe
Copy link
Member

bcoe commented Feb 4, 2016

@micnic could you please try this:

npm i nyc@next

I'll publish this version to latest tomorrow, if things work well 👍

@micnic
Copy link
Author

micnic commented Feb 4, 2016

@bcoe nyc@next works fine for me, tried it on two of my modules, waiting for tomorrow release :)

lloydcotten pushed a commit to lloydcotten/nyc that referenced this issue Feb 11, 2016
Files outside of the current working directory (meaning higher up in the file
hierarchy) should not be instrumented.

Fixes istanbuljs#157 where nyc is run from inside a node_modules directory.

Fixes istanbuljs#154 where 'npm link' is used to fulfill dependencies.

This removes matching the include/exclude patterns to the absolute filename, the
patterns are now applied only to the relative portion within the current working
directory.

addAllFiles() has been changed to explicitly add files in the current working
directory only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants