Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add coverage report #107

Merged
merged 12 commits into from
Dec 24, 2017
Merged

chore: add coverage report #107

merged 12 commits into from
Dec 24, 2017

Conversation

raszi
Copy link
Owner

@raszi raszi commented Nov 27, 2016

No description provided.

@raszi raszi mentioned this pull request Dec 14, 2016
@silkentrance
Copy link
Collaborator

@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
Copy link
Collaborator

👍 Nice work.

@silkentrance
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Collaborator

silkentrance commented Dec 7, 2017

It seems as if mocha has nothing to do with that issue:
gotwarlost/istanbul#90 (comment)

@silkentrance silkentrance force-pushed the add-coverage branch 2 times, most recently from 1198ad3 to 87b38b0 Compare December 8, 2017 21:23
@silkentrance
Copy link
Collaborator

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 silkentrance force-pushed the add-coverage branch 2 times, most recently from 302c782 to 6e30849 Compare December 9, 2017 15:44
@silkentrance
Copy link
Collaborator

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
Copy link
Collaborator

silkentrance commented Dec 9, 2017

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 😱

@raszi raszi merged commit bbdbe16 into master Dec 24, 2017
@raszi raszi deleted the add-coverage branch December 25, 2017 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants