istanbuljs / babel-plugin-istanbul

A babel plugin that adds istanbul instrumentation to ES6 code
BSD 3-Clause "New" or "Revised" License
630 stars 75 forks source link

support non-nyc ignore/exclude option #9

Closed mattfysh closed 8 years ago

mattfysh commented 8 years ago

allow non-nyc projects (such as those using karma) to also provide ignore/exclude options.

bcoe commented 8 years ago

@mattfysh no need to use nyc, we just currently look at the config option nyc in the package.json.

cesarp commented 8 years ago

I also was a bit confused because I thought that only applied to nyc until I tried.

It might be a good idea to put that it applies to karma and others?

bcoe commented 8 years ago

@mattfysh @cesarp agreed, I'm of two minds ... if I started instead reading the configuration istanbul it would be less confusing perhaps, but would also mean that nyc (the bin for running Istanbul directly) has a different configuration setting -- I think I'll patch read-pkg-conf and perhaps make it so it can read from several different config keys? then support both istanbul and nyc as options for where you place your config? what do you think? CC: @gotwarlost

mattfysh commented 8 years ago

given that nyc is just one of many different ways to trigger this plugin, I'd try to avoid any nomenclature that implies usage of any one particular method.

cesarp commented 8 years ago

Yeah I agree that it should have its own key to avoid any confusion or even better use babel plugin options.

MarkAPhillips commented 8 years ago

It would be great if you can update the example 'Ignoring Files' in the Readme as to how this can be done if you are using Karma or similar test runner. I am currently in the process of setting up the plugin in a React project and so far so good except the ignoring of files, which is a very common use case, and am unsure if i need the nyc dependency or not - thanks

MarkAPhillips commented 8 years ago

"nyc": { "exclude": [ "**/*.spec.js" ] needs to be added to package.json so does not need the additional nyc dependency installed via npm - I still think this could be made clearer in the Readme. I think adding the config within the package.json may confuse users unless it is clear thats this is whats needs to be done.

bcoe commented 8 years ago

please feel free to jump in on the conversation here:

https://github.com/istanbuljs/nyc/issues/320

insin commented 8 years ago

Would it be possible to add support for configuration via Babel plugin options, which are used instead of looking for a package.json if given?

{
  plugins: [
    ['babel-plugin-istanbul', {
      exclude: ['src/**/*.test.js']
    }]
  ]
}
bcoe commented 8 years ago

@MarkAPhillips @cesarp @mattfysh, how do folks feel about @insin's solution?

https://github.com/istanbuljs/babel-plugin-istanbul/pull/16

mattfysh commented 8 years ago

looks great!

MarkAPhillips commented 8 years ago

great - will both methods be still available or has the nyc in package.json been deprecated in favour of this? - what happens if you include the above and the nyc config option - does one take precedence over the other or are both exclusions valid - I would recommend once this is released this option is mentioned only in docs - with a reference to the nyc option for users upgrading as I assume many build process have implemented it this way - thanks once again!

insin commented 8 years ago

As currently implemented, if you pass Babel plugin config it won't do a package.json lookup at all.

cesarp commented 8 years ago

@bcoe yes that is better and actually what I suggested since it is a babel plugin just use the babel way of doing it.

bcoe commented 8 years ago

@cesarp @insin @mattfysh please give this a shot,

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

You should now be able to configure istanbul using:

{
  "env": {
    "test": {
      "plugins": [
        ["istanbul", {
          exclude: [
            "**/*.spec.js"
          ]
        }]
      ]
    }
  }
}
insin commented 8 years ago

Kicking the tyres on v1.1.0 while testing the upcoming version of nwb, using exclude config with some funky paths for Karma coverage and include config for Node.js coverage with nyc, working well.