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

Could I place exclude in .nycrc instead of package.json? #419

Closed
BurningLutz opened this issue Oct 19, 2016 · 12 comments
Closed

Could I place exclude in .nycrc instead of package.json? #419

BurningLutz opened this issue Oct 19, 2016 · 12 comments
Labels

Comments

@BurningLutz
Copy link

Currently, setting exclude in .nycrc takes no effect but setting it in package.json did.

My nyc version is: 8.3.1

@bcoe bcoe added the bug label Oct 31, 2016
@rapzo
Copy link
Contributor

rapzo commented Nov 9, 2016

Is this still an issue?
I probed it in a really small test case and couldn't verify the issue...

The project is here: https://github.com/rapzo/test-nyc

There are two tags: with-exclude and without-exclude and checking, for example, the html coverage there's the result of having debug.js with missing coverage (for the first case) and coverage hitting 100% when excluding the aforementioned file.

Am i missing something here?

@BurningLutz
Copy link
Author

BurningLutz commented Nov 11, 2016

@rapzo After I upgrade to version 8.4.0, the issue seems......to remain.

I'm using babel-plugin-istanbul for ES6+ support. Here are my related configs:

.babelrc:

{
  "presets": ["es2015", "stage-3", "react"],
  "env": {
    "test": {
      "plugins": [
        "istanbul"
        ]
    }
  }
}

.nycrc

{
  "exclude": [
    "src/test/**/*.spec.js"
  ],
  "reporter": [
    "text",
    "html"
  ],
  "require": [
    "babel-register"
  ],
  "sourceMap": false,
  "instrument": false
}

And I test codes in this way:

NODE_ENV=test nyc mocha ./src/test/**/*.spec.js --recursive --reporter nyan

@rapzo
Copy link
Contributor

rapzo commented Nov 16, 2016

Thanks for that info @BurningLutz it really cleared the issue here.

Although, from a quick look at the code it looks like an issue in babel-plugin-istanbuland not in nyc.
Here: https://github.com/istanbuljs/babel-plugin-istanbul/blob/master/src/index.js#L26
It looks like it only parses configuration from package.json.

Meanwhile i'll try to write a patch to support .nycrc configuration but we need @bcoe's insight on this cause it may be not an issue at all but a design decision.

@bcoe
Copy link
Member

bcoe commented Nov 16, 2016

@rapzo @BurningLutz an oversight not a design decision; I would happily accept a patch adding .nycrc support to babel-plugin-istanbul.

@rapzo
Copy link
Contributor

rapzo commented Nov 16, 2016

Nice!

As i see it, do you have any preference on the parsing order? 1st .nycrc or
package.json, or the other way around? How about if there are both? Which
takes precedence?

On Wed, Nov 16, 2016 at 4:25 PM, Benjamin E. Coe notifications@github.com
wrote:

@rapzo https://github.com/rapzo @BurningLutz
https://github.com/BurningLutz an oversight not a design decision; I
would happily accept a patch adding .nycrc support to
babel-plugin-istanbul.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#419 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAJBTLgH7rrG_KW1WScYmdrZGFQErzQiks5q-y54gaJpZM4Kaxr1
.

Rui Pedro Lima

@bcoe
Copy link
Member

bcoe commented Nov 16, 2016

@rapzo let's just make sure that nyc and babel-plugin-istanbul are consistent; nyc currently uses yargs' config/pkg-config functionality to do its thing:

https://github.com/istanbuljs/nyc/blob/master/lib/config-util.js#L14

We should probably do the same; there's been some discussion on yargs to change the order of application; And there might be some changes over the coming months, so best that we lean on yargs in my opinion, and let that project determine the most logical order of operation.

@rapzo
Copy link
Contributor

rapzo commented Nov 16, 2016

Great! Thanks! Will do my best.

On Wed, Nov 16, 2016 at 5:23 PM, Benjamin E. Coe notifications@github.com
wrote:

@rapzo https://github.com/rapzo let's just make sure that nyc and
babel-plugin-istanbul are consistent; nyc currently uses yargs'
config/pkg-config functionality to do it's thing:

https://github.com/istanbuljs/nyc/blob/master/lib/config-util.js#L14

We should probably do the same; there's been some discussion on yargs to
change the order of application; And there might be some changes over the
coming months, so best that we lean on yargs in my opinion, and let that
project determine the most logical order of operation.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#419 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAJBTAEX8iAIBvPScy2r9w4caT8I9-rzks5q-zwEgaJpZM4Kaxr1
.

Rui Pedro Lima

@bcoe
Copy link
Member

bcoe commented Feb 7, 2017

@BurningLutz @rapzo could you give this a try:

npm cache clear; npm i babel-plugin-istanbul@next

The bug should be fixed thanks to @alpersogukpinar's hard work.

@rapzo
Copy link
Contributor

rapzo commented Feb 7, 2017 via email

@BurningLutz
Copy link
Author

@bcoe @rapzo
It seems to work now :D. Thx!

@BurningLutz
Copy link
Author

Since the related issue istanbuljs/babel-plugin-istanbul#62 has been solved recently, I'm closing this now.

@bcoe
Copy link
Member

bcoe commented Feb 21, 2017

@BurningLutz awesome 👍

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

No branches or pull requests

3 participants