test(python-sdk): restore the stream-reader coverage dropped with the httpcore tests - #1706
test(python-sdk): restore the stream-reader coverage dropped with the httpcore tests#1706mishushakov wants to merge 1 commit into
Conversation
… httpcore tests #1690 deleted five httpcore-era tests from `test_file_stream_reader.py`. Three of them only ever asserted httpcore internals and are correctly gone, but two covered behavior that is still true — and still worth guarding — once re-anchored the same way the surviving tests were. Restores the abandoned-reader contract. `FileStreamReader` and `AsyncFileStreamReader` both document in their public docstrings that they have no garbage-collection safety net, and the deleted tests were that contract's only executable proof. They re-anchor on `response.is_closed` as cleanly as everything else in #1690 did: drop the reader half-consumed, collect, and assert the response stayed open. The async variant sleeps first so async-generator finalization gets a chance to run, making the assertion meaningful rather than a race the test happens to win. Restores the slow-consumer guard, on the real pyqwest transports. The idle read timeout is wire-only, so a consumer that stalls far longer than the bound must not abort the stream — the JS SDK asserts this in `connectionConfig.test.ts` and Python had no equivalent left. It lives in `test_volume_client.py` now, against actual streaming transports rather than a bare `httpx.Client()`, for both sync and async. Two details make that guard non-vacuous. The body is 2 MiB, because the socket and reqwest buffers together hold ~500 KiB (measured), so a stall at the quarter mark provably lands mid-transfer instead of after the whole body was read ahead. And a third test supplies the A/B: identical conditions plus an explicit `request_timeout` — a documented whole-transfer deadline — does abort the same slow consumer. Without it, a regression that turned the default idle bound into a wall-clock one would be indistinguishable from correct behavior. Also folds `_start_truncating_server` into `_start_chunked_server` as a `truncate_before` index, relaxes the truncated-body assertion to the `httpx.HTTPError` base (which concrete error a truncated body raises is the transport's business, not the reader's contract), renames the three surviving `..._releases_connection` tests to `..._releases_response` to match their assertions, and fixes the request-head read loop both files share: it inspected only the latest `recv` rather than accumulated bytes and spun forever on a closed peer. Test-only, so no changeset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit 06796b5. Bugbot is set up for automated code reviews on this repo. Configure here. |
Package ArtifactsBuilt from 517e2ef. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.41.1-restore-stream-reader-coverage.0.tgzCLI ( npm install ./e2b-cli-2.16.3-restore-stream-reader-coverage.0.tgzPython SDK ( pip install ./e2b-2.41.0+restore.stream.reader.coverage-py3-none-any.whl |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06796b5dea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| SLOW_CONSUMER_CHUNKS = 2048 | ||
| SLOW_CONSUMER_STALL_AT = SLOW_CONSUMER_CHUNKS // 4 |
There was a problem hiding this comment.
On systems where TCP autotuning and the transport buffers can hold this 2 MiB body, or where the server simply wins the race, the terminating chunk may already have been written before the consumer reaches the fixed 512 KiB threshold. Both positive tests would then sleep over an already-completed transfer and pass without exercising slow-consumer behavior; the separate sync deadline test cannot prove that either positive connection—especially the async one—was still active. Gate the pause on server-side progress/backpressure instead of assuming a fixed buffer capacity so these tests actually cover the intended path.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
|
/sdk claim |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claimed into #1710.
Commit 06796b5 was pushed to cursor/author-pr-claiming-d544 unmodified — identical tree, original author and Co-Authored-By trailers preserved. The branch was already current with main, so nothing was merged, and the description was carried over verbatim. Nothing here was reviewed or changed.
I have no way to close a pull request, so please close this one in favour of #1710.
Sent by Cursor Automation: /claim Claim SDK PR by SDK Factory
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only PR restoring dropped stream-reader coverage.
What was reviewed: the abandoned-reader tests (confirmed against the documented no-GC-safety-net contract in filesystem.py), the merged truncate_before chunked-server helper and the relaxed httpx.HTTPError assertion, the accumulated-read fix to the request-head loop, and the slow-consumer/request-timeout A/B in test_volume_client.py. The bug-hunting system's one candidate concern (buffer-size assumption behind the slow-consumer test's mid-transfer guarantee) was checked and ruled out as not portable-fragile enough to block.
Extended reasoning...
Overview
This PR touches only two test files (test_file_stream_reader.py, test_volume_client.py) in packages/python-sdk. It restores two tests deleted in a prior PR (#1690) that covered real behavior — the abandoned-reader GC contract and a slow-consumer/idle-timeout guard — re-anchored on the same non-httpcore-coupled assertions the rest of the suite uses. It also folds a duplicated truncating-server helper back into the main chunked-server helper, relaxes an over-specific exception assertion to the httpx.HTTPError base, renames tests for clarity, and fixes a request-head read loop that only inspected the latest recv() chunk instead of accumulating.
Security risks
None. No production code is touched — this is entirely test infrastructure for the Python SDK's streaming file/volume readers.
Level of scrutiny
Low-to-moderate is appropriate: test-only changes carry no runtime blast radius, but I still checked that the restored assertions are non-vacuous (e.g., the 2 MiB body size and quarter-mark stall for the slow-consumer test, and the async sleep-before-assert for GC finalization) since a test that can't fail is worse than no test. The reasoning in the PR description for each of these details is sound and matches what's in the diff.
Other factors
The bug-hunting system found no bugs, and its one candidate issue (a buffer-size assumption underlying the slow-consumer test's mid-transfer guarantee) was already investigated and refuted by the verifier pass. Verification output in the PR description (32 passed, full suite 267 passed, ruff/ty clean) is consistent with a mechanical, low-risk test restoration. No changeset is needed per repo convention for test-only changes, and there are no outstanding reviewer comments to address.
There was a problem hiding this comment.
The two restored contracts are real and mutation-verified, and every number in the description reproduces. One substantive finding: the slow-consumer tests are non-vacuous on an idle machine, but they degenerate 3 times out of 24 under CI-like load, and the reason the comment gives for the 2 MiB body isn't the reason it works.
This touches two test files and no public surface, so TASTE.md has nothing to grade here — and no changeset is needed, matching the test-only precedent (#1631, #1592, #1600).
Every claimed number reproduced on this VM (Python 3.12, 4 cores):
| claim | result |
|---|---|
32 passed for the two files |
32 passed in 10.25s; green on 3 more runs, and 3/3 under -n4 with CPU load |
267 passed / "262 before" |
267 at head, 262 at base 43c28b15f — exactly +5 |
ruff format 414 files unchanged, ruff check, ty check |
all three clean |
docstrings at filesystem.py:166 and :213 deny a GC safety net |
exact, both line numbers |
JS parity test at connectionConfig.test.ts:561 |
exact name and line |
The abandoned-reader tests have teeth. Giving the readers the safety net their docstrings deny (__del__ = close on FileStreamReader, plus a __del__ scheduling aclose() on the async one) fails exactly those two of the file's 11 tests and nothing else — the async one included, so the asyncio.sleep(0.05) really is giving finalization its chance rather than outracing the loop. They also aren't passing for a stray-reference reason: a weakref to the reader is dead after del + gc.collect() in both flavors, so the object genuinely was collected.
The finding is inline on the SLOW_CONSUMER_* block, with a paced-server alternative I measured passing 9/9 (4 of those under the load that breaks the current shape), failing under a wall-clock-deadline mutation, and asserting the mid-transfer property instead of assuming it. Two nits inline as well: the fixed _read_request_head is duplicated verbatim instead of shared, and the total-deadline A/B has no async counterpart even though the two flavors enforce the deadline through different code.
Sent by Cursor Automation: /check SDK complies with TASTE.md
|
|
||
| # A body far past the ~500 KiB the socket and reqwest buffers hold between | ||
| # them, so a stall a quarter of the way in provably lands mid-transfer with the | ||
| # server still pushing, rather than after the whole body was read ahead. |
There was a problem hiding this comment.
Confirming @chatgpt-codex-connector's P2 with measurements — and the mechanism turns out to be different from what both this comment and the PR description say.
I reproduced the fixture verbatim (the time.sleep(delay) call matters: even at 0.0 it yields the GIL) and recorded server progress at the instant the consumer stalls.
On an idle VM the tests are not vacuous: the server was at 512–586 of 2048 chunks with body_complete=False in 10/10 runs (5 sync, 5 async). But note 512 of 2048 — that is exactly the consumer's own read position, i.e. essentially zero read-ahead. What holds the invariant is that the server thread, yielding the GIL on every time.sleep(0), is no faster than the consumer. It is a scheduling coincidence, not buffer capacity.
The capacity claim is off by more than an order of magnitude, in the direction that removes the headroom. With the GIL yield removed, the client absorbed an entire 2 MiB body and an entire 8 MiB body before the consumer reached the quarter mark; on a 32 MiB body the server was 13 MiB ahead at the stall. So ~500 KiB is not a ceiling this 2 MiB body clears — the buffers here hold at least 8 MiB, four times the whole body. (The ~526 KiB you measured is presumably read-ahead against a consumer that has already stopped reading; before the stall the consumer is reading at full speed, which is what lets the server race ahead.)
Under CI conditions the property does break. python_sdk_tests.yml runs the suite with --numprocesses=4; with that plus CPU oversubscription, 3 of 24 runs had the stall land after the entire body, terminating chunk included. Those runs still pass — they just stop covering slow-consumer behavior. That matters more than it looks, because the wall-clock mutation this test was validated against (_get_request_timeout(None, None) returning 0.3) is also caught by the pre-existing test_{sync,async}_stream_survives_transfers_longer_than_read_timeout. The unique coverage here is the consumer-pace case, which needs a socket read still outstanding after the stall — precisely what the degenerate runs remove.
Verified alternative: pace the server rather than trying to out-size the buffers, and assert the property. The fixture already takes per-chunk delays, so [0.005] * 400 puts a hard 2s floor under the transfer, and a 0.9s stall at the quarter mark cannot land past it:
SLOW_CONSUMER_CHUNKS = 400
SLOW_CONSUMER_PACE = 0.005 # every gap far under the idle bound
SLOW_CONSUMER_STALL_AT = SLOW_CONSUMER_CHUNKS // 4
api_url, server = _start_volume_file_server([SLOW_CONSUMER_PACE] * SLOW_CONSUMER_CHUNKS)
...
if not stalled and received >= len(CHUNK) * SLOW_CONSUMER_STALL_AT:
stalled = True
# The stall has to land while the server is still pushing, or the case
# degenerates into re-reading an already-buffered body.
assert not server.body_complete
time.sleep(short_read_timeout * 3)Measured: 100/400 chunks at the stall in every run, both flavors, 9/9 green including 4 runs under the same load that produced the degenerate runs above; both fail under the wall-clock mutation; and the body drops from 2 MiB to 400 KiB. It needs the server helper to hand back a small progress object next to the URL — tests/envd_frame_server.py's SharedPoolServer.drop_when is the in-repo precedent for a test server the test synchronizes with rather than races.
Non-blocking on correctness, but the whole point of the PR is coverage that can't pass vacuously, and this is the one test here that currently can.
| CHUNK = b"x" * 1024 | ||
|
|
||
|
|
||
| def _read_request_head(conn) -> None: |
There was a problem hiding this comment.
Nit: this helper is byte-identical to the one added at test_file_stream_reader.py:30. The description's stated reason for fixing the loop at all is that #1690 copied it into a second helper — and the fix lands as a third and fourth copy rather than one shared one.
Sharing is already set up: pytest.ini sets pythonpath = tests with the comment "Makes shared test helpers (e.g. envd_frame_server) importable under importlib mode", and this file already imports transport_caches. A tests/http_server.py (or an addition to an existing helper module) would leave one place for the next person to fix.
| assert asyncio.run(run()) == (True, len(CHUNK) * SLOW_CONSUMER_CHUNKS) | ||
|
|
||
|
|
||
| def test_sync_stream_explicit_request_timeout_is_a_total_transfer_deadline( |
There was a problem hiding this comment.
Nit: the A/B counterpart exists only for sync, but the two flavors reach the deadline through different code — the adapter's async path re-applies the remaining deadline around each chunk read (AsyncIteratorByteStream), while the sync path relies on reqwest keeping the deadline attached to the in-flight request. So "a regression that turned the default idle bound into a wall-clock one would look indistinguishable from correct behavior" is only ruled out for one of the two mechanisms, while every other streaming case in this file is paired sync/async.
Smaller point in the same test: request_timeout=1.0 and time.sleep(2.0) are the two numbers that make it valid (the stall has to exceed the deadline), and unlike the SLOW_CONSUMER_* constants above they're unnamed, so the relationship between them is only in prose.
There was a problem hiding this comment.
Coverage is byte-identical on both sides of this PR, so the mutation results below are the report. Three of the five added tests are the sole guard of a contract; two mutants survive the whole suite, one of them in the exact place the PR's new tests were aiming.
The measurement
Base 43c28b15f (= the trigger's quoted base = merge-base = origin/main tip) → head 06796b5de, E2B_API_KEY set so the live suites really run.
| base | head | |
|---|---|---|
| python statements | 69.44% (10304/14838) | 69.44% (10303/14838) |
| python branches | 46.69% (1328/2844) | 46.69% (1328/2844) |
| hand-written | 83.58% (5268/6303) | 83.56% (5267/6303) |
| generated | 59.00% (5036/8535) | 59.00% (5036/8535) |
| tests | 960 passed / 2 failed / 57 skipped | 965 / 2 / 57 |
The only per-file mover is e2b/envd/api.py losing line 61 (except Exception: in acheck_sandbox_health) — an untouched file whose live health-check path flickers by ±1 line between runs off this base, not something this PR caused. No file gains or loses a branch, and no file appears or disappears. Both failures are the pre-existing test_firewall_transform_injects_headers pair (the test org has no httpbin template); they fail identically on base. js-sdk and cli are unaffected by construction — the diff contains no file outside packages/python-sdk/tests/.
The junit inventory diff is +7 added / -2 removed / 0 status changes, and the two removals are the ..._releases_connection → ..._releases_response renames, so this is +5 real tests, all of them running and passing. That is exactly the result #1690 produced in the opposite direction: deleting those tests moved no number either, because a reader's lines stay reachable through the ordinary end-of-stream path. Coverage cannot review this PR, so I reverted behaviour in the source instead and re-ran the two touched files (32 tests, ~10 s).
What the new tests actually pin
| mutant | caught by |
|---|---|
__del__ closing the response on FileStreamReader |
only the new test_sync_abandoned_reader_leaves_the_response_open |
__del__ scheduling aclose() on the loop for AsyncFileStreamReader |
only the new test_async_abandoned_reader_leaves_the_response_open |
streamed read drops an explicit request_timeout |
only the new test_sync_stream_explicit_request_timeout_is_a_total_transfer_deadline |
| default streamed read gets a wall-clock deadline equal to the idle bound | 6 tests: the 2 new slow-consumer ones and the 4 pre-existing survives_* ones |
| streamed reads run on the regular (unbounded) transport | 3 pre-existing stall tests, no new one |
| reader stops releasing the response on a mid-stream error | pre-existing test_sync_read_error_releases_response |
streamed read reuses the non-streaming FILE_TIMEOUT (3600 s) as its deadline |
nothing — survives |
except BaseException narrowed to except httpx.HTTPError in FileStreamReader.__next__ |
nothing — survives |
The first three rows are the PR's real value, and the naive async __del__ mutant (calling self._response.close()) is not a valid test of the async one — it raises inside the finalizer and gets swallowed, so the reader's docstring is right that only a loop-scheduled aclose() can do it, and that is the mutant the test kills. The fourth row is worth naming because it is the property the SLOW_CONSUMER_* comment claims to guard: the wall-clock regression is caught, but not uniquely — test_{sync,async}_stream_survives_transfers_longer_than_read_timeout already fails on it, since 4 × 0.15 s of transfer exceeds the 0.3 s bound on its own.
On the vacuity question @chatgpt-codex-connector raised and the sibling automation measured: I reproduced it independently and got the same answer, including the detail that makes it subtle. My first probe dropped the fixture's time.sleep(delay) and showed the whole 2 MiB body buffered before the stall in 5/5 runs; with the call restored (it yields the GIL even at 0.0) the server sits at 512–581 of 2048 chunks with the body incomplete in 5/5. So the tests are not vacuous on an idle machine, and the mechanism really is scheduling rather than the buffer-capacity reasoning in the comment. Nothing to add beyond what is already on that thread.
Against what #1690 removed
fc3496120 deleted five reader tests: two idle-timeout, one slow-consumer, two abandoned-reader. This PR brings back the abandoned-reader pair (with the assertion inverted to the contract that actually holds) and the slow-consumer case, relocated to test_volume_client.py where it runs against the shipped pyqwest transports instead of a bare httpx.Client() — which answers #1690's own objection to the originals. The two idle-timeout tests stay gone, and that is fine: mutation shows the sync release-on-error contract is already held by test_sync_read_error_releases_response, and the async per-call bound by test_async_reader_explicit_idle_timeout_bounds_each_read.
Sent by Cursor Automation: /coverage SDK Test Coverage Report
| assert asyncio.run(run()) == (True, len(CHUNK) * SLOW_CONSUMER_CHUNKS) | ||
|
|
||
|
|
||
| def test_sync_stream_explicit_request_timeout_is_a_total_transfer_deadline( |
There was a problem hiding this comment.
This test is the sole guard on forwarding an explicit request_timeout (dropping it fails this test and nothing else). But the complementary half — that the default streamed read carries no whole-request deadline — has no guard at all, and it is the half the code comment above stream_timeout cares about.
The surviving mutant is a one-liner, in both volume_sync.py and volume_async.py:
# stream_timeout = VolumeConnectionConfig._get_request_timeout(
# None, opts.get("request_timeout")
# )
stream_timeout = timeout # FILE_TIMEOUT-based, i.e. 3600 stimeout is computed ten lines earlier in the same function and is unused on the streaming branch, so collapsing the two is the natural "cleanup" someone will eventually make. All 21 tests in this file still pass, because 3600 s is longer than every stall any of them can produce — and the SDK is then back to exactly what the comment warns about, a whole-request deadline that kills long downloads, just with a big constant.
A deadline-value assertion closes it without any timing, covers both flavours (which also settles the sync-only concern on my sibling's thread below, since it asserts at the call boundary rather than through the adapter's two different enforcement paths), and runs in 0.5 s:
@pytest.fixture
def recorded_stream_timeouts(monkeypatch):
seen = []
sync_stream, async_stream = httpx.Client.stream, httpx.AsyncClient.stream
def sync_spy(self, *args, **kwargs):
seen.append(kwargs.get("timeout"))
return sync_stream(self, *args, **kwargs)
def async_spy(self, *args, **kwargs):
seen.append(kwargs.get("timeout"))
return async_stream(self, *args, **kwargs)
monkeypatch.setattr(httpx.Client, "stream", sync_spy)
monkeypatch.setattr(httpx.AsyncClient, "stream", async_spy)
return seen
def test_sync_streamed_read_carries_no_deadline_by_default(recorded_stream_timeouts):
api_url = _start_volume_file_server([0.0])
volume = Volume(volume_id="v1", name="test", token="vol-token")
stream = volume.read_file("file.bin", format="stream", api_url=api_url)
assert b"".join(stream) == CHUNK
# A whole-request deadline here would cap total transfer time, which is
# what FILE_TIMEOUT does on the non-streamed path.
assert recorded_stream_timeouts == [None]
def test_sync_streamed_read_forwards_an_explicit_request_timeout(recorded_stream_timeouts):
api_url = _start_volume_file_server([0.0])
volume = Volume(volume_id="v1", name="test", token="vol-token")
stream = volume.read_file(
"file.bin", format="stream", request_timeout=12.0, api_url=api_url
)
assert b"".join(stream) == CHUNK
assert recorded_stream_timeouts == [12.0]I ran these plus their two AsyncVolume counterparts: 4 passed in 0.47 s on this head; under the stream_timeout = timeout mutant the two default-path tests fail with assert [3600.0] == [None] in both flavours; under a stream_timeout = None mutant the two explicit-path tests fail with assert [None] == [12.0]. They complement the timing test above rather than replace it — this one pins the value, that one pins that the value is really enforced end to end.
| # Asserted on the `httpx.HTTPError` base rather than the concrete | ||
| # class, because which error a truncated body surfaces as is the | ||
| # transport's business, not part of the reader's contract. | ||
| with pytest.raises(httpx.HTTPError): |
There was a problem hiding this comment.
Two small things about this file that came out of the mutation run.
On the widening: I checked what a truncated body actually surfaces as here, and it is deterministically httpx.RemoteProtocolError ("peer closed connection without sending complete message body (incomplete chunked read)"). The stated reason for relaxing the assertion — which error a truncation surfaces as is the transport's business — does not apply to this test as written, because _open_stream builds a bare httpx.Client(), so the error class is httpx's own and not the shipped pyqwest transport's. The relaxation is therefore paying specificity for portability the test cannot have: httpx.HTTPError is the base of ReadTimeout, ConnectTimeout, ConnectError and PoolTimeout too, so a regression that turned a truncation into a hang-then-timeout would still satisfy it. Keeping RemoteProtocolError (or asserting on httpx.TransportError if a slightly wider net is wanted) costs nothing today.
Separately, and not something this PR introduced: test_sync_full_consume_releases_response above does not distinguish the reader's release from httpx's. Narrowing FileStreamReader.__next__'s except BaseException to except httpx.HTTPError — which removes the StopIteration → close() path entirely — leaves all 11 tests in this file green, because iter_bytes() closes the response itself when the stream ends. The error path is genuinely pinned (deleting self.close() from the handler fails test_sync_read_error_releases_response); the normal-end path is redundant with httpx and asserted by nobody. Worth a line in that test if the reader is meant to own the release regardless of what httpx does.
1 participant
Follow-up to #1690 (review findings). That PR deleted five httpcore-era tests from
test_file_stream_reader.py; three of them only ever asserted httpcore internals and are correctly gone, but two covered behavior that is still true — and still worth guarding — once re-anchored the way the surviving tests were.Restores the abandoned-reader contract
FileStreamReaderandAsyncFileStreamReaderboth document in their public docstrings (e2b/sandbox/filesystem/filesystem.py:166and:213) that they have no garbage-collection safety net. The deleted tests were that contract's only executable proof. They re-anchor onresponse.is_closedas cleanly as everything else in #1690 did — drop the reader half-consumed, collect, assert the response stayed open.The async variant sleeps before asserting so async-generator finalization gets a chance to run, which is what makes it a real assertion rather than a race the test happens to win.
Restores the slow-consumer guard, on the real pyqwest transports
The idle read timeout is wire-only, so a consumer that stalls far longer than the bound must not abort the stream. The JS SDK asserts this (
packages/js-sdk/tests/connectionConfig.test.ts:561, "does not abort a slow consumer"); Python had no equivalent left after #1690. It now lives intest_volume_client.pyagainst actual streaming transports rather than a barehttpx.Client(), for both sync and async.Two details make it non-vacuous:
request_timeout— a documented whole-transfer deadline — and the same slow consumer does get cut off. Without that pairing, a regression that turned the default idle bound into a wall-clock one would be indistinguishable from correct behavior.Cleanups from the same review
_start_truncating_serverback into_start_chunked_serveras atruncate_beforechunk index, dropping ~25 duplicated lines.httpx.RemoteProtocolErrorto thehttpx.HTTPErrorbase. Which concrete error a truncated body surfaces as is the transport's business; the reader's contract is only that it closes the response. Pinning the concrete class re-introduced exactly the transport coupling test(python-sdk): drop httpcore-era stream reader tests after the pyqwest migration #1690 set out to remove...._releases_connectiontests to..._releases_response, matching their assertions, the module docstring, and test(python-sdk): drop httpcore-era stream reader tests after the pyqwest migration #1690's own newtest_sync_read_error_releases_response.while b"\r\n\r\n" not in conn.recv(65536)inspected only the latestrecvinstead of accumulated bytes, and spun forever on a closed peer. Near-unreachable on loopback, but test(python-sdk): drop httpcore-era stream reader tests after the pyqwest migration #1690 copied it into a second helper, so it's fixed in both.Usage examples
None — this PR touches only
packages/python-sdk/tests/. No public API changes, so nothing user-facing differs.Verification
Both restored contracts were verified to actually hold before being committed as assertions, and the slow-consumer guard was mutation-tested (a wall-clock deadline on the same path does fail it) so it isn't a test that can only pass.
Test-only, so no changeset — matching the convention #1690 followed.
Follow-up to SDK-324.
🤖 Generated with Claude Code