test: close deep-audit test-quality findings (#65)

* docs(planning): audit-test-quality change bundle

* 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) <[email protected]>

* test(errors): cover CookieConflict→TransportError mapping branch

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) <[email protected]>

* 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) <[email protected]>

* 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) <[email protected]>

* 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) <[email protected]>

* test(client): sync Client overload typing tests

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) <[email protected]>

* 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) <[email protected]>

* test(bulkhead): replace flaky sleep with deterministic slot-acquisition 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) <[email protected]>

* 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) <[email protected]>

* test(budget): pin clock so the no-purge deposit-count invariant is enforced

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) <[email protected]>

* 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) <[email protected]>

* 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) <[email protected]>

* 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) <[email protected]>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

Author: lesnik512

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)