tests: remove lax-no-cover pragmas by moving assertions before cancellation#2206
Merged
tests: remove lax-no-cover pragmas by moving assertions before cancellation#2206
Conversation
On Python 3.11 with coverage 7.10.7 (lowest-direct), statements immediately following an async with block whose __aexit__ processes cancellation are intermittently not traced. Both sse_client and streamable_http_client call tg.cancel_scope.cancel() in their exit paths, and test_streamable_http_client_resumption has an explicit in-test cancel, so 11 lines across 6 tests were carrying pragmas. All of these lines check state that is fully populated before the cancellation fires, so they can move inside the blocks: - on_session_created fires when the endpoint SSE event arrives, before sse_client yields to the caller - session IDs and protocol versions are captured during initialize(), so headers for phase-2 reconnects can be built inside phase 1 - the resumption test's while loop only exits after the notification has been appended, and the server tool blocks on a lock, so the count is stable before cancel - resumption tokens are all captured before send_request returns Also drops two dead if-truthy checks where the value was already asserted not-None three lines earlier.
The block-exit arcs from these three async with lines were never actually traced on 3.11 or 3.14 — on main, the arc destinations were lax-no-cover lines, so coverage silently excluded the arcs from counting. Removing those destinations in the previous commit exposed the latent gap. Adding no branch here is a smaller suppression than what we removed: the assertions that used to be lax-no-cover are now fully verified, and we only suppress branch-exit tracking on async with lines that have no actual branching. Matches the existing no branch on the phase-2 ClientSession lines in these same tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2200 — applies the same refactor to the 11 pre-existing pragmas of the same class.
Motivation
On Python 3.11 with coverage 7.10.7 (lowest-direct), statements immediately following an
async withblock whose__aexit__processes cancellation are intermittently not traced. Bothsse_clientandstreamable_http_clientcalltg.cancel_scope.cancel()on exit, so any plain statement after those blocks falls into this gap. 3.10 uses older bytecode, 3.12+ usessys.monitoring— only 3.11+lowest-direct is affected.#1897 and #1991 handled this by adding
# pragma: lax no coverto the affected lines. That works, but it means those assertions are never verified by coverage on any platform.Approach
Every one of these 11 lines checks state that is fully populated before cancellation fires, so they can simply move inside the
async with:on_session_createdcallbackendpointSSE eventsse_clientyieldsinitialize()responsemessage_handlerwhileloop only exits after append; server tool is blocked on a lock, so nothing else arriveson_resumption_token_updatesend_requestreturnsAlso drops two dead
if captured_session_id:branches that were always truthy (asserted not-None three lines earlier).Changes
tests/shared/test_sse.py: 3 pragmas removed across 2 teststests/shared/test_streamable_http.py: 8 pragmas removed across 4 testsAI Disclaimer