Skip to content

test: upgrade pytest-asyncio and add loop scoping#7147

Draft
glevkovich wants to merge 1 commit intomainfrom
glevkovich/upgrade_pytest_asyncio_and_add_loop_scoping
Draft

test: upgrade pytest-asyncio and add loop scoping#7147
glevkovich wants to merge 1 commit intomainfrom
glevkovich/upgrade_pytest_asyncio_and_add_loop_scoping

Conversation

@glevkovich
Copy link
Copy Markdown
Contributor

@glevkovich glevkovich commented Apr 15, 2026

Upgrade pytest-asyncio from 0.20.1 to 0.24.0+ and configure
event loop scope to prevent fixture crashes.

requirements.txt changes:

  • pytest>=8.3,<9.0 (was >=7.1.2)
  • pytest-asyncio>=0.24.0,<0.25.0 (was ==0.20.1)

conftest.py changes:

  • Remove manual event_loop fixture override (deprecated in 0.24+)
  • Rewrite async_client fixture to bypass async_pool. With
    session-scoped loops, the standalone ConnectionPool in async_pool
    retains connections with stale loop references when df_server
    restarts between test classes. Now async_client creates Redis
    directly with connection params, letting Redis manage its own
    internal pool and adding proper aclose() cleanup. This fixes 7
    failing tests (test_publish_stuck, test_nested_client_pause,
    test_cron_snapshot, test_set_cron_snapshot, test_basic_memory_usage,
    test_mixed_append, test_tiered_replication_with_hashes).

pytest.ini changes:

  • Add asyncio_default_fixture_loop_scope=session

pytest-asyncio 0.24+ forbids manual event_loop overrides and
requires explicit loop scope configuration. We use session scope
because:

  1. df_factory and other fixtures are class-scoped
  2. Many async tests are module-level functions (not in classes)
  3. session scope supports both patterns without UsageError

Upper bounds prevent pip from installing pytest 9.x and
pytest-asyncio 1.x, which conflict with dragonfly-fakeredis-tests.

snapshot_test.py changes:

  • test_s3_snapshot, test_s3_save_local_dir: replace async_client
    fixture with df_server.client(). With session-scoped loops,
    the shared async_client pool holds stale connections from a
    previous df_server instance, causing "Future attached to a
    different loop". Creating a fresh client per test avoids this
    (same pattern used by test_s3_reload_snapshot_after_restart).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Upgrades the Python test harness dependencies to support newer pytest / pytest-asyncio behavior and adds explicit asyncio event loop scoping to avoid fixture scope conflicts in async tests.

Changes:

  • Bump pytest to >=8.3,<9.0 and pytest-asyncio to >=0.24.0,<0.25.0 for the tests/dragonfly suite.
  • Configure pytest-asyncio to use function-scoped default fixture event loops via asyncio_default_fixture_loop_scope=function.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/pytest.ini Adds explicit pytest-asyncio default fixture loop scope configuration.
tests/dragonfly/requirements.txt Updates pytest/pytest-asyncio version constraints to newer supported ranges.

Comment thread tests/pytest.ini Outdated
@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented Apr 15, 2026

🤖 Augment PR Summary

Summary: Updates the Python integration test dependencies to use newer pytest/pytest-asyncio versions and adds explicit pytest-asyncio loop scoping.

Why: Aligns with pytest-asyncio 0.24+ strict loop-scope behavior to avoid fixture scope/loop-scope conflicts and resulting test failures.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread tests/pytest.ini Outdated
Comment thread tests/dragonfly/requirements.txt
vyavdoshenko
vyavdoshenko previously approved these changes Apr 15, 2026
Copy link
Copy Markdown
Contributor

@vyavdoshenko vyavdoshenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread tests/pytest.ini
@glevkovich glevkovich enabled auto-merge (squash) April 15, 2026 12:22
@glevkovich glevkovich force-pushed the glevkovich/upgrade_pytest_asyncio_and_add_loop_scoping branch from a622675 to 12ecdb3 Compare April 15, 2026 12:48
Comment thread tests/pytest.ini
log_file_level=INFO
log_cli = true
asyncio_mode=auto
asyncio_default_fixture_loop_scope=session
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is a mistake.
It should be:

