fix: "The test […] is not an async function" when asyncio marker is#1379
fix: "The test […] is not an async function" when asyncio marker is#1379JiwaniZakir wants to merge 2 commits into
asyncio marker is#1379Conversation
…modifyitems Add a trylast pytest_collection_modifyitems hook that converts Function items with a dynamically added asyncio marker to PytestAsyncioFunction.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1379 +/- ##
==========================================
- Coverage 93.64% 93.26% -0.38%
==========================================
Files 2 2
Lines 409 416 +7
Branches 44 47 +3
==========================================
+ Hits 383 388 +5
- Misses 20 21 +1
- Partials 6 7 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ): | ||
| specialized_item_class = PytestAsyncioFunction.item_subclass_for(item) | ||
| if specialized_item_class: | ||
| items[i] = specialized_item_class._from_function(item) |
There was a problem hiding this comment.
I think this will override items in a way that's breaking and could conflict with, e.g., other pytest plugins.
|
Good point -- I'll rework the implementation to avoid overriding items directly and instead use a more targeted approach that won't conflict with other plugins. |
I appreciate you might be busy with 60-ish PRs you've opened on GitHub since this one, but ideally you'd include a test that fails on main but passes on this merging branch. 😊 |
|
Fair point -- I'll add a regression test that demonstrates the fix before updating the implementation. |
Add test_asyncio_mark_added_via_collection_modifyitems_is_recognized that fails on main (pytest raises 'The test is not an async function') but passes on this branch after the pytest_collection_modifyitems hook is in place. Reproduces the exact pattern from the issue: a user conftest that uses pytest_collection_modifyitems + inspect.iscoroutinefunction to add the asyncio marker to async test functions in strict mode.
|
Added a regression test in 5129c8e that reproduces the exact pattern from issue #810 — a user conftest using I looked again at the |
|
@JiwaniZakir I haven't thought much about the issue you're trying to solve, but it seems non-trivial at first glance. |
golikovichev
left a comment
There was a problem hiding this comment.
Took this for a spin against #810. On main the reporter's reproducer fails as described, and with this branch it passes.
I also tried two cases beyond the bare reproducer, since the thread raised a doubt about whether a late-added marker still interacts correctly with parametrization and fixtures. Same conftest.py adding the marker in pytest_collection_modifyitems, plus:
import pytest
import pytest_asyncio
@pytest_asyncio.fixture
async def async_fix():
return 42
async def test_async_fixture(async_fix): # consumes an async fixture
assert async_fix == 42
@pytest.mark.parametrize("n", [1, 2, 3]) # parametrized
async def test_param(n):
assert n > 0On main all of these fail with the "not an async function" warning and the skip. On this branch all four pass, so the conversion in pytest_collection_modifyitems wires up the async fixture and the parametrized variants rather than only silencing the warning. tests/test_asyncio_mark.py stays green here too.
One suggestion: the regression test covers the no-fixture, no-parametrize case only. Given the concern raised earlier in the thread, it might be worth adding a case where the late-marked async test consumes an async fixture and/or is parametrized, so the wider behavior is locked in against future refactors. Glad to share the snippet I used if useful.
|
The issue I saw before was with |
|
Following up on the parametrization concern. I tried to reproduce the A conftest adds the marker in def pytest_collection_modifyitems(config, items):
for item in items:
func = getattr(item, "function", None)
if func is not None and inspect.iscoroutinefunction(func):
if not any(m.name == "asyncio" for m in item.iter_markers()):
item.add_marker(pytest.mark.asyncio(loop_scope="module"))Two module-level async tests record I couldn't get So at least for |
|
That's because the branch is out of date with main and does not contain the loop factory feature. The marker drives parametrization, so we'll need a solution for late parametrization. |
|
I've spent some time on this problem this weekend and I don't see a way to resolve this issue completely without altering the class-based, collection-time setup and ownership of test items, which would be a small breaking change (a small one though, I think). I'll tidy up the proof of concept and post it as a draft PR within the next few days. |
|
Thanks, you're right - I was on the old branch, so loop_factory was not there to test. Pulled main and I can see it now: the loop_factory parametrization happens via That explains why a late marker can not drive it: Happy to review the draft PoC when you post it. |
Fixes #810
When the
asynciomarker is added to a test item viapytest_collection_modifyitemsin user plugins, pytest-asyncio would raise "The test is not an async function" becausepytest_pycollect_makeitem_convert_async_functions_to_subclasshad already run and did not see the marker at collection time. The root cause was that asyncFunctionitems marked after collection were never converted toPytestAsyncioFunctionsubclasses. Adds apytest_collection_modifyitemshook inpytest_asyncio/plugin.pywithtrylast=Truethat iterates collected items and callsPytestAsyncioFunction.item_subclass_forand_from_functionto perform the conversion for anyFunctioncarrying anasynciomarker that was not already specialized. Verified by the existing test suite covering thepytest_collection_modifyitemsmarker injection scenario introduced alongside this fix.