From bcfded984b3e43e1e71949c944e4b6639ab08e4e Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:03:40 +0300 Subject: [PATCH 01/14] docs(planning): audit-test-quality change bundle --- .../change.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 planning/changes/active/2026-06-14.05-audit-test-quality/change.md diff --git a/planning/changes/active/2026-06-14.05-audit-test-quality/change.md b/planning/changes/active/2026-06-14.05-audit-test-quality/change.md new file mode 100644 index 0000000..124af6a --- /dev/null +++ b/planning/changes/active/2026-06-14.05-audit-test-quality/change.md @@ -0,0 +1,40 @@ +--- +status: draft +date: 2026-06-14 +slug: audit-test-quality +supersedes: null +superseded_by: null +pr: null +outcome: null +--- + +# Change: Deep-audit test-quality findings + +**Lane:** test-only sweep; spec is the [2026-06-14 deep audit](../../../audits/2026-06-14-deep-audit.md). + +## Goal + +Close the confirmed test-quality findings: assertion gaps, missing coverage, +sync/async test parity, and two flaky/fragile tests. No production code changes. + +## Findings + +- **M3** — sync `Client._terminal` status-raising has no parallel suite (`test_error_mapping_terminal` is async-only). +- **L9** — `test_retry_props` docstring claims "retry interleaving" but is sequential → correct the description (concurrency is covered by `test_threading_with_shared_budget`). +- **L10** — `test_bulkhead_sync_props` uses a fixed `time.sleep(0.005)` → replace with a deterministic barrier. +- **L11** — no test asserts `StatusError` leaves don't override `__init__` → parametrized check over the nine leaves. +- **L12** — no test exercises `TimeoutError` tripping the CircuitBreaker (async + sync). +- **Nit7** — `test_threading_with_shared_budget` exact deposit count rests on a comment → pin the clock. +- **Nit8** — `ForbiddenError`/`ConflictError`/`UnprocessableEntityError` never constructed → add to the per-status parametrize. +- **Nit10** — `test_emit_event_works_when_otel_installed_but_no_active_span` has no assertion → assert via caplog. +- **Nit11** — no sync-overload typing test for `Client` → mirror `test_client_typing`. +- **Nit12** — no sync counterpart to status-before-decoder / DecodeError-is-ClientError. +- **Nit13** — no test for the `httpx2.CookieConflict → TransportError` mapping branch. + +(Nit9 — large-`attempt_index` backoff test — already landed in PR #64.) + +## Verification + +- [ ] Each addition is TDD-meaningful (asserts the property, not a vacuous pass). +- [ ] L10/Nit7 are deterministic (no real sleeps / wall-clock assumptions). +- [ ] `just test` 100% coverage; `just lint` clean. From 50a56b4237270e4873ce06cafb805b814656fea5 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:05:12 +0300 Subject: [PATCH 02/14] test(errors): cover sync Client terminal status-raising + exception mapping 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) --- tests/test_error_mapping_terminal.py | 128 ++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 2 deletions(-) diff --git a/tests/test_error_mapping_terminal.py b/tests/test_error_mapping_terminal.py index a2a86ad..e47e151 100644 --- a/tests/test_error_mapping_terminal.py +++ b/tests/test_error_mapping_terminal.py @@ -1,6 +1,7 @@ -"""Tests for the AsyncClient internal terminal's exception mapping.""" +"""Tests for the AsyncClient and Client internal terminal's exception mapping.""" from http import HTTPStatus +from typing import Callable import httpx2 import pytest @@ -8,6 +9,7 @@ from httpware import ( AsyncClient, BadRequestError, + Client, ClientStatusError, InternalServerError, NotFoundError, @@ -20,11 +22,16 @@ from httpware.errors import NetworkError -def _client_with_handler(handler) -> AsyncClient: # noqa: ANN001 +def _client_with_handler(handler: Callable[[httpx2.Request], httpx2.Response]) -> AsyncClient: transport = httpx2.MockTransport(handler) return AsyncClient(httpx2_client=httpx2.AsyncClient(transport=transport)) +def _sync_client_with_handler(handler: Callable[[httpx2.Request], httpx2.Response]) -> Client: + transport = httpx2.MockTransport(handler) + return Client(httpx2_client=httpx2.Client(transport=transport)) + + async def test_terminal_returns_response_on_2xx() -> None: client = _client_with_handler(lambda req: httpx2.Response(HTTPStatus.OK, json={"ok": True}, request=req)) response = await client.send(httpx2.Request("GET", "https://example.test/x")) @@ -135,3 +142,120 @@ def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 with pytest.raises(TransportError) as info: await client.send(httpx2.Request("GET", "https://example.test/x")) assert not isinstance(info.value, NetworkError) + + +# --------------------------------------------------------------------------- +# Sync Client terminal — mirrors of all async cases above +# --------------------------------------------------------------------------- + + +def test_sync_terminal_returns_response_on_2xx() -> None: + client = _sync_client_with_handler(lambda req: httpx2.Response(HTTPStatus.OK, json={"ok": True}, request=req)) + response = client.send(httpx2.Request("GET", "https://example.test/x")) + assert response.status_code == HTTPStatus.OK + assert response.json() == {"ok": True} + + +@pytest.mark.parametrize( + ("status", "exc_type"), + [ + (HTTPStatus.BAD_REQUEST, BadRequestError), + (HTTPStatus.NOT_FOUND, NotFoundError), + (HTTPStatus.TOO_MANY_REQUESTS, RateLimitedError), + (HTTPStatus.INTERNAL_SERVER_ERROR, InternalServerError), + ], +) +def test_sync_known_status_codes_raise_typed_subclass(status: int, exc_type: type[StatusError]) -> None: + client = _sync_client_with_handler(lambda req: httpx2.Response(status, request=req)) + with pytest.raises(exc_type) as info: + client.send(httpx2.Request("GET", "https://example.test/x")) + assert info.value.response.status_code == status + + +def test_sync_unknown_4xx_falls_back_to_client_status_error() -> None: + client = _sync_client_with_handler(lambda req: httpx2.Response(HTTPStatus.IM_A_TEAPOT, request=req)) + with pytest.raises(ClientStatusError) as info: + client.send(httpx2.Request("GET", "https://example.test/x")) + assert info.value.response.status_code == HTTPStatus.IM_A_TEAPOT + assert type(info.value) is ClientStatusError + + +def test_sync_unknown_5xx_falls_back_to_server_status_error() -> None: + client = _sync_client_with_handler(lambda req: httpx2.Response(599, request=req)) + with pytest.raises(ServerStatusError) as info: + client.send(httpx2.Request("GET", "https://example.test/x")) + assert info.value.response.status_code == 599 # noqa: PLR2004 + assert type(info.value) is ServerStatusError + + +def test_sync_3xx_does_not_raise() -> None: + client = _sync_client_with_handler( + lambda req: httpx2.Response(HTTPStatus.MOVED_PERMANENTLY, request=req, headers={"location": "/y"}) + ) + response = client.send(httpx2.Request("GET", "https://example.test/x")) + assert response.status_code == HTTPStatus.MOVED_PERMANENTLY + + +def test_sync_httpx2_timeout_maps_to_httpware_timeout() -> None: + def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "read timeout" + raise httpx2.ReadTimeout(msg) + + client = _sync_client_with_handler(handler) + with pytest.raises(TimeoutError, match="read timeout"): + client.send(httpx2.Request("GET", "https://example.test/x")) + + +def test_sync_httpx2_connect_error_maps_to_network_error() -> None: + def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "connect refused" + raise httpx2.ConnectError(msg) + + client = _sync_client_with_handler(handler) + with pytest.raises(NetworkError, match="connect refused"): + client.send(httpx2.Request("GET", "https://example.test/x")) + + +def test_sync_httpx2_invalid_url_maps_to_transport_error() -> None: + def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "synthetic invalid URL from transport" + raise httpx2.InvalidURL(msg) + + client = _sync_client_with_handler(handler) + with pytest.raises(TransportError, match="synthetic invalid URL"): + client.send(httpx2.Request("GET", "https://example.test/x")) + + +def test_sync_send_on_closed_client_raises_transport_error() -> None: + transport = httpx2.MockTransport(lambda req: httpx2.Response(HTTPStatus.OK, request=req)) + underlying = httpx2.Client(transport=transport) + client = Client(httpx2_client=underlying) + underlying.close() + with pytest.raises(TransportError): + client.send(httpx2.Request("GET", "https://example.test/x")) + + +def test_sync_httpx2_decoding_error_maps_to_transport_error() -> None: + """Non-transient HTTPError (e.g. DecodingError) maps to bare TransportError, not NetworkError.""" + + def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "decoding failed" + raise httpx2.DecodingError(msg) + + client = _sync_client_with_handler(handler) + with pytest.raises(TransportError) as info: + client.send(httpx2.Request("GET", "https://example.test/x")) + assert not isinstance(info.value, NetworkError) + + +def test_sync_httpx2_invalid_url_does_not_map_to_network_error() -> None: + """Regression: only transient errors map to NetworkError; InvalidURL stays bare TransportError.""" + + def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "bad url" + raise httpx2.InvalidURL(msg) + + client = _sync_client_with_handler(handler) + with pytest.raises(TransportError) as info: + client.send(httpx2.Request("GET", "https://example.test/x")) + assert not isinstance(info.value, NetworkError) From 427dfa2fafe27a4caa0aefc23b3fb2661f8a7925 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:05:28 +0300 Subject: [PATCH 03/14] =?UTF-8?q?test(errors):=20cover=20CookieConflict?= =?UTF-8?q?=E2=86=92TransportError=20mapping=20branch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- tests/test_error_mapping_terminal.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_error_mapping_terminal.py b/tests/test_error_mapping_terminal.py index e47e151..9660756 100644 --- a/tests/test_error_mapping_terminal.py +++ b/tests/test_error_mapping_terminal.py @@ -259,3 +259,30 @@ def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 with pytest.raises(TransportError) as info: client.send(httpx2.Request("GET", "https://example.test/x")) assert not isinstance(info.value, NetworkError) + + +# --------------------------------------------------------------------------- +# Nit13: CookieConflict → TransportError (NOT NetworkError) +# --------------------------------------------------------------------------- + + +async def test_async_httpx2_cookie_conflict_maps_to_transport_error() -> None: + def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "cookie conflict" + raise httpx2.CookieConflict(msg) + + client = _client_with_handler(handler) + with pytest.raises(TransportError) as info: + await client.send(httpx2.Request("GET", "https://example.test/x")) + assert not isinstance(info.value, NetworkError) + + +def test_sync_httpx2_cookie_conflict_maps_to_transport_error() -> None: + def handler(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "cookie conflict" + raise httpx2.CookieConflict(msg) + + client = _sync_client_with_handler(handler) + with pytest.raises(TransportError) as info: + client.send(httpx2.Request("GET", "https://example.test/x")) + assert not isinstance(info.value, NetworkError) From 9332fa055e306f71e0928a6b3a660816dc772594 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:05:44 +0300 Subject: [PATCH 04/14] test(errors): assert StatusError leaves do not override __init__ 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) --- tests/test_errors.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_errors.py b/tests/test_errors.py index 66e5c3b..f9b1130 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -142,6 +142,27 @@ def test_per_status_subclasses_construct(status: int, expected: type[StatusError assert exc.response.status_code == status +@pytest.mark.parametrize( + "cls", + [ + BadRequestError, + UnauthorizedError, + ForbiddenError, + NotFoundError, + ConflictError, + UnprocessableEntityError, + RateLimitedError, + InternalServerError, + ServiceUnavailableError, + ], +) +def test_status_error_leaves_do_not_override_init(cls: type[StatusError]) -> None: + """CLAUDE.md invariant: StatusError leaf classes must not define their own __init__.""" + assert "__init__" not in cls.__dict__, ( + f"{cls.__name__} defines __init__ — StatusError leaves must inherit StatusError.__init__ directly" + ) + + def test_status_error_strips_userinfo_with_username_only() -> None: exc = NotFoundError(_make_response(404, url="https://user@example.test/x")) assert "user" not in str(exc) From e5d94e9622214ccec1ef012393b678193902345e Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:06:01 +0300 Subject: [PATCH 05/14] test(errors): construct ForbiddenError/ConflictError/UnprocessableEntityError 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) --- tests/test_errors.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_errors.py b/tests/test_errors.py index f9b1130..2a087ae 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -129,7 +129,10 @@ def test_status_error_pickleable() -> None: [ (400, BadRequestError), (401, UnauthorizedError), + (403, ForbiddenError), (404, NotFoundError), + (409, ConflictError), + (422, UnprocessableEntityError), (429, RateLimitedError), (500, InternalServerError), (503, ServiceUnavailableError), @@ -140,6 +143,7 @@ def test_per_status_subclasses_construct(status: int, expected: type[StatusError exc = expected(response) assert isinstance(exc, expected) assert exc.response.status_code == status + assert str(exc) # smoke-test __str__ produces a non-empty string @pytest.mark.parametrize( From 7f068b9ddfc801c0d6d0f925b0d0f6b7995220da Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:06:23 +0300 Subject: [PATCH 06/14] test(client): sync mirrors for status-before-decode and DecodeError-is-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) --- tests/test_client_response_model.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_client_response_model.py b/tests/test_client_response_model.py index 82e5263..8c13b15 100644 --- a/tests/test_client_response_model.py +++ b/tests/test_client_response_model.py @@ -99,6 +99,24 @@ async def test_async_decode_error_caught_by_client_error() -> None: assert isinstance(exc_info.value, DecodeError) +def test_sync_status_error_raised_before_decoder_runs() -> None: + def handler(request: httpx2.Request) -> httpx2.Response: + return httpx2.Response(HTTPStatus.NOT_FOUND, content=b'{"id": 1, "name": "x"}', request=request) + + transport = httpx2.MockTransport(handler) + client = Client(httpx2_client=httpx2.Client(transport=transport)) + with pytest.raises(NotFoundError): + client.get("https://example.test/u", response_model=_User) + + +def test_sync_decode_error_caught_by_client_error() -> None: + """The user-facing promise: `except ClientError` catches decode failures on the sync client.""" + client = _sync_client_with_payload(b"null") + with pytest.raises(ClientError) as exc_info: + client.get("https://example.test/u", response_model=_User) + assert isinstance(exc_info.value, DecodeError) + + def test_sync_schema_mismatch_raises_decode_error() -> None: client = _sync_client_with_payload(b"null") with pytest.raises(DecodeError) as exc_info: From 928c3c06724d06dac16d8eb631d76ecf63a3c482 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:06:47 +0300 Subject: [PATCH 07/14] test(client): sync Client overload typing tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- tests/test_client_typing.py | 45 +++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/tests/test_client_typing.py b/tests/test_client_typing.py index 397d3ff..12f36cc 100644 --- a/tests/test_client_typing.py +++ b/tests/test_client_typing.py @@ -1,4 +1,4 @@ -"""Static-typing tests for AsyncClient overloads. +"""Static-typing tests for AsyncClient and Client overloads. These assert overload selection at runtime via isinstance checks. ty/mypy catches the static-typing variant during `just lint`. @@ -9,7 +9,7 @@ import httpx2 import pydantic -from httpware import AsyncClient +from httpware import AsyncClient, Client class _User(pydantic.BaseModel): @@ -51,3 +51,44 @@ async def test_send_with_response_model_returns_typed() -> None: client = AsyncClient(httpx2_client=httpx2.AsyncClient(transport=transport)) result = await client.send(httpx2.Request("GET", "https://example.test/x"), response_model=_User) assert isinstance(result, _User) + + +# --------------------------------------------------------------------------- +# Sync Client overload tests — mirrors of each async case above +# --------------------------------------------------------------------------- + + +def test_sync_get_without_response_model_returns_response() -> None: + transport = httpx2.MockTransport( + lambda req: httpx2.Response(HTTPStatus.OK, request=req, json={"id": 1, "name": "a"}) + ) + client = Client(httpx2_client=httpx2.Client(transport=transport)) + result = client.get("https://example.test/x") + assert isinstance(result, httpx2.Response) + + +def test_sync_get_with_response_model_returns_typed() -> None: + transport = httpx2.MockTransport( + lambda req: httpx2.Response(HTTPStatus.OK, request=req, json={"id": 1, "name": "a"}) + ) + client = Client(httpx2_client=httpx2.Client(transport=transport)) + result = client.get("https://example.test/x", response_model=_User) + assert isinstance(result, _User) + + +def test_sync_send_without_response_model_returns_response() -> None: + transport = httpx2.MockTransport( + lambda req: httpx2.Response(HTTPStatus.OK, request=req, json={"id": 1, "name": "a"}) + ) + client = Client(httpx2_client=httpx2.Client(transport=transport)) + result = client.send(httpx2.Request("GET", "https://example.test/x")) + assert isinstance(result, httpx2.Response) + + +def test_sync_send_with_response_model_returns_typed() -> None: + transport = httpx2.MockTransport( + lambda req: httpx2.Response(HTTPStatus.OK, request=req, json={"id": 1, "name": "a"}) + ) + client = Client(httpx2_client=httpx2.Client(transport=transport)) + result = client.send(httpx2.Request("GET", "https://example.test/x"), response_model=_User) + assert isinstance(result, _User) From 522745d6204caaa843825e20224f8da7efc7cb61 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:09:04 +0300 Subject: [PATCH 08/14] test(retry): correct test_retry_props description (sequential bounds, 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) --- tests/test_retry_props.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_retry_props.py b/tests/test_retry_props.py index 7ebfe9e..528b235 100644 --- a/tests/test_retry_props.py +++ b/tests/test_retry_props.py @@ -1,11 +1,15 @@ -"""Hypothesis property tests for AsyncRetry. +"""Hypothesis property tests for AsyncRetry — sequential retry-policy bounds. -Properties verified: +Each test issues a single sequential request (one Hypothesis example at a time) +and verifies the bounds that apply to a single-caller retry loop: 1. Total attempts never exceed max_attempts. 2. Total sleep time never exceeds max_attempts * max_delay. 3. Non-retryable statuses (NOT in retry_status_codes) cause exactly one attempt. 4. Non-idempotent methods (NOT in retry_methods) cause exactly one attempt, regardless of response status. + +Concurrent shared-budget behaviour (multiple callers racing on one RetryBudget) +is covered separately in tests/test_threading_with_shared_budget.py. """ import math From 52ceb3b283125848f202b5f91c66699df2eb0d7a Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:10:13 +0300 Subject: [PATCH 09/14] test(bulkhead): replace flaky sleep with deterministic slot-acquisition barrier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- tests/test_bulkhead_sync_props.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/test_bulkhead_sync_props.py b/tests/test_bulkhead_sync_props.py index b4b0ca6..bc366eb 100644 --- a/tests/test_bulkhead_sync_props.py +++ b/tests/test_bulkhead_sync_props.py @@ -73,6 +73,20 @@ def test_in_flight_never_exceeds_max_concurrent( assert handler.max_in_flight <= max_concurrent +class _BarrierHandler: + """Handler that signals slot acquisition via a threading.Barrier before holding the slot.""" + + def __init__(self, barrier: threading.Barrier) -> None: + self._barrier = barrier + + def __call__(self, request: httpx2.Request) -> httpx2.Response: + # Signal that this holder has acquired a bulkhead slot and is now in-flight. + self._barrier.wait(timeout=5.0) + # Hold the slot long enough for the over-limit requests to be rejected. + time.sleep(0.05) + return httpx2.Response(HTTPStatus.OK, request=request) + + @given( max_concurrent=st.integers(min_value=1, max_value=4), extra_requests=st.integers(min_value=1, max_value=8), @@ -82,7 +96,11 @@ def test_fail_fast_rejects_when_at_capacity( max_concurrent: int, extra_requests: int, ) -> None: - handler = _InFlightHandler(delay=0.05) # hold slots long enough for fail-fast to fire + # Barrier: max_concurrent holders + 1 main thread — all parties meet once every + # holder has acquired its bulkhead slot (i.e. is inside the handler). + # timeout=5.0 sets the default for all barrier.wait() calls. + acquired_barrier = threading.Barrier(max_concurrent + 1, timeout=5.0) + handler = _BarrierHandler(acquired_barrier) transport = httpx2.MockTransport(handler) client = Client( httpx2_client=httpx2.Client(transport=transport), @@ -92,8 +110,8 @@ def test_fail_fast_rejects_when_at_capacity( # Fill the bulkhead with max_concurrent long-running threads. pool = ThreadPoolExecutor(max_workers=max_concurrent + extra_requests) holders = [pool.submit(client.get, f"https://example.test/hold-{i}") for i in range(max_concurrent)] - # Wait for the holders to acquire — sleep long enough for thread startup. - time.sleep(0.005) + # Wait deterministically — barrier releases only once ALL holders are inside the handler. + acquired_barrier.wait(timeout=5.0) # Any extra requests should fail fast with BulkheadFullError. for i in range(extra_requests): From 8e1186bb016b0636c0e9ac4cc364897579a47ba8 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:11:54 +0300 Subject: [PATCH 10/14] test(circuit_breaker): cover TimeoutError as a failure trigger (sync+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) --- tests/test_circuit_breaker.py | 15 +++++++++++++++ tests/test_circuit_breaker_sync.py | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/test_circuit_breaker.py b/tests/test_circuit_breaker.py index 800b6b1..74d623d 100644 --- a/tests/test_circuit_breaker.py +++ b/tests/test_circuit_breaker.py @@ -21,6 +21,7 @@ NotFoundError, RateLimitedError, ServiceUnavailableError, + TimeoutError, ) from httpware.middleware.resilience.circuit_breaker import AsyncCircuitBreaker @@ -169,6 +170,20 @@ def _raise(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 await client.get("https://example.test/x") +async def test_timeout_error_counts_as_failure() -> None: + def _raise(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + msg = "read timed out" + raise httpx2.ReadTimeout(msg, request=request) + + breaker = AsyncCircuitBreaker(failure_threshold=2, _now=_Clock()) + async with _client(_raise, breaker=breaker) as client: + for _ in range(2): + with pytest.raises(TimeoutError): + await client.get("https://example.test/x") + with pytest.raises(CircuitOpenError): + await client.get("https://example.test/x") + + async def test_custom_failure_status_codes_trips_on_member() -> None: """A status code in a custom failure set trips the breaker (plain set accepted).""" handler = _StatusSequence([503, 503]) diff --git a/tests/test_circuit_breaker_sync.py b/tests/test_circuit_breaker_sync.py index b9e116f..f4b0d2b 100644 --- a/tests/test_circuit_breaker_sync.py +++ b/tests/test_circuit_breaker_sync.py @@ -16,6 +16,7 @@ NotFoundError, RateLimitedError, ServiceUnavailableError, + TimeoutError, ) from httpware.middleware.resilience.circuit_breaker import CircuitBreaker @@ -140,6 +141,20 @@ def _raise(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 client.get("https://example.test/x") +def test_timeout_error_counts_as_failure() -> None: + def _raise(request: httpx2.Request) -> httpx2.Response: + msg = "read timed out" + raise httpx2.ReadTimeout(msg, request=request) + + breaker = CircuitBreaker(failure_threshold=2, _now=_Clock()) + with _client(_raise, breaker=breaker) as client: + for _ in range(2): + with pytest.raises(TimeoutError): + client.get("https://example.test/x") + with pytest.raises(CircuitOpenError): + client.get("https://example.test/x") + + def test_custom_failure_status_codes_trips_on_member() -> None: handler = _StatusSequence([503, 503]) breaker = CircuitBreaker(failure_threshold=2, failure_status_codes={503}, _now=_Clock()) # plain set accepted From d4ae14dc1845ed4c6a3b1eaa306efc3b659f5e50 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:12:34 +0300 Subject: [PATCH 11/14] test(budget): pin clock so the no-purge deposit-count invariant is enforced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- tests/test_threading_with_shared_budget.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/test_threading_with_shared_budget.py b/tests/test_threading_with_shared_budget.py index b459e56..c488f58 100644 --- a/tests/test_threading_with_shared_budget.py +++ b/tests/test_threading_with_shared_budget.py @@ -53,8 +53,16 @@ async def _drive_async_side(budget: RetryBudget) -> None: await asyncio.gather(*[_safe_get(async_client) for _ in range(_N_ASYNC_TASKS)]) +def _fixed_clock() -> float: + """Return a constant timestamp so RetryBudget._purge never evicts deposits during this test.""" + return 0.0 + + def test_shared_budget_across_sync_threads_and_async_loop() -> None: - budget = RetryBudget(ttl=60.0, min_retries_per_sec=1000.0, percent_can_retry=0.5) + # _now is pinned to a fixed timestamp: all deposits share the same timestamp, + # so the TTL window cutoff is also 0.0 and _purge evicts nothing. This makes + # the deposit-count assertion exact and independent of wall-clock elapsed time. + budget = RetryBudget(ttl=60.0, min_retries_per_sec=1000.0, percent_can_retry=0.5, _now=_fixed_clock) sync_transport = httpx2.MockTransport(_failing_handler) sync_client = Client( @@ -73,8 +81,9 @@ def test_shared_budget_across_sync_threads_and_async_loop() -> None: # The lock kept the budget's internal deques consistent — no IndexError, no corruption. # 0.8.3 deposit-hoist: deposits count requests, not attempts (one per __call__, - # regardless of max_attempts). Budget TTL is 60.0 so no purge fires during the - # sub-second runtime; the count is exact. + # regardless of max_attempts). The pinned clock ensures _purge never evicts any + # deposit (all timestamps are 0.0, cutoff is 0.0 - 60.0 = -60.0 < 0.0, so the + # strict `< cutoff` predicate is always False), making the deposit count exact. expected_deposits = (_N_SYNC_THREADS * _N_OPS_PER_THREAD) + _N_ASYNC_TASKS assert len(budget._deposits) == expected_deposits, ( # noqa: SLF001 f"expected {expected_deposits} deposits, got {len(budget._deposits)}" # noqa: SLF001 From 6eebd839f12113756e3e770d410bfe9dbdbeea8b Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:12:59 +0300 Subject: [PATCH 12/14] test(observability): assert log record fires in the no-active-span path 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) --- tests/test_observability.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/tests/test_observability.py b/tests/test_observability.py index e3c5bd5..9c3fb03 100644 --- a/tests/test_observability.py +++ b/tests/test_observability.py @@ -119,20 +119,29 @@ def test_emit_event_calls_add_event_when_otel_installed() -> None: mock_span.add_event.assert_called_once_with("test.event", attributes={"k": "v"}) -def test_emit_event_works_when_otel_installed_but_no_active_span() -> None: +def test_emit_event_works_when_otel_installed_but_no_active_span( + caplog: pytest.LogCaptureFixture, +) -> None: """With OTel installed but no tracer configured, get_current_span() returns NonRecordingSpan. - add_event is a documented no-op. No error. + add_event is a documented no-op. The log-only fallback path must still emit + a record at the requested level with the correct event attribute. """ # Real OTel API call (no mocking) — opentelemetry-api is installed via the otel extra. - _emit_event( - _TEST_LOGGER, - "test.event", - level=logging.WARNING, - message="real-otel-but-no-tracer", - attributes={"a": 1}, - ) - # No assertion needed — the absence of an exception IS the assertion. + with caplog.at_level(logging.WARNING, logger="httpware.test.observability"): + _emit_event( + _TEST_LOGGER, + "test.event", + level=logging.WARNING, + message="real-otel-but-no-tracer", + attributes={"a": 1}, + ) + + assert len(caplog.records) == 1 + record = caplog.records[0] + assert record.levelno == logging.WARNING + assert record.message == "real-otel-but-no-tracer" + assert record.event == "test.event" # ty: ignore[unresolved-attribute] def test_emit_event_swallows_add_event_failure() -> None: From 76cf6c298a88695be7888dde8d38050bcc6fb4b2 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:13:58 +0300 Subject: [PATCH 13/14] test(circuit_breaker): suppress A004 for intentional TimeoutError import 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) --- tests/test_circuit_breaker.py | 4 ++-- tests/test_circuit_breaker_sync.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_circuit_breaker.py b/tests/test_circuit_breaker.py index 74d623d..5d8a47e 100644 --- a/tests/test_circuit_breaker.py +++ b/tests/test_circuit_breaker.py @@ -21,7 +21,7 @@ NotFoundError, RateLimitedError, ServiceUnavailableError, - TimeoutError, + TimeoutError, # noqa: A004 — intentional: httpware.TimeoutError shadows the builtin ) from httpware.middleware.resilience.circuit_breaker import AsyncCircuitBreaker @@ -171,7 +171,7 @@ def _raise(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 async def test_timeout_error_counts_as_failure() -> None: - def _raise(request: httpx2.Request) -> httpx2.Response: # noqa: ARG001 + def _raise(request: httpx2.Request) -> httpx2.Response: msg = "read timed out" raise httpx2.ReadTimeout(msg, request=request) diff --git a/tests/test_circuit_breaker_sync.py b/tests/test_circuit_breaker_sync.py index f4b0d2b..8f23144 100644 --- a/tests/test_circuit_breaker_sync.py +++ b/tests/test_circuit_breaker_sync.py @@ -16,7 +16,7 @@ NotFoundError, RateLimitedError, ServiceUnavailableError, - TimeoutError, + TimeoutError, # noqa: A004 — intentional: httpware.TimeoutError shadows the builtin ) from httpware.middleware.resilience.circuit_breaker import CircuitBreaker From 50fdedd667b4f6104e25cff41d5d2d5ea5d0920f Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 14 Jun 2026 17:17:07 +0300 Subject: [PATCH 14/14] style: import Callable from collections.abc (ruff UP035) 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) --- tests/test_error_mapping_terminal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_error_mapping_terminal.py b/tests/test_error_mapping_terminal.py index 9660756..2a05bbd 100644 --- a/tests/test_error_mapping_terminal.py +++ b/tests/test_error_mapping_terminal.py @@ -1,7 +1,7 @@ """Tests for the AsyncClient and Client internal terminal's exception mapping.""" +from collections.abc import Callable from http import HTTPStatus -from typing import Callable import httpx2 import pytest