Skip to content

Add live sync and async smoke tests - #320

Merged
Mattsface merged 2 commits into
release/1.1.0from
test/318-live-sync-async-smoke
Aug 21, 2026
Merged

Add live sync and async smoke tests#320
Mattsface merged 2 commits into
release/1.1.0from
test/318-live-sync-async-smoke

Conversation

@Mattsface

Copy link
Copy Markdown
Member

Why

Now that AsyncMlb is in place, I want a small live test suite that proves the sync and async clients both work against the real MLB Stats API.

The goal is not broad endpoint coverage. This is just enough live testing to catch problems that deterministic tests cannot, like a bad endpoint path or request shape.

Closes #318

What

Added matching live smoke tests for the initial shared endpoints:

get_team
get_person
get_schedule

There are three sync tests using Mlb and three matching async tests using AsyncMlb.

The assertions are intentionally small and only check stable behavior like the returned model type, known IDs, and whether a schedule contains dates.

These tests stay under tests/external_tests/ and are not part of normal offline CI.

Tests

Live tests can be run with:

poetry run pytest tests/external_tests/mlb/test_mlb_smoke.py tests/external_tests/async_mlb/test_async_mlb_smoke.py

@Mattsface

Copy link
Copy Markdown
Member Author

@claude take a look please

@claude

claude Bot commented Aug 21, 2026

Copy link
Copy Markdown

Claude finished @Mattsface's task in 1m 41s —— View job


Review: Add live sync and async smoke tests

  • Gather context (diff, repo guidelines)
  • Understand request (code review)
  • Review tests/external_tests/mlb/test_mlb_smoke.py
  • Review tests/external_tests/async_mlb/test_async_mlb_smoke.py
  • Post review feedback

The tests themselves are clean, minimal, and match the stated goal — small, stable assertions (isinstance, known ID, schedule.dates truthy), a fixed historical date (2022-10-07) to avoid "today has no games" flakiness, and no new test dependency (manual asyncio.run() instead of pulling in pytest-asyncio). Good match to the existing Mlb()/AsyncMlb() context-manager patterns already used elsewhere in the repo.

One real issue: this will break the scheduled "External MLB API Tests" workflow.

.github/workflows/external-tests.yml:29 installs deps with plain poetry install --no-interaction (no -E async), then runs pytest tests/external_tests/ -v (.github/workflows/external-tests.yml:33). That directory now includes tests/external_tests/async_mlb/test_async_mlb_smoke.py, which does from mlbstatsapi import AsyncMlb at module scope.

AsyncMlb is lazily resolved via mlbstatsapi/__init__.py's __getattr__, which imports mlbstatsapi/async_mlb.pymlbstatsapi/async_mlb_dataadapter.py, and that module calls import_httpx() at module load time (mlbstatsapi/async_mlb_dataadapter.py:30). Since httpx is an optional extra (pyproject.toml) and isn't installed by this workflow, that import raises ImportError at collection time, e.g.:

Async support requires the optional HTTPX dependency, which is not installed. Install it with:
    pip install "python-mlb-statsapi[async]"

This wasn't a problem before because nothing under tests/external_tests/ imported AsyncMlb. Compare with build-and-test.yml, which correctly installs -E async before running the offline suite. The fix is to add -E async to the poetry install step in .github/workflows/external-tests.yml:29 so the scheduled live-test run can collect the new async file. Fix this →

Minor/optional:

  • tests/external_tests/mlb/test_mlb_smoke.py and tests/external_tests/async_mlb/test_async_mlb_smoke.py aren't run in the offline CI (build-and-test.yml excludes tests/external_tests), so this only ever executes locally or via the weekly scheduled workflow — worth confirming that's the intended coverage story once the extras fix above lands.

No changes were implemented — this was a review-only request.

@Mattsface
Mattsface merged commit 74496bc into release/1.1.0 Aug 21, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant