get and enforce 100% coverage#3159
Conversation
graingert
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3159 +/- ##
====================================================
+ Coverage 99.62461% 100.00000% +0.37538%
====================================================
Files 124 124
Lines 18381 18427 +46
Branches 1226 1215 -11
====================================================
+ Hits 18312 18427 +115
+ Misses 47 0 -47
+ Partials 22 0 -22
🚀 New features to boost your workflow:
|
CoolCat467
left a comment
CoolCat467
left a comment
There was a problem hiding this comment.
Didn't see anything imminently except for a mypy issue
src/trio/_core/_tests/test_io.py
Outdated
| check(expected_readers=1, expected_writers=0) | ||
|
|
||
|
|
||
| @pytest.mark.skipif(sys.platform in {"win32", "linux"}, reason="no kqueue") |
There was a problem hiding this comment.
From mypy run annotations, looks like you need to inform mypy that you don't expect this function to run on windows or linux, because mypy doesn't understand pytest.mark.skipif.
I suggest something like this:
if sys.platform == "win32" or sys.platform == "linux":
raise AssertionError("Should be unreachable")
There was a problem hiding this comment.
Or better yet assert sys.platform not in {"win32", "linux"}
There was a problem hiding this comment.
I couldn't get either option working (assert sys.platform != "win32" and sys.platform != "linux") does work in the playground, and if I create a new file.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
This reverts commit be08460.
| project: | ||
| default: | ||
| target: 100% |
There was a problem hiding this comment.
FTR with #3162, there's going to be a need to split the checks into MyPy vs. pytest.
| "abc.abstractmethod", | ||
| "if TYPE_CHECKING.*:", | ||
| "if _t.TYPE_CHECKING:", | ||
| "if t.TYPE_CHECKING:", | ||
| "@overload", | ||
| 'class .*\bProtocol\b.*\):', | ||
| "raise NotImplementedError", | ||
| 'TODO: test this line' |
There was a problem hiding this comment.
Consider integrating the covdefaults plugin, as it injects most of these automatically. The only thing you'd need to override additionally is the fail_under setting, which it sets to 100%, but it's not compatible with runtime-dependent tests that cannot reach 100% under one specific runtime. So it's good to set it to something sufficiently low while requiring 100% in Codecov.
There was a problem hiding this comment.
ah nice, I'll do that in a followup
jakkdl
left a comment
jakkdl
left a comment
There was a problem hiding this comment.
oops, I forgot to submit my pending comments I wrote yesterday
Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>
for more information, see https://pre-commit.ci
Co-authored-by: John Litborn <11260241+jakkdl@users.noreply.github.com>
4 participants
Using a combination of more tests and TODO comments that work as coverage pragmas