From 96528f300d592e51686884d6f834c8ca96a7dbf1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 19:05:38 +0000 Subject: [PATCH 1/5] test: add sync/async parity coverage for get_team and get_person 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 Claude-Session: https://claude.ai/code/session_01RYDDda51C9LGsiv2cS9Pkr --- tests/test_sync_async_parity.py | 151 ++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 tests/test_sync_async_parity.py diff --git a/tests/test_sync_async_parity.py b/tests/test_sync_async_parity.py new file mode 100644 index 00000000..532b7e3f --- /dev/null +++ b/tests/test_sync_async_parity.py @@ -0,0 +1,151 @@ +"""Sync/async behavioral parity tests (issue #304, batch 1). + +`Mlb` is the compatibility baseline. These tests prove that `AsyncMlb`'s +public endpoint behavior stays aligned with it: the same response produces the +same model type, the same parsed values, and the same "nothing to return" +answer. + +The scope is deliberately narrow. Transport behavior — retries, timeouts, +strict-mode status mapping, exception translation — is already covered by +tests/test_http_contract.py, tests/test_mlb_dataadapter.py and +tests/test_async_mlb_dataadapter.py, and payload parsing by tests/parsers/. +None of that is re-asserted here, and nothing compares Requests internals with +HTTPX internals. Each test drives both public clients over an equivalent canned +response and compares only what a caller can see. + +These tests must not contact the live MLB API. +""" + +from __future__ import annotations + +import asyncio + +import pytest +import requests +import requests_mock + +# The async client needs the optional HTTPX extra; without it there is no +# async side to compare against, so the whole module is skipped rather than +# failing a sync-only install at import time. +httpx = pytest.importorskip("httpx", reason="requires the async extra (HTTPX)") + +from mlbstatsapi import Mlb # noqa: E402 +from mlbstatsapi.async_mlb import AsyncMlb # noqa: E402 +from mlbstatsapi.models.people import Person # noqa: E402 +from mlbstatsapi.models.teams import Team # noqa: E402 + + +TEAM_PAYLOAD = {"teams": [{"id": 133, "link": "/api/v1/teams/133", "name": "Athletics"}]} +PERSON_PAYLOAD = { + "people": [{"id": 660271, "link": "/api/v1/people/660271", "fullName": "Shohei Ohtani"}] +} + +# The two ways a call legitimately comes back with nothing to parse. Both +# clients are expected to answer None for get_team and get_person. +NO_RESULT_RESPONSES = { + "empty 200": (200, {}), + "404": (404, {}), +} + + +def call_sync(method: str, *args, status: int, payload: dict, **kwargs): + """Call a method on `Mlb` against a canned response.""" + adapter = requests_mock.Adapter() + adapter.register_uri("GET", requests_mock.ANY, status_code=status, json=payload) + + session = requests.Session() + session.mount("https://", adapter) + + try: + with Mlb(session=session) as mlb: + return getattr(mlb, method)(*args, **kwargs) + finally: + session.close() + + +def call_async(method: str, *args, status: int, payload: dict, **kwargs): + """Call the matching method on `AsyncMlb` against the same canned response.""" + client = httpx.AsyncClient( + transport=httpx.MockTransport(lambda request: httpx.Response(status, json=payload)) + ) + + async def scenario(): + try: + async with AsyncMlb(client=client) as mlb: + return await getattr(mlb, method)(*args, **kwargs) + finally: + await client.aclose() + + return asyncio.run(scenario()) + + +def call_both(method: str, *args, status: int = 200, payload: dict, **kwargs): + """Return the sync and async results for one call, in that order. + + Both clients are handed an equivalent response through their own public + constructor, so a failure below names the side that drifted. + """ + return ( + call_sync(method, *args, status=status, payload=payload, **kwargs), + call_async(method, *args, status=status, payload=payload, **kwargs), + ) + + +# --------------------------------------------------------------------------- +# get_team +# --------------------------------------------------------------------------- + + +def test_get_team_success_parity(): + """A successful team response parses to the same Team on both clients.""" + sync_team, async_team = call_both("get_team", 133, payload=TEAM_PAYLOAD) + + assert isinstance(sync_team, Team), "sync get_team did not return a Team" + assert isinstance(async_team, Team), "async get_team did not return a Team" + + expected = (133, "/api/v1/teams/133", "Athletics") + assert (sync_team.id, sync_team.link, sync_team.name) == expected + assert (async_team.id, async_team.link, async_team.name) == expected + assert async_team == sync_team + + +@pytest.mark.parametrize("label", list(NO_RESULT_RESPONSES)) +def test_get_team_no_result_parity(label): + """An empty success and a 404 both return None on either client.""" + status, payload = NO_RESULT_RESPONSES[label] + + sync_team, async_team = call_both("get_team", 133, status=status, payload=payload) + + assert sync_team is None, f"sync get_team returned {sync_team!r} for {label}" + assert async_team is None, f"async get_team returned {async_team!r} for {label}" + + +# --------------------------------------------------------------------------- +# get_person +# --------------------------------------------------------------------------- + + +def test_get_person_success_parity(): + """A successful person response parses to the same Person on both clients.""" + sync_person, async_person = call_both("get_person", 660271, payload=PERSON_PAYLOAD) + + assert isinstance(sync_person, Person), "sync get_person did not return a Person" + assert isinstance(async_person, Person), "async get_person did not return a Person" + + expected = (660271, "/api/v1/people/660271", "Shohei Ohtani") + assert (sync_person.id, sync_person.link, sync_person.full_name) == expected + assert (async_person.id, async_person.link, async_person.full_name) == expected + assert async_person == sync_person + + +@pytest.mark.parametrize("label", list(NO_RESULT_RESPONSES)) +def test_get_person_no_result_parity(label): + """An empty success and a 404 both return None on either client.""" + status, payload = NO_RESULT_RESPONSES[label] + + sync_person, async_person = call_both( + "get_person", 660271, status=status, payload=payload + ) + + assert sync_person is None, f"sync get_person returned {sync_person!r} for {label}" + assert async_person is None, f"async get_person returned {async_person!r} for {label}" From c4a354701ebea2269fcd59992dabccc7aaac9310 Mon Sep 17 00:00:00 2001 From: Matthew Spah Date: Fri, 21 Aug 2026 18:44:27 -0700 Subject: [PATCH 2/5] test: add get_schedule sync async parity coverage --- tests/test_sync_async_parity.py | 199 ++++++++++++++++++++++++++++++-- 1 file changed, 188 insertions(+), 11 deletions(-) diff --git a/tests/test_sync_async_parity.py b/tests/test_sync_async_parity.py index 532b7e3f..c306078b 100644 --- a/tests/test_sync_async_parity.py +++ b/tests/test_sync_async_parity.py @@ -1,4 +1,4 @@ -"""Sync/async behavioral parity tests (issue #304, batch 1). +"""Sync/async behavioral parity tests (issue #304, batches 1-2). `Mlb` is the compatibility baseline. These tests prove that `AsyncMlb`'s public endpoint behavior stays aligned with it: the same response produces the @@ -19,6 +19,7 @@ from __future__ import annotations import asyncio +from urllib.parse import parse_qsl, urlsplit import pytest import requests @@ -29,9 +30,9 @@ # failing a sync-only install at import time. httpx = pytest.importorskip("httpx", reason="requires the async extra (HTTPX)") -from mlbstatsapi import Mlb # noqa: E402 -from mlbstatsapi.async_mlb import AsyncMlb # noqa: E402 +from mlbstatsapi import AsyncMlb, Mlb # noqa: E402 from mlbstatsapi.models.people import Person # noqa: E402 +from mlbstatsapi.models.schedules import Schedule # noqa: E402 from mlbstatsapi.models.teams import Team # noqa: E402 @@ -39,6 +40,22 @@ PERSON_PAYLOAD = { "people": [{"id": 660271, "link": "/api/v1/people/660271", "fullName": "Shohei Ohtani"}] } +SCHEDULE_PAYLOAD = { + "totalItems": 1, + "totalEvents": 0, + "totalGames": 1, + "totalGamesInProgress": 0, + "dates": [ + { + "date": "2022-10-07", + "totalItems": 1, + "totalEvents": 0, + "totalGames": 1, + "totalGamesInProgress": 0, + "games": [], + } + ], +} # The two ways a call legitimately comes back with nothing to parse. Both # clients are expected to answer None for get_team and get_person. @@ -46,9 +63,35 @@ "empty 200": (200, {}), "404": (404, {}), } +SCHEDULE_NO_RESULT_RESPONSES = { + "empty 200": ( + 200, + { + "totalItems": 0, + "totalEvents": 0, + "totalGames": 0, + "totalGamesInProgress": 0, + "dates": [], + }, + ), + "404": (404, {}), +} + + +def request_signature(method: str, url: str) -> tuple[str, str, dict[str, str]]: + """Normalize one observed request for transport-independent comparison.""" + parsed_url = urlsplit(url) + return method, parsed_url.path, dict(parse_qsl(parsed_url.query)) -def call_sync(method: str, *args, status: int, payload: dict, **kwargs): +def call_sync( + method: str, + *args, + status: int, + payload: dict, + request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, + **kwargs, +): """Call a method on `Mlb` against a canned response.""" adapter = requests_mock.Adapter() adapter.register_uri("GET", requests_mock.ANY, status_code=status, json=payload) @@ -58,15 +101,35 @@ def call_sync(method: str, *args, status: int, payload: dict, **kwargs): try: with Mlb(session=session) as mlb: - return getattr(mlb, method)(*args, **kwargs) + result = getattr(mlb, method)(*args, **kwargs) + + if request_signatures is not None: + assert len(adapter.request_history) == 1 + request = adapter.request_history[0] + request_signatures.append(request_signature(request.method, request.url)) + + return result finally: session.close() -def call_async(method: str, *args, status: int, payload: dict, **kwargs): +def call_async( + method: str, + *args, + status: int, + payload: dict, + request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, + **kwargs, +): """Call the matching method on `AsyncMlb` against the same canned response.""" + requests_seen = [] + + def handler(request): + requests_seen.append(request) + return httpx.Response(status, json=payload) + client = httpx.AsyncClient( - transport=httpx.MockTransport(lambda request: httpx.Response(status, json=payload)) + transport=httpx.MockTransport(handler) ) async def scenario(): @@ -76,18 +139,46 @@ async def scenario(): finally: await client.aclose() - return asyncio.run(scenario()) + result = asyncio.run(scenario()) + if request_signatures is not None: + assert len(requests_seen) == 1 + request = requests_seen[0] + request_signatures.append(request_signature(request.method, str(request.url))) -def call_both(method: str, *args, status: int = 200, payload: dict, **kwargs): + return result + + +def call_both( + method: str, + *args, + status: int = 200, + payload: dict, + request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, + **kwargs, +): """Return the sync and async results for one call, in that order. Both clients are handed an equivalent response through their own public constructor, so a failure below names the side that drifted. """ return ( - call_sync(method, *args, status=status, payload=payload, **kwargs), - call_async(method, *args, status=status, payload=payload, **kwargs), + call_sync( + method, + *args, + status=status, + payload=payload, + request_signatures=request_signatures, + **kwargs, + ), + call_async( + method, + *args, + status=status, + payload=payload, + request_signatures=request_signatures, + **kwargs, + ), ) @@ -149,3 +240,89 @@ def test_get_person_no_result_parity(label): assert sync_person is None, f"sync get_person returned {sync_person!r} for {label}" assert async_person is None, f"async get_person returned {async_person!r} for {label}" + + +# --------------------------------------------------------------------------- +# get_schedule +# --------------------------------------------------------------------------- + + +def test_get_schedule_success_parity(): + """A successful date schedule parses identically and sends the same request.""" + requests_seen = [] + + sync_schedule, async_schedule = call_both( + "get_schedule", + date="2022-10-07", + payload=SCHEDULE_PAYLOAD, + request_signatures=requests_seen, + ) + + assert isinstance(sync_schedule, Schedule), "sync get_schedule did not return a Schedule" + assert isinstance(async_schedule, Schedule), "async get_schedule did not return a Schedule" + + expected = (1, 1, "2022-10-07", 1) + assert ( + sync_schedule.total_items, + sync_schedule.total_games, + sync_schedule.dates[0].date, + sync_schedule.dates[0].total_games, + ) == expected + assert ( + async_schedule.total_items, + async_schedule.total_games, + async_schedule.dates[0].date, + async_schedule.dates[0].total_games, + ) == expected + assert async_schedule == sync_schedule + + expected_request = ( + "GET", + "/api/v1/schedule", + {"date": "2022-10-07", "sportId": "1"}, + ) + assert requests_seen == [expected_request, expected_request] + + +@pytest.mark.parametrize("label", list(SCHEDULE_NO_RESULT_RESPONSES)) +def test_get_schedule_no_result_parity(label): + """An empty success and a 404 both return None on either client.""" + status, payload = SCHEDULE_NO_RESULT_RESPONSES[label] + + sync_schedule, async_schedule = call_both( + "get_schedule", + date="2022-10-07", + status=status, + payload=payload, + ) + + assert sync_schedule is None, f"sync get_schedule returned {sync_schedule!r} for {label}" + assert async_schedule is None, f"async get_schedule returned {async_schedule!r} for {label}" + + +def test_get_schedule_range_team_and_sport_request_parity(): + """A date range, team, and non-default sport produce equivalent requests.""" + requests_seen = [] + + sync_schedule, async_schedule = call_both( + "get_schedule", + start_date="2022-10-07", + end_date="2022-10-09", + team_id=133, + sport_id=11, + payload=SCHEDULE_PAYLOAD, + request_signatures=requests_seen, + ) + + assert async_schedule == sync_schedule + expected_request = ( + "GET", + "/api/v1/schedule", + { + "startDate": "2022-10-07", + "endDate": "2022-10-09", + "teamId": "133", + "sportId": "11", + }, + ) + assert requests_seen == [expected_request, expected_request] From 076d7a8775adefd7ba4ab4c631098201025aa594 Mon Sep 17 00:00:00 2001 From: Matthew Spah Date: Sat, 22 Aug 2026 04:42:39 -0700 Subject: [PATCH 3/5] test: add sync async failure parity coverage --- tests/test_sync_async_parity.py | 218 ++++++++++++++++++++++++++++++-- 1 file changed, 208 insertions(+), 10 deletions(-) diff --git a/tests/test_sync_async_parity.py b/tests/test_sync_async_parity.py index c306078b..4ae9d5e5 100644 --- a/tests/test_sync_async_parity.py +++ b/tests/test_sync_async_parity.py @@ -1,17 +1,17 @@ -"""Sync/async behavioral parity tests (issue #304, batches 1-2). +"""Sync/async behavioral parity tests (issue #304, batches 1-3). `Mlb` is the compatibility baseline. These tests prove that `AsyncMlb`'s public endpoint behavior stays aligned with it: the same response produces the same model type, the same parsed values, and the same "nothing to return" answer. -The scope is deliberately narrow. Transport behavior — retries, timeouts, -strict-mode status mapping, exception translation — is already covered by +The scope is deliberately narrow. Detailed transport behavior — retries, +timing, backoff, and transport-specific context — is already covered by tests/test_http_contract.py, tests/test_mlb_dataadapter.py and tests/test_async_mlb_dataadapter.py, and payload parsing by tests/parsers/. None of that is re-asserted here, and nothing compares Requests internals with HTTPX internals. Each test drives both public clients over an equivalent canned -response and compares only what a caller can see. +response or failure and compares only what a caller can see. These tests must not contact the live MLB API. """ @@ -19,6 +19,7 @@ from __future__ import annotations import asyncio +from http import HTTPStatus from urllib.parse import parse_qsl, urlsplit import pytest @@ -30,7 +31,15 @@ # failing a sync-only install at import time. httpx = pytest.importorskip("httpx", reason="requires the async extra (HTTPX)") -from mlbstatsapi import AsyncMlb, Mlb # noqa: E402 +from mlbstatsapi import ( # noqa: E402 + AsyncMlb, + Mlb, + MlbDecodeError, + MlbHttpCompatibilityWarning, + MlbHttpError, + MlbTimeoutError, + MlbTransportError, +) from mlbstatsapi.models.people import Person # noqa: E402 from mlbstatsapi.models.schedules import Schedule # noqa: E402 from mlbstatsapi.models.teams import Team # noqa: E402 @@ -88,19 +97,52 @@ def call_sync( method: str, *args, status: int, - payload: dict, + payload: dict | None = None, + raw_body: bytes | None = None, + failure: str | None = None, + mlb_options: dict | None = None, request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, **kwargs, ): """Call a method on `Mlb` against a canned response.""" adapter = requests_mock.Adapter() - adapter.register_uri("GET", requests_mock.ANY, status_code=status, json=payload) + if failure == "timeout": + adapter.register_uri( + "GET", + requests_mock.ANY, + exc=requests.exceptions.Timeout("timed out"), + ) + elif failure == "transport": + adapter.register_uri( + "GET", + requests_mock.ANY, + exc=requests.exceptions.ConnectionError("connection refused"), + ) + elif failure is not None: + raise ValueError(f"Unsupported canned failure: {failure}") + elif raw_body is not None: + adapter.register_uri( + "GET", + requests_mock.ANY, + status_code=status, + content=raw_body, + reason=HTTPStatus(status).phrase, + ) + else: + assert payload is not None + adapter.register_uri( + "GET", + requests_mock.ANY, + status_code=status, + json=payload, + reason=HTTPStatus(status).phrase, + ) session = requests.Session() session.mount("https://", adapter) try: - with Mlb(session=session) as mlb: + with Mlb(session=session, **(mlb_options or {})) as mlb: result = getattr(mlb, method)(*args, **kwargs) if request_signatures is not None: @@ -117,7 +159,10 @@ def call_async( method: str, *args, status: int, - payload: dict, + payload: dict | None = None, + raw_body: bytes | None = None, + failure: str | None = None, + mlb_options: dict | None = None, request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, **kwargs, ): @@ -126,6 +171,16 @@ def call_async( def handler(request): requests_seen.append(request) + if failure == "timeout": + raise httpx.ReadTimeout("timed out", request=request) + if failure == "transport": + raise httpx.ConnectError("connection refused", request=request) + if failure is not None: + raise ValueError(f"Unsupported canned failure: {failure}") + if raw_body is not None: + return httpx.Response(status, content=raw_body) + + assert payload is not None return httpx.Response(status, json=payload) client = httpx.AsyncClient( @@ -134,7 +189,7 @@ def handler(request): async def scenario(): try: - async with AsyncMlb(client=client) as mlb: + async with AsyncMlb(client=client, **(mlb_options or {})) as mlb: return await getattr(mlb, method)(*args, **kwargs) finally: await client.aclose() @@ -326,3 +381,146 @@ def test_get_schedule_range_team_and_sport_request_parity(): }, ) assert requests_seen == [expected_request, expected_request] + + +# --------------------------------------------------------------------------- +# Representative public failure behavior (get_team) +# --------------------------------------------------------------------------- + + +def assert_http_error_parity( + sync_error: MlbHttpError, + async_error: MlbHttpError, + *, + status_code: int, + reason: str, + response_data: dict, +): + """Compare stable public HTTP error context without transport internals.""" + expected = ( + status_code, + reason, + "GET", + "https://statsapi.mlb.com/api/v1/teams/133", + response_data, + ) + attributes = ("status_code", "reason", "method", "url", "response_data") + + assert tuple(getattr(sync_error, name) for name in attributes) == expected + assert tuple(getattr(async_error, name) for name in attributes) == expected + + +def test_get_team_strict_client_error_parity(): + """Strict non-404 4xx responses expose equivalent public error context.""" + payload = {"message": "access denied"} + options = {"strict_http": True} + + with pytest.raises(MlbHttpError) as sync_exc: + call_sync( + "get_team", + 133, + status=403, + payload=payload, + mlb_options=options, + ) + with pytest.raises(MlbHttpError) as async_exc: + call_async( + "get_team", + 133, + status=403, + payload=payload, + mlb_options=options, + ) + + assert_http_error_parity( + sync_exc.value, + async_exc.value, + status_code=403, + reason="Forbidden", + response_data=payload, + ) + + +def test_get_team_compatibility_client_error_parity(): + """Compatibility mode warns and returns None on both public clients.""" + options = {"strict_http": False} + + with pytest.warns(MlbHttpCompatibilityWarning) as sync_warnings: + sync_team = call_sync( + "get_team", + 133, + status=403, + payload={"message": "access denied"}, + mlb_options=options, + ) + with pytest.warns(MlbHttpCompatibilityWarning) as async_warnings: + async_team = call_async( + "get_team", + 133, + status=403, + payload={"message": "access denied"}, + mlb_options=options, + ) + + assert sync_team is None + assert async_team is None + assert len(sync_warnings) == len(async_warnings) == 1 + assert ( + sync_warnings[0].category + is async_warnings[0].category + is MlbHttpCompatibilityWarning + ) + assert str(sync_warnings[0].message) == str(async_warnings[0].message) + + +def test_get_team_server_error_parity(): + """One representative 5xx exposes equivalent public error context.""" + payload = {"message": "server error"} + + with pytest.raises(MlbHttpError) as sync_exc: + call_sync("get_team", 133, status=500, payload=payload) + with pytest.raises(MlbHttpError) as async_exc: + call_async("get_team", 133, status=500, payload=payload) + + assert_http_error_parity( + sync_exc.value, + async_exc.value, + status_code=500, + reason="Internal Server Error", + response_data=payload, + ) + + +def test_get_team_timeout_parity(): + """A deterministic timeout raises the same public exception on both clients.""" + with pytest.raises(MlbTimeoutError) as sync_exc: + call_sync("get_team", 133, status=200, failure="timeout") + with pytest.raises(MlbTimeoutError) as async_exc: + call_async("get_team", 133, status=200, failure="timeout") + + assert type(sync_exc.value) is type(async_exc.value) is MlbTimeoutError + assert str(sync_exc.value) == str(async_exc.value) + + +def test_get_team_transport_failure_parity(): + """A generic transport failure has the same public result on both clients.""" + with pytest.raises(MlbTransportError) as sync_exc: + call_sync("get_team", 133, status=200, failure="transport") + with pytest.raises(MlbTransportError) as async_exc: + call_async("get_team", 133, status=200, failure="transport") + + assert type(sync_exc.value) is type(async_exc.value) is MlbTransportError + assert str(sync_exc.value) == str(async_exc.value) + + +def test_get_team_invalid_json_parity(): + """Invalid JSON in a successful response raises on both public clients.""" + raw_body = b'{"teams": [' + + with pytest.raises(MlbDecodeError) as sync_exc: + call_sync("get_team", 133, status=200, raw_body=raw_body) + with pytest.raises(MlbDecodeError) as async_exc: + call_async("get_team", 133, status=200, raw_body=raw_body) + + assert type(sync_exc.value) is type(async_exc.value) is MlbDecodeError + assert str(sync_exc.value) == str(async_exc.value) From 0eec6f62b43d6386921c207f304569fa0a3a46c7 Mon Sep 17 00:00:00 2001 From: Matthew Spah Date: Sat, 22 Aug 2026 05:05:11 -0700 Subject: [PATCH 4/5] test: refine sync async parity assertions --- tests/test_sync_async_parity.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_sync_async_parity.py b/tests/test_sync_async_parity.py index 4ae9d5e5..1c04df4c 100644 --- a/tests/test_sync_async_parity.py +++ b/tests/test_sync_async_parity.py @@ -255,6 +255,15 @@ def test_get_team_success_parity(): assert async_team == sync_team +def test_get_team_empty_response_body_parity(): + """A successful response with no body returns None on both clients.""" + sync_team = call_sync("get_team", 133, status=200, raw_body=b"") + async_team = call_async("get_team", 133, status=200, raw_body=b"") + + assert sync_team is None + assert async_team is None + + @pytest.mark.parametrize("label", list(NO_RESULT_RESPONSES)) def test_get_team_no_result_parity(label): """An empty success and a 404 both return None on either client.""" @@ -470,7 +479,6 @@ def test_get_team_compatibility_client_error_parity(): is async_warnings[0].category is MlbHttpCompatibilityWarning ) - assert str(sync_warnings[0].message) == str(async_warnings[0].message) def test_get_team_server_error_parity(): @@ -499,7 +507,6 @@ def test_get_team_timeout_parity(): call_async("get_team", 133, status=200, failure="timeout") assert type(sync_exc.value) is type(async_exc.value) is MlbTimeoutError - assert str(sync_exc.value) == str(async_exc.value) def test_get_team_transport_failure_parity(): @@ -510,7 +517,6 @@ def test_get_team_transport_failure_parity(): call_async("get_team", 133, status=200, failure="transport") assert type(sync_exc.value) is type(async_exc.value) is MlbTransportError - assert str(sync_exc.value) == str(async_exc.value) def test_get_team_invalid_json_parity(): @@ -523,4 +529,3 @@ def test_get_team_invalid_json_parity(): call_async("get_team", 133, status=200, raw_body=raw_body) assert type(sync_exc.value) is type(async_exc.value) is MlbDecodeError - assert str(sync_exc.value) == str(async_exc.value) From b365df88f9021050e23e0de6a4c4b74215a3059e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 12:39:21 +0000 Subject: [PATCH 5/5] test: simplify and strengthen the sync/async parity suite 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 Claude-Session: https://claude.ai/code/session_01CqA6vNdLdccVfTPB8Dj9yq --- tests/test_sync_async_parity.py | 519 +++++++++++++------------------- 1 file changed, 206 insertions(+), 313 deletions(-) diff --git a/tests/test_sync_async_parity.py b/tests/test_sync_async_parity.py index 1c04df4c..813e0507 100644 --- a/tests/test_sync_async_parity.py +++ b/tests/test_sync_async_parity.py @@ -1,9 +1,9 @@ -"""Sync/async behavioral parity tests (issue #304, batches 1-3). +"""Sync/async behavioral parity tests (issue #304). `Mlb` is the compatibility baseline. These tests prove that `AsyncMlb`'s public endpoint behavior stays aligned with it: the same response produces the -same model type, the same parsed values, and the same "nothing to return" -answer. +same request, the same model type, the same parsed values, and the same +"nothing to return" answer. The scope is deliberately narrow. Detailed transport behavior — retries, timing, backoff, and transport-specific context — is already covered by @@ -19,7 +19,9 @@ from __future__ import annotations import asyncio +from dataclasses import dataclass from http import HTTPStatus +from typing import Any from urllib.parse import parse_qsl, urlsplit import pytest @@ -66,28 +68,44 @@ ], } -# The two ways a call legitimately comes back with nothing to parse. Both -# clients are expected to answer None for get_team and get_person. +# Every way a call legitimately comes back with nothing to parse, held as the +# keyword arguments that produce it. Both clients are expected to answer None. NO_RESULT_RESPONSES = { - "empty 200": (200, {}), - "404": (404, {}), + "empty 200": {"payload": {}}, + "empty body": {"raw_body": b""}, + "404": {"status": 404, "payload": {}}, } -SCHEDULE_NO_RESULT_RESPONSES = { - "empty 200": ( - 200, - { +# A schedule can also answer with a well-formed envelope holding no dates. +SCHEDULE_NO_RESULT_RESPONSES = NO_RESULT_RESPONSES | { + "no dates": { + "payload": { "totalItems": 0, "totalEvents": 0, "totalGames": 0, "totalGamesInProgress": 0, "dates": [], - }, + } + }, +} + +# The canned transport failures, per client. Each pair is the closest +# equivalent the two libraries offer, so the public exception is the only +# thing being compared. +SYNC_FAILURES = { + "timeout": requests.exceptions.Timeout("timed out"), + "transport": requests.exceptions.ConnectionError("connection refused"), +} +ASYNC_FAILURES = { + "timeout": lambda request: httpx.ReadTimeout("timed out", request=request), + "transport": lambda request: httpx.ConnectError( + "connection refused", request=request ), - "404": (404, {}), } +RequestSignature = tuple[str, str, dict[str, str]] + -def request_signature(method: str, url: str) -> tuple[str, str, dict[str, str]]: +def request_signature(method: str, url: str) -> RequestSignature: """Normalize one observed request for transport-independent comparison.""" parsed_url = urlsplit(url) return method, parsed_url.path, dict(parse_qsl(parsed_url.query)) @@ -96,46 +114,26 @@ def request_signature(method: str, url: str) -> tuple[str, str, dict[str, str]]: def call_sync( method: str, *args, - status: int, + status: int = 200, payload: dict | None = None, raw_body: bytes | None = None, failure: str | None = None, mlb_options: dict | None = None, - request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, + observed: list[RequestSignature] | None = None, **kwargs, ): """Call a method on `Mlb` against a canned response.""" adapter = requests_mock.Adapter() - if failure == "timeout": - adapter.register_uri( - "GET", - requests_mock.ANY, - exc=requests.exceptions.Timeout("timed out"), - ) - elif failure == "transport": - adapter.register_uri( - "GET", - requests_mock.ANY, - exc=requests.exceptions.ConnectionError("connection refused"), - ) - elif failure is not None: - raise ValueError(f"Unsupported canned failure: {failure}") - elif raw_body is not None: - adapter.register_uri( - "GET", - requests_mock.ANY, - status_code=status, - content=raw_body, - reason=HTTPStatus(status).phrase, - ) + if failure is not None: + adapter.register_uri("GET", requests_mock.ANY, exc=SYNC_FAILURES[failure]) else: - assert payload is not None + body = {"content": raw_body} if raw_body is not None else {"json": payload} adapter.register_uri( "GET", requests_mock.ANY, status_code=status, - json=payload, reason=HTTPStatus(status).phrase, + **body, ) session = requests.Session() @@ -143,243 +141,169 @@ def call_sync( try: with Mlb(session=session, **(mlb_options or {})) as mlb: - result = getattr(mlb, method)(*args, **kwargs) - - if request_signatures is not None: - assert len(adapter.request_history) == 1 - request = adapter.request_history[0] - request_signatures.append(request_signature(request.method, request.url)) - - return result + return getattr(mlb, method)(*args, **kwargs) finally: + if observed is not None: + observed.extend( + request_signature(request.method, request.url) + for request in adapter.request_history + ) + # Mlb leaves a caller-injected session open, so closing it is this + # helper's job. session.close() def call_async( method: str, *args, - status: int, + status: int = 200, payload: dict | None = None, raw_body: bytes | None = None, failure: str | None = None, mlb_options: dict | None = None, - request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, + observed: list[RequestSignature] | None = None, **kwargs, ): """Call the matching method on `AsyncMlb` against the same canned response.""" - requests_seen = [] - - def handler(request): - requests_seen.append(request) - if failure == "timeout": - raise httpx.ReadTimeout("timed out", request=request) - if failure == "transport": - raise httpx.ConnectError("connection refused", request=request) + seen: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request) if failure is not None: - raise ValueError(f"Unsupported canned failure: {failure}") + raise ASYNC_FAILURES[failure](request) if raw_body is not None: return httpx.Response(status, content=raw_body) - - assert payload is not None return httpx.Response(status, json=payload) - client = httpx.AsyncClient( - transport=httpx.MockTransport(handler) - ) - async def scenario(): + client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) try: async with AsyncMlb(client=client, **(mlb_options or {})) as mlb: return await getattr(mlb, method)(*args, **kwargs) finally: + # AsyncMlb leaves a caller-injected client open, as Mlb does above. await client.aclose() - result = asyncio.run(scenario()) - - if request_signatures is not None: - assert len(requests_seen) == 1 - request = requests_seen[0] - request_signatures.append(request_signature(request.method, str(request.url))) - - return result - - -def call_both( - method: str, - *args, - status: int = 200, - payload: dict, - request_signatures: list[tuple[str, str, dict[str, str]]] | None = None, - **kwargs, -): - """Return the sync and async results for one call, in that order. - - Both clients are handed an equivalent response through their own public - constructor, so a failure below names the side that drifted. - """ - return ( - call_sync( - method, - *args, - status=status, - payload=payload, - request_signatures=request_signatures, - **kwargs, - ), - call_async( - method, - *args, - status=status, - payload=payload, - request_signatures=request_signatures, - **kwargs, - ), - ) + try: + return asyncio.run(scenario()) + finally: + if observed is not None: + observed.extend( + request_signature(request.method, str(request.url)) + for request in seen + ) -# --------------------------------------------------------------------------- -# get_team -# --------------------------------------------------------------------------- +@dataclass(frozen=True) +class ParityResult: + """What each client returned, plus the one request they both sent.""" + sync: Any + asynchronous: Any + request: RequestSignature -def test_get_team_success_parity(): - """A successful team response parses to the same Team on both clients.""" - sync_team, async_team = call_both("get_team", 133, payload=TEAM_PAYLOAD) - assert isinstance(sync_team, Team), "sync get_team did not return a Team" - assert isinstance(async_team, Team), "async get_team did not return a Team" +def call_both(method: str, *args, **kwargs) -> ParityResult: + """Drive both public clients over one canned response. - expected = (133, "/api/v1/teams/133", "Athletics") - assert (sync_team.id, sync_team.link, sync_team.name) == expected - assert (async_team.id, async_team.link, async_team.name) == expected - assert async_team == sync_team + Each client is handed an equivalent response through its own public + constructor, so a failure below names the side that drifted. Request + parity is asserted here rather than per test, which also rules out a + client that quietly fanned one call out into several. + """ + sync_requests: list[RequestSignature] = [] + async_requests: list[RequestSignature] = [] + sync_result = call_sync(method, *args, observed=sync_requests, **kwargs) + async_result = call_async(method, *args, observed=async_requests, **kwargs) -def test_get_team_empty_response_body_parity(): - """A successful response with no body returns None on both clients.""" - sync_team = call_sync("get_team", 133, status=200, raw_body=b"") - async_team = call_async("get_team", 133, status=200, raw_body=b"") + assert len(sync_requests) == 1, f"sync sent {len(sync_requests)} requests" + assert async_requests == sync_requests, "the clients sent different requests" - assert sync_team is None - assert async_team is None + return ParityResult(sync_result, async_result, sync_requests[0]) -@pytest.mark.parametrize("label", list(NO_RESULT_RESPONSES)) -def test_get_team_no_result_parity(label): - """An empty success and a 404 both return None on either client.""" - status, payload = NO_RESULT_RESPONSES[label] +def raise_both(expected: type[BaseException], method: str, *args, **kwargs): + """Return the exception each client raised for one canned failure.""" + with pytest.raises(expected) as sync_exc: + call_sync(method, *args, **kwargs) + with pytest.raises(expected) as async_exc: + call_async(method, *args, **kwargs) - sync_team, async_team = call_both("get_team", 133, status=status, payload=payload) + # pytest.raises accepts subclasses, so pin the exact type on both sides: + # MlbTimeoutError is itself an MlbTransportError. + assert type(sync_exc.value) is type(async_exc.value) is expected - assert sync_team is None, f"sync get_team returned {sync_team!r} for {label}" - assert async_team is None, f"async get_team returned {async_team!r} for {label}" + return sync_exc.value, async_exc.value # --------------------------------------------------------------------------- -# get_person +# Successful responses # --------------------------------------------------------------------------- -def test_get_person_success_parity(): - """A successful person response parses to the same Person on both clients.""" - sync_person, async_person = call_both("get_person", 660271, payload=PERSON_PAYLOAD) - - assert isinstance(sync_person, Person), "sync get_person did not return a Person" - assert isinstance(async_person, Person), "async get_person did not return a Person" - - expected = (660271, "/api/v1/people/660271", "Shohei Ohtani") - assert (sync_person.id, sync_person.link, sync_person.full_name) == expected - assert (async_person.id, async_person.link, async_person.full_name) == expected - assert async_person == sync_person - - -@pytest.mark.parametrize("label", list(NO_RESULT_RESPONSES)) -def test_get_person_no_result_parity(label): - """An empty success and a 404 both return None on either client.""" - status, payload = NO_RESULT_RESPONSES[label] +def test_get_team_success_parity(): + """A successful team response parses to the same Team on both clients.""" + result = call_both("get_team", 133, payload=TEAM_PAYLOAD) - sync_person, async_person = call_both( - "get_person", 660271, status=status, payload=payload + assert isinstance(result.sync, Team), "sync get_team did not return a Team" + assert (result.sync.id, result.sync.link, result.sync.name) == ( + 133, + "/api/v1/teams/133", + "Athletics", ) + # Pydantic equality compares the model class too, so this pins the async + # return type as well as every parsed field. + assert result.asynchronous == result.sync + assert result.request == ("GET", "/api/v1/teams/133", {}) - assert sync_person is None, f"sync get_person returned {sync_person!r} for {label}" - assert async_person is None, f"async get_person returned {async_person!r} for {label}" +def test_get_person_success_parity(): + """A successful person response parses to the same Person on both clients.""" + result = call_both("get_person", 660271, payload=PERSON_PAYLOAD) -# --------------------------------------------------------------------------- -# get_schedule -# --------------------------------------------------------------------------- + assert isinstance(result.sync, Person), "sync get_person did not return a Person" + assert (result.sync.id, result.sync.link, result.sync.full_name) == ( + 660271, + "/api/v1/people/660271", + "Shohei Ohtani", + ) + assert result.asynchronous == result.sync + assert result.request == ("GET", "/api/v1/people/660271", {}) def test_get_schedule_success_parity(): """A successful date schedule parses identically and sends the same request.""" - requests_seen = [] - - sync_schedule, async_schedule = call_both( - "get_schedule", - date="2022-10-07", - payload=SCHEDULE_PAYLOAD, - request_signatures=requests_seen, - ) + result = call_both("get_schedule", date="2022-10-07", payload=SCHEDULE_PAYLOAD) - assert isinstance(sync_schedule, Schedule), "sync get_schedule did not return a Schedule" - assert isinstance(async_schedule, Schedule), "async get_schedule did not return a Schedule" - - expected = (1, 1, "2022-10-07", 1) - assert ( - sync_schedule.total_items, - sync_schedule.total_games, - sync_schedule.dates[0].date, - sync_schedule.dates[0].total_games, - ) == expected + assert isinstance(result.sync, Schedule), "sync get_schedule did not return a Schedule" assert ( - async_schedule.total_items, - async_schedule.total_games, - async_schedule.dates[0].date, - async_schedule.dates[0].total_games, - ) == expected - assert async_schedule == sync_schedule - - expected_request = ( + result.sync.total_items, + result.sync.total_games, + result.sync.dates[0].date, + result.sync.dates[0].total_games, + ) == (1, 1, "2022-10-07", 1) + assert result.asynchronous == result.sync + assert result.request == ( "GET", "/api/v1/schedule", {"date": "2022-10-07", "sportId": "1"}, ) - assert requests_seen == [expected_request, expected_request] - - -@pytest.mark.parametrize("label", list(SCHEDULE_NO_RESULT_RESPONSES)) -def test_get_schedule_no_result_parity(label): - """An empty success and a 404 both return None on either client.""" - status, payload = SCHEDULE_NO_RESULT_RESPONSES[label] - - sync_schedule, async_schedule = call_both( - "get_schedule", - date="2022-10-07", - status=status, - payload=payload, - ) - - assert sync_schedule is None, f"sync get_schedule returned {sync_schedule!r} for {label}" - assert async_schedule is None, f"async get_schedule returned {async_schedule!r} for {label}" def test_get_schedule_range_team_and_sport_request_parity(): """A date range, team, and non-default sport produce equivalent requests.""" - requests_seen = [] - - sync_schedule, async_schedule = call_both( + result = call_both( "get_schedule", start_date="2022-10-07", end_date="2022-10-09", team_id=133, sport_id=11, payload=SCHEDULE_PAYLOAD, - request_signatures=requests_seen, ) - assert async_schedule == sync_schedule - expected_request = ( + assert result.asynchronous == result.sync + assert result.request == ( "GET", "/api/v1/schedule", { @@ -389,7 +313,46 @@ def test_get_schedule_range_team_and_sport_request_parity(): "sportId": "11", }, ) - assert requests_seen == [expected_request, expected_request] + + +# --------------------------------------------------------------------------- +# Nothing to return +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("label", list(NO_RESULT_RESPONSES)) +def test_get_team_no_result_parity(label): + """Every no-result response returns None on either client.""" + result = call_both("get_team", 133, **NO_RESULT_RESPONSES[label]) + + assert result.sync is None, f"sync get_team returned {result.sync!r} for {label}" + assert result.asynchronous is None, ( + f"async get_team returned {result.asynchronous!r} for {label}" + ) + + +@pytest.mark.parametrize("label", list(NO_RESULT_RESPONSES)) +def test_get_person_no_result_parity(label): + """Every no-result response returns None on either client.""" + result = call_both("get_person", 660271, **NO_RESULT_RESPONSES[label]) + + assert result.sync is None, f"sync get_person returned {result.sync!r} for {label}" + assert result.asynchronous is None, ( + f"async get_person returned {result.asynchronous!r} for {label}" + ) + + +@pytest.mark.parametrize("label", list(SCHEDULE_NO_RESULT_RESPONSES)) +def test_get_schedule_no_result_parity(label): + """Every no-result response returns None on either client.""" + result = call_both( + "get_schedule", date="2022-10-07", **SCHEDULE_NO_RESULT_RESPONSES[label] + ) + + assert result.sync is None, f"sync get_schedule returned {result.sync!r} for {label}" + assert result.asynchronous is None, ( + f"async get_schedule returned {result.asynchronous!r} for {label}" + ) # --------------------------------------------------------------------------- @@ -397,135 +360,65 @@ def test_get_schedule_range_team_and_sport_request_parity(): # --------------------------------------------------------------------------- -def assert_http_error_parity( - sync_error: MlbHttpError, - async_error: MlbHttpError, - *, - status_code: int, - reason: str, - response_data: dict, -): - """Compare stable public HTTP error context without transport internals.""" +@pytest.mark.parametrize( + "status, reason", + [ + # A final non-404 4xx under the strict_http default, and one + # representative 5xx. + (403, "Forbidden"), + (500, "Internal Server Error"), + ], +) +def test_get_team_http_error_parity(status, reason): + """Both clients expose the same stable public HTTP error context.""" + payload = {"message": "no"} + + sync_error, async_error = raise_both( + MlbHttpError, "get_team", 133, status=status, payload=payload + ) + expected = ( - status_code, + status, reason, "GET", "https://statsapi.mlb.com/api/v1/teams/133", - response_data, + payload, ) attributes = ("status_code", "reason", "method", "url", "response_data") - assert tuple(getattr(sync_error, name) for name in attributes) == expected assert tuple(getattr(async_error, name) for name in attributes) == expected -def test_get_team_strict_client_error_parity(): - """Strict non-404 4xx responses expose equivalent public error context.""" - payload = {"message": "access denied"} - options = {"strict_http": True} - - with pytest.raises(MlbHttpError) as sync_exc: - call_sync( - "get_team", - 133, - status=403, - payload=payload, - mlb_options=options, - ) - with pytest.raises(MlbHttpError) as async_exc: - call_async( - "get_team", - 133, - status=403, - payload=payload, - mlb_options=options, - ) - - assert_http_error_parity( - sync_exc.value, - async_exc.value, - status_code=403, - reason="Forbidden", - response_data=payload, - ) - - def test_get_team_compatibility_client_error_parity(): """Compatibility mode warns and returns None on both public clients.""" - options = {"strict_http": False} + response = { + "status": 403, + "payload": {"message": "access denied"}, + "mlb_options": {"strict_http": False}, + } with pytest.warns(MlbHttpCompatibilityWarning) as sync_warnings: - sync_team = call_sync( - "get_team", - 133, - status=403, - payload={"message": "access denied"}, - mlb_options=options, - ) + sync_team = call_sync("get_team", 133, **response) with pytest.warns(MlbHttpCompatibilityWarning) as async_warnings: - async_team = call_async( - "get_team", - 133, - status=403, - payload={"message": "access denied"}, - mlb_options=options, - ) + async_team = call_async("get_team", 133, **response) assert sync_team is None assert async_team is None assert len(sync_warnings) == len(async_warnings) == 1 - assert ( - sync_warnings[0].category - is async_warnings[0].category - is MlbHttpCompatibilityWarning - ) - - -def test_get_team_server_error_parity(): - """One representative 5xx exposes equivalent public error context.""" - payload = {"message": "server error"} - - with pytest.raises(MlbHttpError) as sync_exc: - call_sync("get_team", 133, status=500, payload=payload) - with pytest.raises(MlbHttpError) as async_exc: - call_async("get_team", 133, status=500, payload=payload) - - assert_http_error_parity( - sync_exc.value, - async_exc.value, - status_code=500, - reason="Internal Server Error", - response_data=payload, - ) -def test_get_team_timeout_parity(): - """A deterministic timeout raises the same public exception on both clients.""" - with pytest.raises(MlbTimeoutError) as sync_exc: - call_sync("get_team", 133, status=200, failure="timeout") - with pytest.raises(MlbTimeoutError) as async_exc: - call_async("get_team", 133, status=200, failure="timeout") - - assert type(sync_exc.value) is type(async_exc.value) is MlbTimeoutError - - -def test_get_team_transport_failure_parity(): - """A generic transport failure has the same public result on both clients.""" - with pytest.raises(MlbTransportError) as sync_exc: - call_sync("get_team", 133, status=200, failure="transport") - with pytest.raises(MlbTransportError) as async_exc: - call_async("get_team", 133, status=200, failure="transport") - - assert type(sync_exc.value) is type(async_exc.value) is MlbTransportError +@pytest.mark.parametrize( + "failure, expected", + [ + ("timeout", MlbTimeoutError), + ("transport", MlbTransportError), + ], +) +def test_get_team_transport_failure_parity(failure, expected): + """A deterministic transport failure raises the same exception on both.""" + raise_both(expected, "get_team", 133, failure=failure) def test_get_team_invalid_json_parity(): """Invalid JSON in a successful response raises on both public clients.""" - raw_body = b'{"teams": [' - - with pytest.raises(MlbDecodeError) as sync_exc: - call_sync("get_team", 133, status=200, raw_body=raw_body) - with pytest.raises(MlbDecodeError) as async_exc: - call_async("get_team", 133, status=200, raw_body=raw_body) - - assert type(sync_exc.value) is type(async_exc.value) is MlbDecodeError + raise_both(MlbDecodeError, "get_team", 133, raw_body=b'{"teams": [')