pick up all files for coverage reporting

[scottty881]

Previously coverage reporting was only done for js files that had an associated test file. This change allows for coverage reporting on all js files(except for src/index.js), even if there is no test.

New example coverage output(auto.js is a new file with no tests):

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

[mackbyte]

Out of interest why have you excluded src/index.js ?

coveragePathIgnorePatterns: ['src/index.js', '/node_modules/', '/vendor/'],
collectCoverageFrom: ['src/*/.{js,jsx}']

It also seems that since the includes are ['src/**/*.{js,jsx}'] which starts at src then the exclusion of node_modules and vendor are not needed. However it does make it obvious.

Maybe just omit coveragePathIgnorePatterns ?

[scottty881]

@pmackcode I agree ideally index.js would also be included, any thoughts on what a test for it would look like?

[mackbyte]

@scottty881 I'm not sure there would be one for specifically for create-react-app as index.js is fairly minimal as you would expect. I personally use it for wiring together things, for example I am using redux and this is where I create my store and apply middleware. I'm fairly new to this so not sure if thats the best place but since its fairly central to (I assume) most applications it would make sense to have test coverage of it. Maybe not at a unit level but at least integration / end to end.

To summarise I don't see enough reason to exclude it even if there is not a clear cut reason to include it. Does that make sense ?

[jarlah]

i agree with @pmackcode, if there was no excludes for index.js before, we shouldn't exclude it now. The exclude block should be removed if possible.

I am using the script unejected. So I really need this fix. Also, I tried to find the place to edit after doing a test eject of the scripts, but its not ejected. So I cannot fix this by ejecting. Possibly there are still some dependencies after ejecting.

@scottty881 can you look at this? Or should I make my own PR trying to fix this (without the exclude block) ?

We are using this for a semi large project now and the test coverage is not correct, at all. If we have all green tests covering everything, test coverage is 100% even if there exist code without tests.

[mackbyte]

I submitted my own pull request for this above as I am keen to get this introduced. Take a look at #961

Edit: Looks this has been amended whilst I was uploading mine, I guess whichever gets merged we can just close the other. Apologies.

[scottty881]

@pmackcode i also updated mine, whichever works

[gaearon]

I'm really sorry @scottty881, I missed your PR and already merged #961.

Can you help me understand why exclusion is necessary? Do you think #961 is enough to cover your use case?

Again, apologies for missing your earlier PR.

[scottty881]

@gaearon no worries, #961 is great. Was going for explicitly declaring exclusion since it was a feature. Thanks for gettting #961 merged!