test: close deep-audit test-quality findings

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.

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):

tests/test_circuit_breaker.py

@@ -21,6 +21,7 @@
     NotFoundError,
     RateLimitedError,
     ServiceUnavailableError,
+    TimeoutError,  # noqa: A004 — intentional: httpware.TimeoutError shadows the builtin
 )
 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:
+        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])

tests/test_circuit_breaker_sync.py

@@ -16,6 +16,7 @@
     NotFoundError,
     RateLimitedError,
     ServiceUnavailableError,
+    TimeoutError,  # noqa: A004 — intentional: httpware.TimeoutError shadows the builtin
 )
 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

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:

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)