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

How to ignore coverage data (instrumented code) from dependencies? #956

Closed
henriquemotaesteves opened this issue Dec 28, 2018 · 29 comments · Fixed by #1010
Closed

How to ignore coverage data (instrumented code) from dependencies? #956

henriquemotaesteves opened this issue Dec 28, 2018 · 29 comments · Fixed by #1010
Assignees

Comments

@henriquemotaesteves
Copy link
Contributor

Link to bug demonstration repository.

https://github.com/henriquemotaesteves/nyc-bug-demo

Expected Behavior

How to ignore coverage data (instrumented code) from dependencies?

Observed Behavior

If a dependency is packed and published with coverage data (instrumented code), then these data are included in the coverage report.

Forensic Information

https://gist.github.com/henriquemotaesteves/2bcb39a27e1beda6a8a22c47805c8f95

1

[1] - https://github.com/webpack-contrib/istanbul-instrumenter-loader
[2] - https://github.com/dolanmiu/docx

@bcoe
Copy link
Member

bcoe commented Dec 29, 2018

Hello @henriquemotaesteves 👋

You should be able to ignore specific files by customizing nyc's exclude rules:

https://github.com/istanbuljs/nyc#excluding-files

This can be done either in your package.json, in a .nycrc, or by passing additional exclude rules to the CLI using -x. The rules use globs:

https://www.npmjs.com/package/glob

@henriquemotaesteves
Copy link
Contributor Author

henriquemotaesteves commented Jan 2, 2019

Hello @bcoe.

I tested the proposed solution and it did not work. The location of the file is already ignored by default.

https://github.com/henriquemotaesteves/nyc-bug-demo/commit/fb5a8c370caeeba5d2140460409f088be9052648

The coverage data (exported cov_... functions) displayed by the report is injected indirectly when the dependency is imported by a file that can't be ignored because it is the target of the coverage process. The paths "Users/Dolan/Documents/docx/..." reported did not exist in my local disk.

I did a test commenting the exported coverage data and this solution works, despite it is impractical.

https://github.com/henriquemotaesteves/nyc-bug-demo/commit/a5905186e594758d656000ea6bc626861a95257d

@c0bra
Copy link

c0bra commented Jan 4, 2019

Struggling with this as well on a rollup-based project. No amount of futzing with the exclude option seems to do anything when istanbul is processing the source-mapped bundle.

@ssube
Copy link

ssube commented Jan 13, 2019

I'm having a similar issue with webpack (#953) where exclude options (default, from package.json, or from .nycrc) are being ignored when run against the bundle, while source maps are respected. Setting --exclude-after-remap didn't help.

@henriquemotaesteves
Copy link
Contributor Author

The merged pull request istanbuljs/istanbuljs#275 closes this issue, at least for me. @c0bra and @ssube, could you, please, confirm that is also true for your issues?

@c0bra
Copy link

c0bra commented Jan 22, 2019

@henriquemotaesteves I might be misunderstanding how to test this locally:

  1. I cloned istanbuljs, did npm install and npm link
  2. Then npm link istanbuljs in nyc in my node_modules
  3. Then added this to my package.json and ran cover:
"nyc": {
    "instrumentation": {
      "excludes": ["**/node_modules/**"]
    }
  }

No difference. I'm probably missing something, yea?

@henriquemotaesteves
Copy link
Contributor Author

@c0bra, could you provide a coverage output (screenshot)?

@c0bra
Copy link

c0bra commented Jan 22, 2019

@henriquemotaesteves yup:

---------------------|----------|----------|----------|----------|-------------------|
File                 |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
---------------------|----------|----------|----------|----------|-------------------|
All files            |    57.09 |    43.55 |    62.39 |    56.64 |                   |
 node_modules/svelte |    52.94 |    38.46 |    61.11 |    53.25 |                   |
  shared.js          |     84.4 |       56 |       80 |    87.37 |... 1015,1043,1048 |
  store.js           |     8.97 |     7.14 |     7.14 |     9.46 |... 65,166,168,170 |
 src                 |    86.25 |    68.48 |    82.05 |    89.17 |                   |
  config.js          |      100 |      100 |      100 |      100 |                   |
  events.js          |      100 |     87.5 |      100 |      100 |                11 |
  index.js           |    64.71 |    41.67 |    57.14 |       75 |          32,36,40 |
  methods.js         |    79.75 |     62.5 |    77.78 |    82.76 |... ,52,63,131,135 |
  registry.js        |      100 |     87.5 |      100 |      100 |          46,89,90 |
  util.js            |      100 |      100 |      100 |      100 |                   |
 src/components      |    30.61 |    19.23 |    18.75 |    31.58 |                   |
  Backdrop.svelte    |        0 |        0 |      100 |        0 |          13,14,16 |
  Box.svelte         |        0 |        0 |        0 |        0 |                   |
  Hotspot.svelte     |        0 |        0 |      100 |        0 |                37 |
  Modal.svelte       |    14.29 |        0 |        0 |    16.67 |     5,22,24,29,39 |
  Tooltip.svelte     |    83.33 |       75 |        0 |       80 |                41 |
  index.js           |    78.57 |      100 |       50 |    76.92 |          25,31,37 |
  tour.js            |    24.14 |    10.17 |        0 |    25.71 |... 10,213,214,218 |
