Bug fix: Issue #708

[franco-grobler]

During teardown, the final test fails due to the event loop already being closed.

The PR addresses that by checking if the look is open in the teardown function.

[codecov-commenter]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (b614c77) 94.89% compared to head (4637c07) 94.29%.

Files	Patch %	Lines
pytest_asyncio/plugin.py	0.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #709      +/-   ##
==========================================
- Coverage   94.89%   94.29%   -0.61%     
==========================================
  Files           2        2              
  Lines         470      473       +3     
  Branches       93       94       +1     
==========================================
  Hits          446      446              
- Misses         16       18       +2     
- Partials        8        9       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[seifertm]

Thanks for the initiative!
I believe this would fix the issue for many users, but there are some edge cases that are not addressed by the patch.

The underlying problem is not just that the fixture finalizer doesn't have a working event loop. It's that anything after he yield of an async generator fixture has no working event loop when using a (non-function-scoped) fixture with a function-scoped test.

The following example would still fails with the proposed patch, because await task raises an "attached to different loop" RuntimeError:

import pytest
import pytest_asyncio

@pytest_asyncio.fixture(scope="module")
async def async_generator_fixture():
    task = asyncio.create_task(asyncio.sleep(0))
    yield
    await task


@pytest.mark.asyncio
async def test_asdf(async_generator_fixture):
    pass

The problematic piece of code lies in the setup of the event_loop fixture:

pytest-asyncio/pytest_asyncio/plugin.py

Lines 749 to 753 in b614c77

	with warnings.catch_warnings():
	warnings.simplefilter("ignore", DeprecationWarning)
	old_loop = policy.get_event_loop()
	if old_loop is not loop:
	old_loop.close()

We cannot just remove old_loop.close(), because that would break backwards compatibility. The v0.23 release is intended to introduce a bunch of deprecations to give users the chance to update their test suites accordingly.

I was thinking that we could attach a special attribute to the event loops provided by pytest-asyncio and use that attribute to determine if we should close the event loop or not. We can be sure that the loops provided by pytest-asyncio are cleaned up properly, so we skip the closing logic in this case, in order to prevent cleaning up the loop prematurely. A similar approach exists in the event_loop fixture implementation of pytest-asyncio:

pytest-asyncio/pytest_asyncio/plugin.py

Lines 951 to 956 in b614c77

	# Add a magic value to the event loop, so pytest-asyncio can determine if the
	# event_loop fixture was overridden. Other implementations of event_loop don't
	# set this value.
	# The magic value must be set as part of the function definition, because pytest
	# seems to have multiple instances of the same FixtureDef or fixture function
	loop.__original_fixture_loop = True # type: ignore[attr-defined]

This is pretty hacky, but it's only a temporary solution for the v0.23 release.

And of course the PR would need to a test that reproduces the error along with a fix :)

[seifertm]

Closing this in favor of #714

I hope this doesn't discourage you from future contributions.