fix: add @pytest.mark.asyncio to async test methods (#21) #30

Claude · 2026-03-20T19:34:04Z

Claude commented

2026-03-20 19:34:04 +00:00

Closes #21

Summary

Added @pytest.mark.asyncio to all 14 async def test methods in tests/test_automated_data_fetcher.py
Without this decorator, pytest collects async tests and silently "passes" them without executing the coroutine body, giving false coverage confidence

Files Changed

tests/test_automated_data_fetcher.py — 14 decorators added

Tests

No test infrastructure to run. Changes verified by reading back the modified file: every async def test_* method now has @pytest.mark.asyncio immediately before it.

Note: pyproject.toml already has asyncio_mode = "auto" which makes the marker optional at runtime, but adding it explicitly makes intent unambiguous and protects against future config changes.

Other observations

tests/test_helpers.py:4-7 (issue #16) has a related test quality problem worth addressing separately.

Closes #21 ## Summary - Added `@pytest.mark.asyncio` to all 14 `async def` test methods in `tests/test_automated_data_fetcher.py` - Without this decorator, pytest collects async tests and silently "passes" them without executing the coroutine body, giving false coverage confidence ## Files Changed - `tests/test_automated_data_fetcher.py` — 14 decorators added ## Tests No test infrastructure to run. Changes verified by reading back the modified file: every `async def test_*` method now has `@pytest.mark.asyncio` immediately before it. Note: `pyproject.toml` already has `asyncio_mode = "auto"` which makes the marker optional at runtime, but adding it explicitly makes intent unambiguous and protects against future config changes. ## Other observations - `tests/test_helpers.py:4-7` (issue #16) has a related test quality problem worth addressing separately.

Claude added 1 commit 2026-03-20 19:34:04 +00:00

fix: add @pytest.mark.asyncio to async test methods (#21 ) ee4dae0985

Closes #21

All 14 async test methods in tests/test_automated_data_fetcher.py were
missing @pytest.mark.asyncio. Without it, pytest collects them and
silently passes without executing the coroutine body, providing no
coverage.

Added explicit @pytest.mark.asyncio to each async def test_* method.
This makes the async intent unambiguous and is robust against any
future asyncio_mode configuration changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-03-20 19:34:12 +00:00

Fix async test methods missing `pytest.mark.asyncio` decorator #21

Claude added the

ai-reviewing

label 2026-03-20 19:45:48 +00:00

Claude reviewed 2026-03-20 19:47:40 +00:00

Claude left a comment

Review: Non-standard decorator ordering on all 14 additions

The intent is correct — adding @pytest.mark.asyncio prevents silent false-positive test passes if asyncio_mode is ever changed from "auto". However, all 14 placements use non-standard ordering.

Issue: `@pytest.mark.asyncio` placed below `@patch(...)` instead of above it

The standard convention (per pytest-asyncio docs) is for the asyncio marker to be the outermost decorator — topmost in the source:

# ✅ Standard
@pytest.mark.asyncio
@patch("automated_data_fetcher.pb.batting_stats_bref")
@patch("automated_data_fetcher.pb.pitching_stats_bref")
async def test_fetch_baseball_reference_data(self, mock_pitching, mock_batting, ...):
    ...

# ❌ Current PR — marker sandwiched below @patch decorators
@patch("automated_data_fetcher.pb.batting_stats_bref")
@patch("automated_data_fetcher.pb.pitching_stats_bref")
@pytest.mark.asyncio
async def test_fetch_baseball_reference_data(self, mock_pitching, mock_batting, ...):
    ...

Why it matters

With asyncio_mode = "auto" (pyproject.toml:49) and unittest.mock.patch's use of functools.wraps (which propagates __dict__ including pytestmark), the current ordering works at runtime. But:

If asyncio_mode is ever switched to "strict", whether the mark on the inner (unwrapped) function propagates through @patch wrappers is an implementation detail of unittest.mock — not something to rely on.
The whole rationale for adding the marker explicitly (resilience against future config changes) is undermined if the placement itself is fragile.
pytest-asyncio documentation consistently shows the asyncio marker as the outermost decorator.

Fix

Move @pytest.mark.asyncio to be the topmost decorator on all 14 methods. Mechanical change — just move each line above the @patch(...) stack.

The core fix (adding the marker at all) is sound. Recommend correcting placement before merging.

## Review: Non-standard decorator ordering on all 14 additions The intent is correct — adding `@pytest.mark.asyncio` prevents silent false-positive test passes if `asyncio_mode` is ever changed from `"auto"`. However, all 14 placements use non-standard ordering. ### Issue: `@pytest.mark.asyncio` placed below `@patch(...)` instead of above it The standard convention (per pytest-asyncio docs) is for the asyncio marker to be the **outermost** decorator — topmost in the source: ```python # ✅ Standard @pytest.mark.asyncio @patch("automated_data_fetcher.pb.batting_stats_bref") @patch("automated_data_fetcher.pb.pitching_stats_bref") async def test_fetch_baseball_reference_data(self, mock_pitching, mock_batting, ...): ... # ❌ Current PR — marker sandwiched below @patch decorators @patch("automated_data_fetcher.pb.batting_stats_bref") @patch("automated_data_fetcher.pb.pitching_stats_bref") @pytest.mark.asyncio async def test_fetch_baseball_reference_data(self, mock_pitching, mock_batting, ...): ... ``` ### Why it matters With `asyncio_mode = "auto"` (pyproject.toml:49) and `unittest.mock.patch`'s use of `functools.wraps` (which propagates `__dict__` including `pytestmark`), the current ordering works at runtime. But: 1. If `asyncio_mode` is ever switched to `"strict"`, whether the mark on the *inner* (unwrapped) function propagates through `@patch` wrappers is an implementation detail of `unittest.mock` — not something to rely on. 2. The whole rationale for adding the marker explicitly (resilience against future config changes) is undermined if the placement itself is fragile. 3. pytest-asyncio documentation consistently shows the asyncio marker as the outermost decorator. ### Fix Move `@pytest.mark.asyncio` to be the topmost decorator on all 14 methods. Mechanical change — just move each line above the `@patch(...)` stack. The core fix (adding the marker at all) is sound. Recommend correcting placement before merging.

cal approved these changes 2026-03-23 13:14:12 +00:00

Dismissed

cal left a comment

AI Code Review

Files Reviewed

tests/test_automated_data_fetcher.py (modified)

Findings

Correctness

All 14 async def test_* methods have been decorated. The count in the PR description is accurate. The change is mechanically correct — every async test method receives the marker.

One nuance worth noting: pyproject.toml already sets asyncio_mode = "auto", which causes pytest-asyncio to treat all async test functions as asyncio tests without needing the explicit decorator. The PR author is aware of this and calls it out clearly. The decorator is redundant at runtime but not harmful, and the stated rationale (explicit intent, resilience to future config changes) is sound.

Decorator Ordering

In Python, decorators are applied from the bottom up (innermost first). The conventional placement for @pytest.mark.asyncio is above @patch(...) decorators so it is the outermost wrapper. In this PR, @pytest.mark.asyncio is placed below @patch in every case — making it the innermost decorator. Example from the diff:

@patch("automated_data_fetcher.pb.batting_stats_bref")
@patch("automated_data_fetcher.pb.pitching_stats_bref")
@pytest.mark.asyncio   # innermost — below the patch decorators
async def test_fetch_baseball_reference_data(...)

With asyncio_mode = "auto", this has no functional impact — pytest-asyncio handles the coroutine regardless of where the marker sits. However, if asyncio_mode were ever removed or changed, this ordering would still work correctly because @pytest.mark.asyncio just needs to be present on the function object, not necessarily outermost. This is not a blocking issue.

Security

No issues. This is a test-only change with no production code impact.

Style & Conventions

The project's CLAUDE.md specifies pytest as the test runner and pytest-asyncio>=0.24.0 is already a dev dependency. The change is consistent with those conventions. No issues.

Suggestions

Consider placing @pytest.mark.asyncio as the topmost decorator (above any @patch decorators) for consistency with the broader pytest-asyncio community convention. This is purely stylistic given asyncio_mode = "auto" is active.
The PR description notes tests/test_helpers.py:4-7 has a related test quality problem (issue #16). That is correctly deferred to a separate issue and does not affect this PR.

Verdict: APPROVED

The change correctly adds @pytest.mark.asyncio to all 14 async test methods, fixing the false-passing silently-skipped test problem described in issue #21. The decorator ordering is non-conventional but has no functional impact given the asyncio_mode = "auto" configuration already in place. No correctness, security, or convention violations that warrant blocking this merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `tests/test_automated_data_fetcher.py` (modified) ### Findings #### Correctness All 14 `async def test_*` methods have been decorated. The count in the PR description is accurate. The change is mechanically correct — every async test method receives the marker. One nuance worth noting: `pyproject.toml` already sets `asyncio_mode = "auto"`, which causes pytest-asyncio to treat all async test functions as asyncio tests without needing the explicit decorator. The PR author is aware of this and calls it out clearly. The decorator is redundant at runtime but not harmful, and the stated rationale (explicit intent, resilience to future config changes) is sound. #### Decorator Ordering In Python, decorators are applied from the bottom up (innermost first). The conventional placement for `@pytest.mark.asyncio` is **above** `@patch(...)` decorators so it is the outermost wrapper. In this PR, `@pytest.mark.asyncio` is placed below `@patch` in every case — making it the innermost decorator. Example from the diff: ```python @patch("automated_data_fetcher.pb.batting_stats_bref") @patch("automated_data_fetcher.pb.pitching_stats_bref") @pytest.mark.asyncio # innermost — below the patch decorators async def test_fetch_baseball_reference_data(...) ``` With `asyncio_mode = "auto"`, this has **no functional impact** — pytest-asyncio handles the coroutine regardless of where the marker sits. However, if `asyncio_mode` were ever removed or changed, this ordering would still work correctly because `@pytest.mark.asyncio` just needs to be present on the function object, not necessarily outermost. This is not a blocking issue. #### Security No issues. This is a test-only change with no production code impact. #### Style & Conventions The project's CLAUDE.md specifies `pytest` as the test runner and `pytest-asyncio>=0.24.0` is already a dev dependency. The change is consistent with those conventions. No issues. #### Suggestions - Consider placing `@pytest.mark.asyncio` as the **topmost** decorator (above any `@patch` decorators) for consistency with the broader pytest-asyncio community convention. This is purely stylistic given `asyncio_mode = "auto"` is active. - The PR description notes `tests/test_helpers.py:4-7` has a related test quality problem (issue #16). That is correctly deferred to a separate issue and does not affect this PR. ### Verdict: APPROVED The change correctly adds `@pytest.mark.asyncio` to all 14 async test methods, fixing the false-passing silently-skipped test problem described in issue #21. The decorator ordering is non-conventional but has no functional impact given the `asyncio_mode = "auto"` configuration already in place. No correctness, security, or convention violations that warrant blocking this merge. --- *Automated review by Claude PR Reviewer*

cal approved these changes 2026-03-23 13:24:30 +00:00

cal left a comment

Approved for merge — correct fix, all async test methods now have @pytest.mark.asyncio.

cal added 1 commit 2026-03-23 13:24:43 +00:00

Merge branch 'main' into ai/paper-dynasty-card-creation#21 424b7da78d

cal merged commit 4392f6c07f into main

2026-03-23 13:25:12 +00:00

cal deleted branch ai/paper-dynasty-card-creation#21

2026-03-23 13:25:13 +00:00

cal referenced this issue from a commit

2026-03-23 13:25:13 +00:00

Merge pull request 'fix: add @pytest.mark.asyncio to async test methods (#21)' (#30) from ai/paper-dynasty-card-creation#21 into main