---------------------|----------|----------|----------|----------|-------------------|

@henriquemotaesteves
Copy link
Contributor Author

@c0bra, instead of define your exclude rule inside package.json, try create a .istanbul.yml file with the following content:

instrumentation:
  excludes: ['**/node_modules/**']

@c0bra
Copy link

c0bra commented Jan 22, 2019

@henriquemotaesteves same result. I removed the "nyc" config from package.json as well, just in case.

@henriquemotaesteves
Copy link
Contributor Author

@c0bra, what test runner are you using? Is your project public?

I'm using jest and I suppose that it does not pass the nyc config section in package.json to istanbul. This is why I used a .istanbul.yml file.

@c0bra
Copy link

c0bra commented Jan 22, 2019

@henriquemotaesteves I'm using ava, project is here: https://github.com/onboardist/ui

Let me dig and see if I can figure out where it's not being set.

@henriquemotaesteves
Copy link
Contributor Author

henriquemotaesteves commented Jan 22, 2019

@c0bra, I did not found any reference to istanbul-api in nyc. If this is true, the solution provided by the pull request istanbuljs/istanbuljs#275 will not work when nycis used.

@c0bra
Copy link

c0bra commented Jan 22, 2019

@henriquemotaesteves well that stinks. I suppose there's no way around that if nyc is using lower-level access to istanbul.

@ssube
Copy link

ssube commented Jan 22, 2019

Followed the steps in #956 (comment) to link nyc, then ran my project's coverage target again (https://github.com/ssube/noicejs/blob/feature/nyc-filter/Makefile#L104). I'm not seeing a difference in the report either, likely thanks to running nyc mocha tests from the CLI.

image

@henriquemotaesteves
Copy link
Contributor Author

@c0bra the change made in the istanbul-api (istanbuljs/istanbuljs#275) is now avaiable for nyc users (#982).

@JaKXz
Copy link
Member

JaKXz commented Feb 20, 2019

Yeah feel free to test out nyc#master, let us know if this is fixed for you! @coreyfarrell will probably do a release sometime soon and close this issue with it.

@coreyfarrell
Copy link
Member

We've run into some issues with the updated code not honoring negated excludes. I need to address this before a new release can be made. I will follow-up here when I have more information / a new version to test.

@c0bra
Copy link

c0bra commented Feb 25, 2019

I'll try to give master a shot soon.

@c0bra
Copy link

c0bra commented Feb 27, 2019

@henriquemotaesteves looks great! thanks very much

@coreyfarrell
Copy link
Member

@c0bra could you test again using #1010? I believe this will fix the issue without creating issues for negated excludes. You should be able to install that test version using:

npm i -D git://github.com/coreyfarrell/nyc.git#exclude-after-remap

@coreyfarrell
Copy link
Member

@c0bra Update, the PR I mentioned has been merged, so when you get a chance please test against the current git://github.com/istanbuljs/nyc#master.

@c0bra
Copy link

c0bra commented Mar 8, 2019

@coreyfarrell Seems like it's reverted back to old behavior unless I've done something wrong:

image

@coreyfarrell
Copy link
Member

@c0bra this is with https://github.com/onboardist/ui? Do you run npm i && npm run cover?

BTW I see a bunch of babel 6 dependencies, is this used?

@c0bra
Copy link

c0bra commented Mar 8, 2019

@coreyfarrell yes, that library. I ran npm i -D git://github.com/coreyfarrell/nyc.git#master before covering, yes.

The babel deps are probably leftovers from earlier. I removed and pruned them with no change.

Also I've tried using --exclude-after-remap=**/node_modules/** as well as:

"nyc": {
    "exclude-after-remap": "**/node_modules/**"
  }

in package.json

And lastly I tried blowing away node_modules just in case, but not joy. Perhaps I've misconfigured something?

@coreyfarrell
Copy link
Member

exclude-after-remap is just a boolean and it's true by default, no need to configure it.

  1. Verify that your package.json lists "nyc": "git://github.com/istanbuljs/nyc.git#master" under devDependencies.
  2. Run rm -fr node_modules package-lock.json
  3. Run npm i && npm run cover

Following these steps I got the expected coverage results (node_modules were not listed).

@c0bra
Copy link

c0bra commented Mar 8, 2019

@coreyfarrell OK I tried a fresh checkout and it works, with those steps on my original checkout it's still providing the same result so clearly I've got something weird going on.

I'll try to figure it out. Thanks for the fix!

Update: got it. Not sure where I went wrong but it's working now!

@c0bra
Copy link

c0bra commented Mar 8, 2019

Oh, also maybe I misunderstand this (and it's probably unrelated), but in this screenshot my src/components directory has 0% function coverage while all files except 2 have 100% coverage. Is there a reason for this

image

@coreyfarrell
Copy link
Member

@c0bra Don't know but this is a completely different issue. Could you post a separate issue for that? Please be sure to link your repo to the new bug so we have all the information in one place.

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

Successfully merging a pull request may close this issue.

6 participants