Add sync/async behavioral parity tests - #321
Merged
Merged
Conversation
Batch 1 of issue #304. Adds offline parity tests that drive the public Mlb and AsyncMlb clients over equivalent canned responses and compare only what a caller can see: - successful 2xx produces the same model type and parsed values - a successful empty response returns None on both clients - a 404 returns None on both clients Transport behavior is already covered elsewhere, so nothing here compares Requests and HTTPX internals. Later #304 batches cover get_schedule, strict non-404 4xx, compatibility mode, 5xx, timeout, transport and decode errors. Refs #304 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RYDDda51C9LGsiv2cS9Pkr
Restructure the #304 parity tests around a single `call_both` helper that always captures and compares both clients' requests, rather than an opt-in `request_signatures` list threaded through three helper signatures. Every parity test now checks request parity, not just the two get_schedule cases. Injecting endpoint drift into AsyncMlb.get_team is caught by six tests instead of two: the MockTransport handler answers any path, so a wrong async endpoint previously slipped past the success and no-result tests and showed up only in the URL carried by MlbHttpError. Other cleanups: - Hold the no-result responses as the keyword arguments that produce them, so the empty-body case folds into the table and covers get_person and get_schedule too. The schedule table now extends the shared one with its empty-envelope case instead of restating 404. - Table-drive the two canned transport failures per client, replacing the branch-per-failure dispatch duplicated in both helpers. The async side now rejects an unknown failure name at the same point the sync side does, instead of lazily inside the transport handler. - Merge the two MlbHttpError tests and the two transport-failure tests into parametrized pairs, and share the pytest.raises pairing in `raise_both`. The exact-type assertion stays: MlbTimeoutError subclasses MlbTransportError, which pytest.raises alone would not distinguish. - Drop the duplicated per-field assertions on the async result. Pydantic equality compares the model class, so asserting the sync type plus cross-client equality pins the async type and every field. - Default `status` to 200 so failure cases stop passing a status that is never used, and build the httpx client inside the coroutine. 424 lines and 20 tests, from 531 lines and 17 tests. Full offline suite: 765 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CqA6vNdLdccVfTPB8Dj9yq
Member
Author
|
@claude take a look |
|
Claude finished @Mattsface's task in 1m 39s —— View job Review of
|
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
2 participants
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.
Why
Add deterministic coverage to protect against behavioral drift between the synchronous Mlb client and the new AsyncMlb client.
The sync client is the compatibility baseline. These tests verify that equivalent inputs and responses produce equivalent caller-visible behavior without requiring the sync and async transports to have identical internals.
Closes #304
What
Added sync/async parity tests covering:
Shared transport failures are tested through one representative endpoint rather than duplicating the same error matrix for every endpoint.
No production code was changed.
Tests
Tested with the deterministic parity suite:
poetry run pytest tests/test_sync_async_parity.py
Also ran the full offline test suite:
poetry run pytest tests/ --ignore=tests/external_tests
And checked the diff with:
git diff --check
No live MLB API calls are used by the new parity tests.
Risk and impact
Risk: Minimal
This PR only adds deterministic tests and does not change production behavior or the public API.
If something does go wrong, the likely impact would be an overly strict or incorrect test causing CI failures. Runtime library behavior for users would not be affected.