asyncio_default_fixture_loop_scope=class

or even

asyncio_default_fixture_loop_scope=function

Copy link
Copy Markdown
Contributor Author

@glevkovich glevkovich Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a mistake - tried both and they crash:

  • function: crash with "Future attached to a different loop" because df_factory in tests/dragonfly/conftest.py is class-scoped and outlives the per-function loop.

  • class: crash with with "UsageError: test_client_migrate is marked to be run in an event loop with scope class, but is not part of any class" - many tests are standalone functions, not inside a class.

session is the only scope that works for me, with both class-scoped fixtures and module-level test functions under pytest-asyncio 0.24's scoping rules.

Upgrade pytest-asyncio from 0.20.1 to 0.24.0+ and configure
event loop scope to prevent fixture crashes.

requirements.txt changes:
- pytest>=8.3,<9.0 (was >=7.1.2)
- pytest-asyncio>=0.24.0,<0.25.0 (was ==0.20.1)

conftest.py changes:
- Remove manual event_loop fixture override (deprecated in 0.24+)
- Rewrite async_client fixture to bypass async_pool. With
  session-scoped loops, the standalone ConnectionPool in async_pool
  retains connections with stale loop references when df_server
  restarts between test classes. Now async_client creates Redis
  directly with connection params, letting Redis manage its own
  internal pool and adding proper aclose() cleanup. This fixes 7
  failing tests (test_publish_stuck, test_nested_client_pause,
  test_cron_snapshot, test_set_cron_snapshot, test_basic_memory_usage,
  test_mixed_append, test_tiered_replication_with_hashes).

pytest.ini changes:
- Add asyncio_default_fixture_loop_scope=session

pytest-asyncio 0.24+ forbids manual event_loop overrides and
requires explicit loop scope configuration. We use session scope
because:
1. df_factory and other fixtures are class-scoped
2. Many async tests are module-level functions (not in classes)
3. session scope supports both patterns without UsageError

Upper bounds prevent pip from installing pytest 9.x and
pytest-asyncio 1.x, which conflict with dragonfly-fakeredis-tests.

snapshot_test.py changes:
- test_s3_snapshot, test_s3_save_local_dir: replace async_client
  fixture with df_server.client(). With session-scoped loops,
  the shared async_client pool holds stale connections from a
  previous df_server instance, causing "Future attached to a
  different loop". Creating a fresh client per test avoids this
  (same pattern used by test_s3_reload_snapshot_after_restart).

Signed-off-by: Gil Levkovich <69595609+glevkovich@users.noreply.github.com>
@glevkovich glevkovich force-pushed the glevkovich/upgrade_pytest_asyncio_and_add_loop_scoping branch from 12ecdb3 to 64084ac Compare April 15, 2026 13:47
@glevkovich
Copy link
Copy Markdown
Contributor Author

Adding more context on the conftest.py changes :
Even with session scope, the old async_pool fixture was caching stale references to the loop across server teardowns, causing "Event loop is closed" and "Future attached to a different loop" errors during garbage collection.

So i decided to bypass async_pool and instantiating aioredis.Redis directly inside the async_client fixture, the client dynamically binds to the active loop every time and safely cleans up its sockets before Pytest tears the loop down. I hope that this combination finally makes the v0.24 upgrade stable - lets see if tests pass.

@glevkovich
Copy link
Copy Markdown
Contributor Author

@vyavdoshenko please hold on with the review. This PR will take me more work than I thought to finalise (still lots of tests are failing so what I have no is probably not the final product). I need to fix more of tests/infra to make it work. Holding it for now...

@glevkovich glevkovich marked this pull request as draft April 15, 2026 15:29
auto-merge was automatically disabled April 15, 2026 15:29

Pull request was converted to draft

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.

3 participants