Strict typing #1046
Strict typing #1046
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
CodSpeed Performance ReportMerging #1046 will improve performances by 12.56%Comparing Summary
Benchmarks breakdown
|
Dreamsorcerer
force-pushed
the
strict-typing
branch
from
e27a0b7 to
4da95eb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1046 +/- ##
==========================================
+ Coverage 94.87% 97.53% +2.66%
==========================================
Files 23 22 -1
Lines 2671 2597 -74
Branches 250 376 +126
==========================================
- Hits 2534 2533 -1
+ Misses 67 36 -31
+ Partials 70 28 -42
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
psf-chronographer
bot
added
the
bot:chronographer:provided
| @@ -11,8 +11,10 @@ _site-packages-to-src-mapping = | |||
|
|
|||
| [report] | |||
| exclude_also = | |||
| pass | |||
| ^\s*@pytest\.mark\.xfail | |||
There was a problem hiding this comment.
@Dreamsorcerer why did you drop this one?
There was a problem hiding this comment.
We've seen cases on aiohttp in the past where an xfail test wasn't running at all and codecov highlighted the mistake. A change in coverage for an xfail test in a PR may also indicate a mistake (whether in the test or the PR changes).
There was a problem hiding this comment.
@Dreamsorcerer those cases in aiohttp weren't because of the marker. But because of the test names being borked. This exclusion brings stability to the coverage metrics.
There was a problem hiding this comment.
But because of the test names being borked.
But, coverage is what finds the issue and allows us to fix it. With no coverage on decorated functions, we have no way to detect those issues currently.
This exclusion brings stability to the coverage metrics.
xfail tests should still have stable coverage, otherwise those tests are probably written incorrectly, just as they should have stable results (otherwise xfail strict would fail our CI).
There was a problem hiding this comment.
But because of the test names being borked.
But, coverage is what finds the issue and allows us to fix it. With no coverage on decorated functions, we have no way to detect those issues currently.
It does and it finds. Just not in the things decorated with xfail marker. It's a special case that we shouldn't rely on to create incidental coverage in other places.
This exclusion brings stability to the coverage metrics.
xfail tests should still have stable coverage, otherwise those tests are probably written incorrectly, just as they should have stable results (otherwise xfail strict would fail our CI).
A portion of xfail would be flaky, interrupting the test functions at random places inside. So different number of lines would be covered.
And the rest of invocations might create stable coverage with like half of lines being red in such test functions. And this is bad because it prevents us from having a stable requirement of 100% on the tests dir.
|
It may be a good idea to do a beta release and test out the new typing on aiohttp. |
No description provided.