bcoe / c8

output coverage reports using Node.js' built in coverage
ISC License
2k stars 91 forks source link

feat: ignore files outside of the project root by default?feat: #110

Open eight04 opened 5 years ago

eight04 commented 5 years ago

I have some JavaScript configuration files that are dynamically generated during tests. It seems that those generated JS files are also counted as part of the source code. These config files are generated inside a temporary directory created by tmp, and they will be require()ed during the test.

Here is a simple repro, run npm test to see the error. It seems that this problem is Windows-only. At least, Travis doesn't show this error:


D:\Dev\node-test>npm test

> node-test@1.0.1 test D:\Dev\node-test
> c8 --reporter lcov node index

file: D:\Dev\node-test\config.js error: Error: ENOENT: no such file or directory
, open 'D:\Dev\node-test\config.js'
    at Object.openSync (fs.js:454:3)
    at readFileSync (fs.js:354:35)
    at V8ToIstanbul.load (D:\Dev\node-test\node_modules\v8-to-istanbul\lib\v8-to
-istanbul.js:30:23)
    at Report.getCoverageMapFromAllCoverageFiles (D:\Dev\node-test\node_modules\
c8\lib\report.js:65:25)
    at Report.run (D:\Dev\node-test\node_modules\c8\lib\report.js:37:28)
    at exports.outputReport (D:\Dev\node-test\node_modules\c8\lib\commands\repor
t.js:24:16)
    at D:\Dev\node-test\node_modules\c8\bin\c8.js:31:13
    at ChildProcess.<anonymous> (D:\Dev\node-test\node_modules\foreground-child\
index.js:52:5)
    at ChildProcess.emit (events.js:196:13)
    at ChildProcess.cp.emit (D:\Dev\node-test\node_modules\cross-spawn\lib\enoen
t.js:40:29)

So:

  1. Is there a way to exclude a path/pattern from being processed by c8?
  2. Can we ignore files outside of the project root by default?
bcoe commented 5 years ago
  1. you should be able to exclude these files today using the --exclude parameter, or by setting an exclude field in a .c8rc.
  2. this seems like a reasonable feature request, any interest in helping out with it?
eight04 commented 5 years ago

Sure. How can I help?

bcoe commented 5 years ago

@eight04 we have similar functionality to this in the library nyc, I think we would modify the logic here:

https://github.com/bcoe/c8/blob/master/lib/report.js#L164

To have similar logic to nyc, in which it detects a path is outside the current working directory and excludes it.

eight04 commented 5 years ago

After digging into the code, I think it is a test-exclude bug. test-exclude does exclude files outside of CWD, but it doesn't work if two paths have different drives: https://github.com/istanbuljs/istanbuljs/blob/c4e8b8e640acba43ecbe26ca3032a951de3ac3a6/packages/test-exclude/index.js#L96

const cwd = "D:\\dev\\project";
const filename = "C:\\Users\\Owner\\AppData\\Local\\Temp\\config.js";
path.relative(cwd, filename); // "C:\\Users\\...."

This also explains why this bug only occurs on Windows.

eight04 commented 5 years ago

https://github.com/istanbuljs/istanbuljs/issues/418

bcoe commented 5 years ago

@eight04 would we be able to pull in a similar fix to the work done in istanbul do you think?

eight04 commented 5 years ago

I think this will be fixed automatically when https://github.com/istanbuljs/istanbuljs/pull/422 is released.