test: close deep-audit test-quality findings#65
Merged
Conversation
…apping Add sync mirrors for every async case in test_error_mapping_terminal.py: 2xx pass-through, typed status subclasses, unknown 4xx/5xx fallbacks, 3xx no-raise, timeout/connect/invalid-url/decoding-error transport mapping, and closed-client TransportError. Uses Client(httpx2_client=httpx2.Client(...)) pattern per CLAUDE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add async and sync tests asserting that httpx2.CookieConflict raised inside a transport handler surfaces as TransportError (not NetworkError), covering the mapping branch that was exercised only for InvalidURL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parametrize over all nine StatusError leaf subclasses and assert that none defines __init__ in its own __dict__, enforcing the CLAUDE.md invariant that leaf classes must not override StatusError.__init__. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ityError Extend test_per_status_subclasses_construct parametrize from 6 to all 9 leaf status subclasses, adding 403/ForbiddenError, 409/ConflictError, and 422/UnprocessableEntityError. Also smoke-tests __str__ on each. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s-ClientError Add test_sync_status_error_raised_before_decoder_runs (4xx with response_model raises StatusError, not DecodeError) and test_sync_decode_error_caught_by_client_error (malformed body raises DecodeError which is-a ClientError) for the sync Client, mirroring the existing async counterparts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add sync mirrors for all four AsyncClient overload tests: get/send × with/without response_model. Verifies at runtime that the sync Client's overloads resolve httpx2.Response vs typed model correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… not interleaving) The module docstring implied concurrent "retry interleaving" but every test issues one sequential request per Hypothesis example. Rewrite the docstring to accurately say it tests sequential retry-policy bounds and explicitly point to test_threading_with_shared_budget.py for the concurrent case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…on barrier The old test used time.sleep(0.005) hoping all holder threads had acquired their bulkhead slots before issuing the over-limit requests. On slow CI a holder might not have acquired yet, causing a spurious pass of the extra request and a false test outcome. Replace the sleep with threading.Barrier(max_concurrent + 1, timeout=5.0). _BarrierHandler.wait() is called from inside each holder's handler — meaning the slot is already held — and the main thread joins the same barrier. The main thread only proceeds past the barrier once every holder has acquired its slot, making the synchronization fully deterministic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…async) The circuit breaker catches both NetworkError and TimeoutError as counted failures, but tests only covered NetworkError (via ConnectError). Add test_timeout_error_counts_as_failure to both test_circuit_breaker.py (async) and test_circuit_breaker_sync.py (sync): a handler raising httpx2.ReadTimeout (maps to httpware TimeoutError) with failure_threshold=2 causes two such requests to open the circuit, and the third raises CircuitOpenError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…forced The deposit-count assertion relied on a comment saying the 60s TTL means no _purge fires during the sub-second run. RetryBudget accepts an injectable _now clock, so inject _fixed_clock (always returns 0.0). All deposit timestamps are 0.0, the purge cutoff is 0.0 - 60.0 = -60.0, and the strict "< cutoff" predicate is never true — making the no-purge guarantee a provable invariant rather than a time-dependent assumption. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test previously passed even if _emit_event became a no-op because its only assertion was "no exception raised." Strengthen it with caplog: capture at WARNING level on the test logger and assert that exactly one record fires with the correct level, message, and event attribute. This confirms the log-only fallback path actually emitted (not silently swallowed) when OTel is installed but no tracer is active. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ort shadow httpware.TimeoutError deliberately shadows the Python builtin (noqa A001 at the definition site). Tests importing it need noqa A004 at the import line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An auto-fix left uncommitted by the test-quality sweep; CI lint-ci (no autofix) caught the committed typing.Callable import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Closes the confirmed test-quality findings from the 2026-06-14 deep audit. Test-only — no
src/changes.Clientterminal had no exception-mapping/status-raising suite → added the sync mirrors oftest_error_mapping_terminalhttpx2.CookieConflict → TransportErrormapping branch was untested → added (sync + async)__init__" invariant → parametrized check over all 9 leavesForbiddenError/ConflictError/UnprocessableEntityErrornever constructed → added to the per-status parametrizeDecodeError-is-ClientError→ addedClientoverload typing untested → mirrored theAsyncClientoverload teststest_retry_propsoverclaimed "interleaving" → corrected to "sequential bounds" (concurrency is covered bytest_threading_with_shared_budget)test_bulkhead_sync_propsfixedtime.sleep(0.005)was flaky → replaced with athreading.Barrieron actual slot acquisitionTimeoutErrortripping the CircuitBreaker was untested → added (sync + async, viahttpx2.ReadTimeout)test_threading_with_shared_budgetdeposit count rested on a comment → pinned the injected clock so_purgeprovably can't firecaplog(Nit9 — large-
attempt_indexbackoff test — already landed in #64.)Test plan
just test— 656 passed, 100% coverage;just lintcleansrc/files changed (git diff origin/main..HEAD -- srcempty)🤖 Generated with Claude Code