test: close the audit's "additional test findings" tail#74
Merged
Conversation
The 2026-06-12 code audit's formally-tracked test holes (T1–T8) shipped in
0.9.0, but the prose "Additional test findings" list below that table was
never tracked or fixed. Close all of it. One small source change (a fake
fidelity fix); everything else is test-only. 482 tests pass at 100% coverage.
Fake / subscriber fidelity:
- testing.py: FakeOutboxClient.delete_with_lease / mark_pending_with_lease now
require ``acquired_token is not None`` to match, mirroring SQL's
``NULL = NULL`` (no match). Previously a None token matched a None-token row
(``None == None``) where the real client no-ops. Pinned with a direct test.
- test_fake.py: the two loop-mode no-lease-token tests now assert the row is
preserved (the worker's early-return), not just that the handler ran.
- test_fake.py: the lease-lost terminal/retry tests now assert the lease_lost
metric fires with the right phase AND that the paired acked/nacked_retried is
suppressed (P17 emit-after-flush), not just that the handler ran.
Flake / robustness:
- test_fake.py: replace the load-sensitive ``elapsed < 0.7`` drain bound with a
graceful_timeout-relative ``< 1.6x`` bound (still catches the 2x-drain
regression, won't flake under CI load).
Coverage gaps:
- test_unit.py: pin _default_retry_strategy() to its documented parameters.
- test_relay.py: a STARTED foreign broker emits NO warning at start() (negative
of the existing unstarted-broker warning test).
- test_fastapi.py: the transactional contract end-to-end — a Depends-resolved
session flows into a chained OutboxResponse through the FastAPI wrapper, and
each delivery resolves its own fresh session (session-per-delivery); plus
OutboxRouter forwards broker kwargs to the inner broker.
- test_middleware_opentelemetry.py: assert the consume-scope span actually fires
(was: meter-only assertions, zero span coverage).
- test_metrics_opentelemetry.py: de-tautologize the two recorder tests — the
meter-precedence test now uses distinct readers to prove the meter wins over
meter_provider; the default-provider test patches get_meter and asserts the
event lands.
Convention / hygiene:
- test_unit.py: hoist the 6 inline imports (11 occurrences) to module top — all
were convenience shadows of existing top-level imports or trivially hoistable;
none were import-behavior tests.
- test_relay.py: drop stale dev-plan references ("this plan", "Task 6") from a
comment, making it self-contained.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The archived findings doc claimed nothing was deferred, but the prose "Additional test findings" tail below the T1–T8 table was never tracked or fixed until #74. Name the tail and record the PR that closed it. 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.
What
The 2026-06-12 code audit's formally-tracked test holes (T1–T8) shipped in 0.9.0, but the prose "Additional test findings" list below that table was never tracked. This closes all ~11 of them. One small source change (a fake-client fidelity fix); everything else is test-only. 482 tests pass at 100% coverage.
Substance (not just count)
FakeOutboxClientterminal writes matched aNonetoken against a None-token row (None == None) where the real SQL no-ops (NULL = NULL). Now guarded + pinned. The two loop-mode no-token tests and the lease-lost tests previously asserted only "the handler ran" — now they assert the row is preserved /lease_lostfires with the right phase / the pairedacked/nacked_retriedis suppressed (P17).elapsed < 0.7drain bound → agraceful_timeout-relative< 1.6xbound (still catches the 2x-drain regression).OutboxResponsethrough the wrapper + session-per-delivery); OTel consume-span assertion (was meter-only); started-foreign-broker no-warn; router kwarg forwarding;_default_retry_strategy()pinned.get_meterand asserts the event lands).test_unit.py(all redundant shadows / trivially hoistable, no import-behavior tests); dropped stale dev-plan references from atest_relay.pycomment.Verification
just test— 482 passed, 100% coverage (the--cov-fail-under=100gate).just lint-ci— clean.A follow-up commit corrects the archived code-audit findings doc, which had claimed "nothing deferred."
🤖 Generated with Claude Code