raszi / node-tmp

Temporary file and directory creator for node.js
MIT License
737 stars 93 forks source link

chore: add coverage report #107

Closed raszi closed 6 years ago

silkentrance commented 6 years ago

@raszi The coverage seems off. I think that all spawned sub processes do not collect their data. The data from the sub processes must be merged with the other coverage data.

silkentrance commented 6 years ago

👍 Nice work.

silkentrance commented 6 years ago

I have prepared an additional commit for you to review and which still requires some more work. This will include all coverage data collected from the child processes that we spawn during the testing process. We need to add a few // istanbul ignore else comments to the code, but otherwise this is looking good and test coverage is also rather good. For now, I set .istanbul.yml checks/global to 100% and did not let the build fail in case that it these goals have not been reached.

Additionally, there are still some tests missing, if you look into the generated html report.

silkentrance commented 6 years ago

@raszi builds on appveyor are still failing due to an error in the _mocha.CMD. We might want to report this to the guys over at mocha.

silkentrance commented 6 years ago

It seems as if mocha has nothing to do with that issue: https://github.com/gotwarlost/istanbul/issues/90#issuecomment-22331096

silkentrance commented 6 years ago

Deployment of dependent packages seems to be different on appveyor. I need to run these tests on my windows machine and fix it there.

silkentrance commented 6 years ago

The coverage will now be gathered from child processes, too. This also works on windows. Changes had to be made to test/child-process.js that now detects whether it is run under istanbul.

The npm test script still needs some work, though.

silkentrance commented 6 years ago

npm test script fixed.

Statements   : 80.53% ( 153/190 )
Branches     : 79.7% ( 106/133 )
Functions    : 93.75% ( 30/32 )
Lines        : 81.97% ( 150/183 )

Could be better 😱