-
-
Notifications
You must be signed in to change notification settings - Fork 359
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
Instrumenter should ignore excluded files and folders #890
Comments
I think I am seeing the same issue. I am trying to |
Bump, stale bot |
Expected Behavior
The instrumenter should ignore files specified in the list of exclude globs
$ nyc instrument content instrumented
with .nycrc
Observed Behavior
The instrumenter ignores files specified in the defaultExclude instead, (istanbuljs/packages/test-exclude/index.js)
Bonus Points! Code (or Repository) that Reproduces Issue
https://github.com/AndrewFinlay/nyc-instrument-exclude-example
Forensic Information
Operating System: macOS High Sierra
Environment Information:
node v8.11.1
npm v6.1.0
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]
After doing some digging it seems that the nyc instrumenter simply isn't using the include/exclude parameters from the config. I'm pretty certain that it's the same issue referenced in #596, and related to #573
When NYC is configured in "commands/instrument.js" it excludes any configuration setup in "config-util.js". This means that it will fallback onto the defaults set in "istanbul/test-exclude/index.js".
The good news is it looks like a pretty easy fix, the bad news is I'm assuming that specifying exclude/include paths in the instrument code is desired behaviour. So I have no idea what would break by changing this. It may be best to play it safe and add a pair of new config options, instrumentInclude and instrumentExclude.
Let me know what you think of this PR #893.
The text was updated successfully, but these errors were encountered: