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

Percent rounding makes 99,995% cases pass the 100% threshold #593

Closed
jvanoostveen opened this issue Jan 11, 2021 · 2 comments
Closed

Percent rounding makes 99,995% cases pass the 100% threshold #593

jvanoostveen opened this issue Jan 11, 2021 · 2 comments

Comments

@jvanoostveen
Copy link
Contributor

We have a pre-commit hook and other checks to ensure 100% code coverage, but although the summary reports 100% coverage, when I take a look at the report output, I notice uncovered code.

=============================== Coverage summary ===============================
Statements   : 100% ( 42520/42520 )
Branches     : 100% ( 21031/21032 )    <-- left/right mismatch, but 100% is reported....
Functions    : 100% ( 9367/9367 )
Lines        : 100% ( 38434/38434 )
================================================================================

I tracked the issue to the following file:
https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-coverage/lib/percent.js

When I enter the numbers above, I get 100 as output, but this is incorrect. As there is no history for this file, I can't figure out why the tmp var is calculated as such.

Maybe for a quick/workable fix, have a configuration option for nyc to check for the actual numbers being equal instead of relying on the percentage.

@coreyfarrell
Copy link
Member

#452 moved the percent function from packages/istanbul-lib-coverage/lib/file.js to a new file but didn't change the code. Maybe the +5 should be removed from tmp = (1000 * 100 * covered) / total + 5? This way it would always round down.

I'm unlikely to have time to work on this for a while. Please modify the percent.js in your local node_modules to see if this resolves your issue (should cause it to calculate 99.99% branches). If so I would welcome a patch for this, please just make sure any testing issues are resolved (this might break some snapshot tests).

@jvanoostveen
Copy link
Contributor Author

I now used the workaround to have nyc check the limits 'per file' instead of the total.
I'll try to look into it and provide a patch if time allows, for now, the 'per file' workaround at least saves my team some frustration (as the files should not become so large, or we would have a completely different problem 😄).

jvanoostveen added a commit to jvanoostveen/istanbuljs that referenced this issue Jan 15, 2021
… to fix off by one errors when dealing with large numbers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants