istanbuljs / nyc

the Istanbul command line interface
https://istanbul.js.org/
ISC License
5.6k stars 359 forks source link

folders created by nyc #605

Open dkern opened 7 years ago

dkern commented 7 years ago

I've notices two things about the folders being created by nyc:

  1. When executing a simple test with nyc mocha a ./coverage folder will be created, but without any stored file in it. If there is no output there should be simply no folder created!

  2. Since switching from istanbul to nyc there are additional files inside the ./.nyc_output folder. They seems to be needed to use for things like nyc report. But why the heck is there a need of another folder in the rood directory? You already create the ./coverage folder. So if there is any need of files for code coverage report it should be placed in that folder. Maybe like ./coverage/nyc_output. Even better for .gitignore and old users of isanbul.

bcoe commented 7 years ago

@eisbehr- this seems like a reasonable request; I think for starters I'd like to only create the coverage folder when required; I'm a little hesitant to move the .nyc_output folder into ./coverage, mainly because I don't want the report generation logic to interfere with the low-level functionality of the .nyc_output folder -- this folder contains the raw coverage information from each subprocess that nyc stitches together test coverage information from.

Justus-Maier commented 7 years ago

+1 Default: ./coverage/.nyc_output @eisbehr-

Also, the options "temp-directory" and "report-dir" are inconsitently named and not well documented. (Backwards-compatible solution: allow "temp-dir" and "report-directory" aswell)

dkern commented 7 years ago

Yes, the documentation of these folder is pretty bad. Was searching for them too as I switched to the new version. No hint or anything.

I would still recommend to place this folder inside the ./coverage/, because it all belongs to the reports and the root dir becomes a mess. Starting to creade folders and folders in the root is not helpful for anyone. One folder, okay, but any folder more in the root dir belongs to a bad useage / implementation imo.

bcoe commented 7 years ago

@eisbehr- @Justus-Maier if someone wants to take a stab at implementing this change, I'd be interested to see how difficult it would be; mainly I'd want to make sure that the coverage/.nyc_output is not nuked between reporting runs -- there might need to be some changes to how reports are generated and how nyc runs to ensure this.

xPaw commented 6 years ago

I was looking at this as well, and decided to default temp directory to node_modules/.cache/nyc_output in our project.

Is there any reason not to do that by default in nyc?

I imagine this line: https://github.com/istanbuljs/nyc/blob/893345a193829802bee2c5047a78ad0addeb8b50/index.js#L44

could be changed to this:

  this._tempDirectory = config.tempDirectory || findCacheDir({name: 'nyc_output', cwd: this.cwd})
JaKXz commented 5 years ago

@xPaw I think your suggestion makes sense. I don't think there's a technical reason we can't do this right now so if you want to take a crack at a PR that would be great! If not, this is an easy one for any newcomers to the nyc codebase :)