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

Disallow impossible values for fail_under #746

Merged
merged 1 commit into from Dec 24, 2018

Conversation

@miketheman
Copy link
Contributor

miketheman commented Dec 24, 2018

Since there's no way were likely to achieve greater than 100% code coverage,
disallow usage of any value above 100.

Resolves #743

Signed-off-by: Mike Fiedler miketheman@gmail.com

@miketheman

This comment has been minimized.

Copy link
Contributor Author

miketheman commented Dec 24, 2018

I tried to find a place to implement this other than a method that currently only returns a bool - but I didn't find anywhere that configs are validated, so I hope this is ok.

Very open to feedback here!

@nedbat

This comment has been minimized.

Copy link
Owner

nedbat commented Dec 24, 2018

Thanks! I took a quick look earlier, and I think I would have put the check in the same place you did.

I wouldn't make the message so flip :) Feel free to add yourself to the CONTRIBUTORS file. :)

@miketheman miketheman force-pushed the miketheman:miketheman/more_than_100 branch from 21c91a9 to bb58942 Dec 24, 2018
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 24, 2018

Codecov Report

Merging #746 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #746      +/-   ##
==========================================
- Coverage   89.94%   89.92%   -0.02%     
==========================================
  Files          78       78              
  Lines       10799    10804       +5     
  Branches     1116     1117       +1     
==========================================
+ Hits         9713     9716       +3     
- Misses        959      960       +1     
- Partials      127      128       +1
Impacted Files Coverage Δ
tests/test_results.py 100% <100%> (ø) ⬆️
coverage/results.py 95.91% <100%> (+0.05%) ⬆️
tests/test_summary.py 97.01% <0%> (-0.5%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce9194a...09da2fc. Read the comment docs.

@miketheman

This comment has been minimized.

Copy link
Contributor Author

miketheman commented Dec 24, 2018

@nedbat I updated the error message to be a little less... opinionated. 😀

Codecov failure appears once I force-pushed the branch, it should probably re-report at some later time?

tests/test_results.py Outdated Show resolved Hide resolved
Since there's no way were likely to achieve greater than 100% code coverage,
disallow usage of any value above 100.

Resolves #743

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman miketheman force-pushed the miketheman:miketheman/more_than_100 branch from bb58942 to 09da2fc Dec 24, 2018
@nedbat

This comment has been minimized.

Copy link
Owner

nedbat commented Dec 24, 2018

Thanks!

@nedbat nedbat merged commit 9f29efd into nedbat:master Dec 24, 2018
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
Travis CI - Pull Request Build Passed
Details
@miketheman miketheman deleted the miketheman:miketheman/more_than_100 branch Dec 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.