[DNM] Revert "🧪 Run tests from sdist like downstreams" - #13425
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #13425 +/- ##
===========================================
- Coverage 98.42% 47.49% -50.93%
===========================================
Files 133 132 -1
Lines 49747 49432 -315
Branches 2628 2571 -57
===========================================
- Hits 48961 23478 -25483
- Misses 662 25327 +24665
- Partials 124 627 +503
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will not alter performance
Comparing Footnotes
|
|
Observations comparing https://app.codecov.io/gh/aio-libs/aiohttp/commit/4e6ca6cc2f06c8d1a3a609793c50445893e01e84 (the HEAD of this PR) vs https://app.codecov.io/gh/aio-libs/aiohttp/commit/356d355b7928ada4c2deb86e13992310b33d52f9 (the BASE of this PR), looking at the uploads from the same GHA job found by matching the same labels ( Looking into the XML itself, shows that whatever coveragepy produces includes the following entry in the Cobertura report of the BASE upload (pre-#13388): <class name="reader_c.py" filename="aiohttp/_websocket/reader_c.py" complexity="0" line-rate="0" branch-rate="0.01754">
<methods/>
<lines>
<line number="3" hits="0"/>
<line number="4" hits="0"/>
<line number="5" hits="0"/>
<line number="6" hits="0"/>
<line number="8" hits="0"/>
<line number="9" hits="0"/>
<line number="10" hits="0"/>
<line number="11" hits="0"/>
<line number="12" hits="0"/>
<line number="13" hits="0"/>
<line number="27" hits="0"/>
<line number="31" hits="0"/>
<line number="32" hits="0"/>
<line number="33" hits="0"/>
<line number="34" hits="0"/>
<line number="36" hits="0"/>
<line number="37" hits="0"/>
<line number="40" hits="0"/>
<line number="41" hits="0"/>
<line number="42" hits="0"/>
<line number="43" hits="0"/>
<line number="44" hits="0"/>
<line number="45" hits="0"/>
<line number="46" hits="0"/>
<line number="48" hits="0"/>
<line number="49" hits="0"/>
<line number="51" hits="0"/>
<line number="52" hits="0"/>
<line number="53" hits="0"/>
<line number="55" hits="0"/>
<line number="57" hits="0"/>
<line number="60" hits="0"/>
<line number="66" hits="0"/>
<line number="69" hits="0"/>
<line number="70" hits="0"/>
<line number="71" hits="0"/>
<line number="72" hits="0"/>
<line number="73" hits="0"/>
<line number="74" hits="0"/>
<line number="75" hits="0"/>
<line number="76" hits="0"/>
<line number="77" hits="0"/>
<line number="78" hits="0"/>
<line number="80" hits="0"/>
<line number="81" hits="0"/>
<line number="83" hits="0"/>
<line number="84" hits="0"/>
<line number="86" hits="0"/>
<line number="91" hits="0"/>
<line number="92" hits="0"/>
<line number="93" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="exit,94"/>
<line number="94" hits="0"/>
<line number="95" hits="0"/>
<line number="97" hits="0"/>
<line number="98" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="99,100"/>
<line number="99" hits="0"/>
<line number="100" hits="0"/>
<line number="101" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="exit,102"/>
<line number="102" hits="0"/>
<line number="104" hits="0"/>
<line number="105" hits="0"/>
<line number="106" hits="0"/>
<line number="107" hits="0"/>
<line number="109" hits="0"/>
<line number="110" hits="0"/>
<line number="111" hits="0"/>
<line number="112" hits="0"/>
<line number="113" hits="0"/>
<line number="114" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="exit,115"/>
<line number="115" hits="0"/>
<line number="117" hits="0"/>
<line number="118" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="119,126"/>
<line number="119" hits="0"/>
<line number="120" hits="0"/>
<line number="121" hits="0"/>
<line number="122" hits="0"/>
<line number="123" hits="0"/>
<line number="124" hits="0"/>
<line number="125" hits="0"/>
<line number="126" hits="0"/>
<line number="128" hits="0"/>
<line number="129" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="130,136"/>
<line number="130" hits="0"/>
<line number="131" hits="0"/>
<line number="132" hits="0"/>
<line number="133" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="134,135"/>
<line number="134" hits="0"/>
<line number="135" hits="0"/>
<line number="136" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="137,138"/>
<line number="137" hits="0"/>
<line number="138" hits="0"/>
<line number="141" hits="0"/>
<line number="142" hits="0"/>
<line number="149" hits="0"/>
<line number="150" hits="0"/>
<line number="151" hits="0"/>
<line number="153" hits="0"/>
<line number="154" hits="0"/>
<line number="155" hits="0"/>
<line number="157" hits="0"/>
<line number="158" hits="0"/>
<line number="159" hits="0"/>
<line number="160" hits="0"/>
<line number="163" hits="0"/>
<line number="164" hits="0"/>
<line number="166" hits="0"/>
<line number="167" hits="0"/>
<line number="168" hits="0"/>
<line number="169" hits="0"/>
<line number="170" hits="0"/>
<line number="171" hits="0"/>
<line number="172" hits="0"/>
<line number="173" hits="0"/>
<line number="175" hits="0"/>
<line number="176" hits="0"/>
<line number="181" hits="0"/>
<line number="182" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="183,185"/>
<line number="183" hits="0"/>
<line number="185" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="186,188"/>
<line number="186" hits="0"/>
<line number="188" hits="0"/>
<line number="189" hits="0"/>
<line number="190" hits="0"/>
<line number="191" hits="0"/>
<line number="192" hits="0"/>
<line number="193" hits="0"/>
<line number="195" hits="0"/>
<line number="197" hits="0"/>
<line number="205" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="207,301"/>
<line number="207" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="208,214"/>
<line number="208" hits="0"/>
<line number="214" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="216,221"/>
<line number="216" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="217,218"/>
<line number="217" hits="0"/>
<line number="218" hits="0"/>
<line number="219" hits="0"/>
<line number="221" hits="0"/>
<line number="222" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="223,227"/>
<line number="223" hits="0"/>
<line number="224" hits="0"/>
<line number="227" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="228,235"/>
<line number="228" hits="0"/>
<line number="235" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="236,239"/>
<line number="236" hits="0"/>
<line number="237" hits="0"/>
<line number="239" hits="0"/>
<line number="243" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="244,270"/>
<line number="244" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="245,251"/>
<line number="245" hits="0"/>
<line number="251" hits="0"/>
<line number="252" hits="0"/>
<line number="260" hits="0"/>
<line number="261" hits="0"/>
<line number="265" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="266,275"/>
<line number="266" hits="0"/>
<line number="270" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="271,273"/>
<line number="271" hits="0"/>
<line number="273" hits="0"/>
<line number="275" hits="0"/>
<line number="276" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="277,296"/>
<line number="277" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="278,292"/>
<line number="278" hits="0"/>
<line number="279" hits="0"/>
<line number="280" hits="0"/>
<line number="281" hits="0"/>
<line number="289" hits="0"/>
<line number="292" hits="0"/>
<line number="296" hits="0"/>
<line number="300" hits="0"/>
<line number="301" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="302,331"/>
<line number="302" hits="0"/>
<line number="303" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="304,322"/>
<line number="304" hits="0"/>
<line number="306" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="309,313"/>
<line number="309" hits="0"/>
<line number="313" hits="0"/>
<line number="314" hits="0"/>
<line number="315" hits="0"/>
<line number="316" hits="0"/>
<line number="319" hits="0"/>
<line number="322" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="323,328"/>
<line number="323" hits="0"/>
<line number="328" hits="0"/>
<line number="330" hits="0"/>
<line number="331" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="332,335"/>
<line number="332" hits="0"/>
<line number="335" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="336,340"/>
<line number="336" hits="0"/>
<line number="340" hits="0"/>
<line number="344" hits="0"/>
<line number="346" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="347,349"/>
<line number="347" hits="0"/>
<line number="349" hits="0"/>
<line number="350" hits="0"/>
<line number="351" hits="0"/>
<line number="353" hits="0"/>
<line number="355" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="356,444"/>
<line number="356" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="357,358"/>
<line number="357" hits="0"/>
<line number="358" hits="0"/>
<line number="359" hits="0"/>
<line number="360" hits="0"/>
<line number="362" hits="0"/>
<line number="363" hits="0"/>
<line number="364" hits="0"/>
<line number="365" hits="0"/>
<line number="366" hits="0"/>
<line number="378" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="379,384"/>
<line number="379" hits="0"/>
<line number="384" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="392,397"/>
<line number="392" hits="0"/>
<line number="397" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="398,403"/>
<line number="398" hits="0"/>
<line number="403" hits="0"/>
<line number="404" hits="0"/>
<line number="408" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="409,419"/>
<line number="409" hits="0"/>
<line number="419" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="420,429"/>
<line number="420" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="421,438"/>
<line number="421" hits="0"/>
<line number="429" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="430,431"/>
<line number="430" hits="0"/>
<line number="431" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="432,436"/>
<line number="432" hits="0"/>
<line number="436" hits="0"/>
<line number="438" hits="0"/>
<line number="439" hits="0"/>
<line number="440" hits="0"/>
<line number="441" hits="0"/>
<line number="444" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="445,480"/>
<line number="445" hits="0"/>
<line number="446" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="447,453"/>
<line number="447" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="448,449"/>
<line number="448" hits="0"/>
<line number="449" hits="0"/>
<line number="450" hits="0"/>
<line number="451" hits="0"/>
<line number="452" hits="0"/>
<line number="453" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="454,459"/>
<line number="454" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="455,456"/>
<line number="455" hits="0"/>
<line number="456" hits="0"/>
<line number="457" hits="0"/>
<line number="459" hits="0"/>
<line number="464" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="469,477"/>
<line number="469" hits="0"/>
<line number="470" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="471,477"/>
<line number="471" hits="0"/>
<line number="477" hits="0"/>
<line number="480" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="481,487"/>
<line number="481" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="482,483"/>
<line number="482" hits="0"/>
<line number="483" hits="0"/>
<line number="484" hits="0"/>
<line number="485" hits="0"/>
<line number="487" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="353,488"/>
<line number="488" hits="0"/>
<line number="489" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="490,493"/>
<line number="490" hits="0"/>
<line number="491" hits="0"/>
<line number="493" hits="0"/>
<line number="494" hits="0"/>
<line number="496" hits="0"/>
<line number="497" hits="0"/>
<line number="498" hits="0"/>
<line number="499" hits="0"/>
<line number="501" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="504,514"/>
<line number="504" hits="0"/>
<line number="505" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="510,511"/>
<line number="510" hits="0"/>
<line number="511" hits="0"/>
<line number="514" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="516,525"/>
<line number="516" hits="0"/>
<line number="517" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="518,523"/>
<line number="518" hits="0"/>
<line number="519" hits="0"/>
<line number="520" hits="0"/>
<line number="521" hits="0"/>
<line number="523" hits="0"/>
<line number="524" hits="0"/>
<line number="525" hits="0" branch="true" condition-coverage="0% (0/2)" missing-branches="526,536"/>
<line number="526" hits="0"/>
<line number="527" hits="0"/>
<line number="528" hits="0" branch="true" condition-coverage="100% (2/2)"/>
<line number="532" hits="0"/>
<line number="533" hits="0"/>
<line number="534" hits="0"/>
<line number="536" hits="0"/>
<line number="538" hits="0"/>
<line number="541" hits="0"/>
<line number="542" hits="0"/>
<line number="545" hits="0"/>
</lines>
</class> |
|
I wonder if this might be due to |
c8ce171 to
7cb97a0
Compare
|
@Dreamsorcerer here's a minimal change that causes coveragepy to notice diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml
index 72b287e47..611770cf9 100644
--- a/.github/workflows/ci-cd.yml
+++ b/.github/workflows/ci-cd.yml
@@ -214,6 +214,14 @@ jobs:
uses: actions/checkout@v7
with:
submodules: true
+ - name: DEBUG:unsymlink-aiohttp._websocket.reader_c
+ run: |
+ set -x
+ ls -lah aiohttp/_websocket/reader_c.py aiohttp/_websocket/reader_py.py
+ rm -fv aiohttp/_websocket/reader_c.py
+ cp -v aiohttp/_websocket/reader_py.py aiohttp/_websocket/reader_c.py
+ ls -slah aiohttp/_websocket/reader_c.py aiohttp/_websocket/reader_py.py
+ shell: bash
- name: Setup Python ${{ matrix.pyver }}
id: python-install
# important: do not use system python |
|
Looks like @bdraco was worried about the symlink since its initial addition in: #9543 (comment). The |
f63865a to
491bedd
Compare
|
So, we're now testing inside the wheels, so we get the duplicated file now? I guess the main question is why it doesn't get any coverage though. |
|
@Dreamsorcerer well, that is the mystery I'd very much like to solve on this quest. I suspect it might have something to do with coveragepy assigning the executed code coverage to the wrong file (symlink vs non-symlink), but that's a speculation at this point. Claude Code was pointing fingers in the same direction earlier implying there's a bug in coveragepy but I resisted the urge to jump to conclusions before checking a few other things I wanted to understand better. I'd also be interested in what @bdraco thinks... cc @rodrigobnogueira you were looking into coveragepy in other projects lately. This one is an especially obscure corner case — if you're interested — feel free to join the search party and help us understand how it handles the module in question and what can we do about it. P.S. @Dreamsorcerer we sort of test from wheels, yes 📢 PSA: I may not have much time this week. But I think that |
This may show us what happens if regular wheels are built in CI instead of stuffing C-exts in-tree.
f7a9a68 to
573d495
Compare
If nothing else, we can just exclude that file? It looks to me like Cython coverage is being reported on the original reader_py.py. If we want to continue tracking it on the same file like that, then we can also drop the no branch comment, as it is covered now. |
|
@Dreamsorcerer yeah, I was considering |
Reverts #13388
DO NOT MERGE
Caution
THIS IS AN EXPLORATORY PR FOR GIT BISECT ADVANTURES