Expand integration tests, fix transport response reading bug#10
Merged
Conversation
This reverts commit edf974f.
Transport: - Move response.read()/aread() before _should_retry so exhausted retries also have a read body for error parsing - Add await response.aread() in async transport (was sync-only) - Remove response.read() from _parse_error_body (caller's job) Tests: - Consolidate redundant API calls via shared fixtures - Fix session tests: status() returns a string, not Session object - Actually test context manager protocol with `with` statement - Use itertools.islice instead of zip(range()) - Remove redundant warnings.catch_warnings blocks (conftest handles it) - Extract _submit() helper for repeated job creation - Add docstring explaining why async_client fixture exists separately
- Flatten TestListBackends into plain functions with a shared fixture - Flatten single-test classes (cancel, delete, estimate, poll) - Add assert to _submit helper for clearer failures - Use context manager in test_session_lifecycle instead of manual cleanup
- scope="module" on backends fixture (1 API call instead of 3) - scope="class" on completed_job_id fixture (1 call instead of 3) - _submit accepts **overrides, simplifying dry_run test - Remove test_session_context_manager (covered by test_session_lifecycle)
- Move SSL warning filters from module-level to session-scoped autouse fixture so they don't leak into unit test collection - Add track_job to test_delete_job for cleanup if delete fails
Add missing blank line between fixture and module-level variable.
…ompat pip install -e '.[dev]' only works with [project.optional-dependencies], not [dependency-groups] (PEP 735). Add the dev extra so both pip and uv workflows work. This matches the pattern used by pypa/build.
Extract _paginate/_apaginate helpers to eliminate duplicated cursor- following logic across 4 pagination functions. Extract _ext() helper in IonQClient to consolidate the repeated extension-or-default precedence pattern and group sync/async transport chain construction. Net reduction: ~25 lines with improved readability.
Replace the _ext() closure in IonQClient with direct attribute access. The closure's docstring claimed extension-or-default precedence, but its implementation couldn't actually resolve extension attributes when a default was provided. Change _build_settings() to return UNSET instead of None, matching the auto-generated model's field type and eliminating conditional dict-splat at both sync and async call sites.
Replace the custom RetryTransport/AsyncRetryTransport (192 lines) with httpx-retries, a lightweight library (8.9kB) that provides the same retry logic out of the box: exponential backoff, jitter, Retry-After header support, status code filtering, and idempotent method awareness. What changed: - _transport.py: Remove 7 helper functions and 2 transport classes, replace with build_retry/build_sync_transport/build_async_transport that configure httpx-retries, plus thin ErrorRaisingTransport that converts error responses to structured IonQ exceptions. - _extensions.py: Switch ClientExtension from stdlib dataclasses to attrs for consistency with the rest of the codebase. - ionq_client.py: Use new transport builder functions. Hand-written code reduced from 1,130 to 1,058 lines (-72). All 221 tests pass with 100% branch coverage.
Merge ErrorRaisingTransport/AsyncErrorRaisingTransport into a single dual-inheritance class (same pattern httpx-retries uses). Do the same for HookTransport, _ErrorMapperTransport, and their async counterparts. Also inline _parse_error_body and _parse_retry_after into _raise_for_response, remove unused future annotations imports, unify build_sync_transport/build_async_transport into build_transport, and trim docstrings to essential content. Hand-written code: 1058 -> 966 lines (-92, -8.7%). All 220 tests pass with 100% branch coverage.
RetryTransport(retry=retry) with no explicit inner transport auto- creates both sync and async transports internally (per httpx-retries docs). Remove the async_ parameter and build a single transport that serves both the sync Client and AsyncClient. Hand-written code: 968 -> 965 lines.
…tations Replace typing.Any on the transport parameter with a proper Protocol that defines both sync and async transport interfaces. Move ErrorRaisingTransport above build_transport to eliminate the forward reference, removing the need for from __future__ import annotations. The remaining files (_session.py, _pagination.py, _polling.py) keep from __future__ import annotations because they use TYPE_CHECKING guards to avoid circular imports - the correct pattern for Python 3.12-3.13 per PEP 563/749.
- Remove redundant Retry() params (respect_retry_after_header, allowed_methods) that matched library defaults - Use @attrs.frozen shorthand instead of @attrs.define(frozen=True) - Inline _build_user_agent and use ClientExtension() default to simplify IonQClient transport chain setup - Merge sync/async test transport classes using dual-inheritance pattern - Extract shared make_job_json() helper to conftest, replacing duplicated 20+ line job JSON builders in test_polling and test_pagination
Bump dev dependency floors to match actual major versions in use: - pytest >=8 -> >=9 (required by pytest-httpx >=0.36) - pytest-httpx >=0.35 -> >=0.36 - pytest-asyncio >=0.25 -> >=1 - pytest-cov >=6 -> >=7 - ruff >=0.9 -> >=0.15 Lock file upgraded: - certifi 2026.2.25 -> 2026.4.22 - idna 3.11 -> 3.13 - packaging 26.0 -> 26.1 - pytest-httpx 0.36.0 -> 0.36.2 - ruff 0.15.9 -> 0.15.11 - ty 0.0.29 -> 0.0.32
- Fix bug where ClientExtension(max_retries=0) was silently ignored because Python's `or` operator treats 0 as falsy, falling through to DEFAULT_MAX_RETRIES. Use explicit `is not None` checks instead. - Remove duplicate [project.optional-dependencies] dev section that mirrored [dependency-groups] dev; CI uses `uv sync` which reads dependency-groups. - Add asyncio_default_fixture_loop_scope to silence pytest-asyncio deprecation warning about unset default scope. - Replace 50-iteration response registration loops with pytest-httpx is_reusable=True in polling timeout tests. - Remove 11 unnecessary assert_all_responses_were_requested=False markers from tests where the single registered response is always consumed. - Fix pre-existing ruff format violation in build_transport().
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
1 participant
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.
Summary
_transport.pywhere_parse_error_bodyaccessedresponse.json()/response.textwithout callingresponse.read()first, causingResponseNotReadon error status codesIntegration test coverage
get_backends,get_backendget_characterizations_for_backend,get_characterizationcreate_job,get_job,get_jobs,cancel_job,delete_job,get_job_cost,get_job_probabilities,estimate_job_cost,wait_for_job,iter_jobsget_sessions,create_session,end_session,get_session_jobs,SessionManagerget_whoami(sync + detailed)get_usagesasynciovariants of whoami, list jobs,aiter_jobsTests that depend on account-specific features (sessions, usage) skip gracefully when the API key lacks the required scope.
Test plan
uv run pytest- 234 unit tests pass, 100% coverage