Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async fixtures request wrong event loop #675

Merged

Conversation

seifertm
Copy link
Contributor

@seifertm seifertm commented Nov 15, 2023

Fixes a bug that caused tests to run in the wrong event loop when requesting larger-scoped fixtures in a narrower-scoped test.

Previously, pytest-asyncio relied on marks applied to pytest Collectors (e.g. classes and modules) to determine the loop scope. This logic is no longer applicable for fixtures, because pytest-asyncio now relies on the asyncio mark applied to tests. As a result, fixtures were looking for an "asyncio" mark on surrounding collectors to no avail and defaulted to choosing a function-scoped loop.

This patch chooses the loop scope based on the fixture scope.

Resolves #658
Resolves #670

This avoids failing tests when running the tests with "python -m pytest" and installing a warning filter on the interpreter level. This can be useful for running tests in development mode, i.e. "python -Xdev -m pytest".

Signed-off-by: Michael Seifert <[email protected]>
…en requesting larger-scoped fixtures in a narrower-scoped test.

Previously, pytest-asyncio relied on marks applied to pytest Collectors (e.g. classes and modules) to determine the loop scope. This logic is no longer applicable for fixtures, because pytest-asyncio now relies on the asyncio mark applied to tests. As a result, fixtures were looking for an "asyncio" mark on surrounding collectors to no avail and defaulted to choosing a function-scoped loop.

This patch chooses the loop scope based on the fixture scope.

Signed-off-by: Michael Seifert <[email protected]>
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ee1589f) 94.72% compared to head (fbc32e0) 94.75%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #675      +/-   ##
==========================================
+ Coverage   94.72%   94.75%   +0.03%     
==========================================
  Files           2        2              
  Lines         455      458       +3     
  Branches       92       93       +1     
==========================================
+ Hits          431      434       +3     
  Misses         15       15              
  Partials        9        9              

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

@seifertm seifertm added this pull request to the merge queue Nov 16, 2023
Merged via the queue into pytest-dev:main with commit 810c9d7 Nov 16, 2023
8 checks passed
@seifertm seifertm deleted the async-fixtures-request-wrong-event-loop branch November 16, 2023 12:42
@Mexflubber
Copy link

Mexflubber commented Nov 18, 2023

After this change I'm getting the new warning you added.

Replacing the event_loop fixture with a custom implementation is deprecated and will lead to errors in the future ...

On my test setup I initialize a database for some end-to-end testing between my api and my db. Some tests send a put request to the API, then the API writes to the DB. And in the following test I send a get request to the API, expecting the data to be there.

To achieve this I'm using pytest_postgresql and I'm using the databaseJanitor to setup a db from a schema in file (won't cover how to do it here). By using scope session on my db-init I guarantee my DB will stay up through the entire test suite.

So my current setup code is as follows

@pytest_asyncio.fixture(scope="session")
def event_loop(request):
    loop = asyncio._get_running_loop() or asyncio.new_event_loop()
    yield loop
    loop.close()

@pytest_asyncio.fixture(scope="session", autouse=True)
async def connection_test(test_db):
    pg_host = test_db.host
    pg_port = test_db.port
    pg_user = test_db.user
    pg_db = test_db.dbname
    pg_password = test_db.password

    with DatabaseJanitor(pg_user, pg_host, pg_port, pg_db, test_db.version, pg_password):
        connection_str = f"postgresql+asyncpg://{pg_user}:@{pg_host}:{pg_port}/{pg_db}"
        database_session.init(connection_str)
        yield
        await database_session.close()

I have no idea how to get rid of the warning while still making my tests work.

Could you please help me?

Thanks,
Aldo

@Mexflubber
Copy link

@seifertm any chance you can take a look at my previous comment?

@seifertm
Copy link
Contributor Author

@Mexflubber I'm not sure which version of pytest-asyncio you're using. The latest stable version is pytest-asyncio v0.21.1, which doesn't have the warning. If you're using the yanked v0.22, you cannot do anything about it. If you're using v0.23.0a1, I would expect that you can simply delete your current event_loop fixture.

Starting from v0.23, each pytest collector (i.e. classes, modules, packages, and the session) have their own event loop. Async tests and fixtures run in the event loop that matches their scope. That means connection_test would run in the event_loop provided by the session, because it's a session-scoped fixture.

Async tests run in the scope provided by the scope keyword argument to @pytest.mark.asyncio(…). If no scope is given for a test, they will run in a function-scoped event loop.

Issue #677 is meant to bring the documentation up to speed before the v0.23 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants