-
Notifications
You must be signed in to change notification settings - Fork 347
Add support for db fixture (test inside transaction) for asyncio tests #1223
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
Open
lode-braced
wants to merge
23
commits into
pytest-dev:main
Choose a base branch
from
lode-braced:fix/async_fixture
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
58f7f24
Add async support: run transaction start/stop in an async context whe…
3ee56c4
Revertme: test pipeline
3515460
Move paramspec into type check block for python 3.9
3f1fe98
Revertme: remove cov step
1a09850
Fix type hints
629cafd
Add docs for async, thread checks, refer to docs in plugin errors
4a79664
Revert "Revertme: test pipeline"
f46bac0
Revert "Revertme: remove cov step"
7673814
Drop sync extra checks
061ca4b
Fix docs layout
cca2dff
Remove unblock kwarg from sync
1bf9be6
Reduce code duplication: reuse test case class creation logic for syn…
0525d62
Revert "Drop sync extra checks"
2c1d21b
Revert "Remove unblock kwarg from sync"
95b03a0
Wrap patched method with one containing explicit wrapped self to fix …
b219fcb
Chore: reformat
593d02f
Chore: update type hints
3c986bf
Add test case for db access outside main thread in sync context
0fb5f38
Add unblock test case when both flags are set
4b1274c
chore: format
215c679
Merge remote-tracking branch 'refs/remotes/origin/main' into fix/asyn…
cff3a16
Merge remote-tracking branch 'origin/main' into fix/async_fixture
9aff5fd
Chore: fix tox indenting
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,7 +3,7 @@ | |||||
from __future__ import annotations | ||||||
|
||||||
import os | ||||||
from collections.abc import Generator, Iterable, Sequence | ||||||
from collections.abc import AsyncGenerator, Generator, Iterable, Sequence | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please move
Suggested change
|
||||||
from contextlib import AbstractContextManager, contextmanager | ||||||
from functools import partial | ||||||
from typing import TYPE_CHECKING, Any, Callable, Literal, Optional, Protocol, Union | ||||||
|
@@ -201,8 +201,49 @@ def django_db_setup( | |||||
) | ||||||
|
||||||
|
||||||
def _build_pytest_django_test_case( | ||||||
test_case_class, | ||||||
*, | ||||||
reset_sequences: bool, | ||||||
serialized_rollback: bool, | ||||||
databases, | ||||||
available_apps, | ||||||
skip_django_testcase_class_setup: bool, | ||||||
): | ||||||
# Build a custom TestCase subclass with configured attributes and optional | ||||||
# overrides to skip Django's TestCase class-level setup/teardown. | ||||||
import django.test # local import to avoid hard dependency at import time | ||||||
|
||||||
_reset_sequences = reset_sequences | ||||||
_serialized_rollback = serialized_rollback | ||||||
_databases = databases | ||||||
_available_apps = available_apps | ||||||
|
||||||
class PytestDjangoTestCase(test_case_class): | ||||||
reset_sequences = _reset_sequences | ||||||
serialized_rollback = _serialized_rollback | ||||||
if _databases is not None: | ||||||
databases = _databases | ||||||
if _available_apps is not None: | ||||||
available_apps = _available_apps | ||||||
|
||||||
if skip_django_testcase_class_setup: | ||||||
|
||||||
@classmethod | ||||||
def setUpClass(cls) -> None: | ||||||
# Skip django.test.TestCase.setUpClass, call its super instead | ||||||
super(django.test.TestCase, cls).setUpClass() | ||||||
|
||||||
@classmethod | ||||||
def tearDownClass(cls) -> None: | ||||||
# Skip django.test.TestCase.tearDownClass, call its super instead | ||||||
super(django.test.TestCase, cls).tearDownClass() | ||||||
|
||||||
return PytestDjangoTestCase | ||||||
|
||||||
|
||||||
@pytest.fixture | ||||||
def _django_db_helper( | ||||||
def _sync_django_db_helper( | ||||||
request: pytest.FixtureRequest, | ||||||
django_db_setup: None, | ||||||
django_db_blocker: DjangoDbBlocker, | ||||||
|
@@ -239,7 +280,7 @@ def _django_db_helper( | |||||
"django_db_serialized_rollback" in request.fixturenames | ||||||
) | ||||||
|
||||||
with django_db_blocker.unblock(): | ||||||
with django_db_blocker.unblock(sync_only=not transactional): | ||||||
import django.db | ||||||
import django.test | ||||||
|
||||||
|
@@ -248,41 +289,14 @@ def _django_db_helper( | |||||
else: | ||||||
test_case_class = django.test.TestCase | ||||||
|
||||||
_reset_sequences = reset_sequences | ||||||
_serialized_rollback = serialized_rollback | ||||||
_databases = databases | ||||||
_available_apps = available_apps | ||||||
|
||||||
class PytestDjangoTestCase(test_case_class): # type: ignore[misc,valid-type] | ||||||
reset_sequences = _reset_sequences | ||||||
serialized_rollback = _serialized_rollback | ||||||
if _databases is not None: | ||||||
databases = _databases | ||||||
if _available_apps is not None: | ||||||
available_apps = _available_apps | ||||||
|
||||||
# For non-transactional tests, skip executing `django.test.TestCase`'s | ||||||
# `setUpClass`/`tearDownClass`, only execute the super class ones. | ||||||
# | ||||||
# `TestCase`'s class setup manages the `setUpTestData`/class-level | ||||||
# transaction functionality. We don't use it; instead we (will) offer | ||||||
# our own alternatives. So it only adds overhead, and does some things | ||||||
# which conflict with our (planned) functionality, particularly, it | ||||||
# closes all database connections in `tearDownClass` which inhibits | ||||||
# wrapping tests in higher-scoped transactions. | ||||||
# | ||||||
# It's possible a new version of Django will add some unrelated | ||||||
# functionality to these methods, in which case skipping them completely | ||||||
# would not be desirable. Let's cross that bridge when we get there... | ||||||
if not transactional: | ||||||
|
||||||
@classmethod | ||||||
def setUpClass(cls) -> None: | ||||||
super(django.test.TestCase, cls).setUpClass() | ||||||
|
||||||
@classmethod | ||||||
def tearDownClass(cls) -> None: | ||||||
super(django.test.TestCase, cls).tearDownClass() | ||||||
PytestDjangoTestCase = _build_pytest_django_test_case( | ||||||
test_case_class, | ||||||
reset_sequences=reset_sequences, | ||||||
serialized_rollback=serialized_rollback, | ||||||
databases=databases, | ||||||
available_apps=available_apps, | ||||||
skip_django_testcase_class_setup=(not transactional), | ||||||
) | ||||||
|
||||||
PytestDjangoTestCase.setUpClass() | ||||||
|
||||||
|
@@ -298,6 +312,112 @@ def tearDownClass(cls) -> None: | |||||
PytestDjangoTestCase.doClassCleanups() | ||||||
|
||||||
|
||||||
try: | ||||||
import pytest_asyncio | ||||||
except ImportError: | ||||||
|
||||||
async def _async_django_db_helper( | ||||||
request: pytest.FixtureRequest, | ||||||
django_db_blocker: DjangoDbBlocker, | ||||||
) -> AsyncGenerator[None, None]: | ||||||
raise RuntimeError( | ||||||
"The `pytest_asyncio` plugin is required to use the `async_django_db` fixture." | ||||||
) | ||||||
yield # pragma: no cover | ||||||
else: | ||||||
|
||||||
@pytest_asyncio.fixture | ||||||
async def _async_django_db_helper( | ||||||
request: pytest.FixtureRequest, | ||||||
django_db_blocker: DjangoDbBlocker, | ||||||
) -> AsyncGenerator[None, None]: | ||||||
# same as _sync_django_db_helper, except for running the transaction start and rollback wrapped in a | ||||||
# `sync_to_async` call | ||||||
transactional, reset_sequences, databases, serialized_rollback, available_apps = ( | ||||||
_get_django_db_settings(request) | ||||||
) | ||||||
|
||||||
with django_db_blocker.unblock(async_only=True): | ||||||
import django.db | ||||||
import django.test | ||||||
|
||||||
test_case_class = django.test.TestCase | ||||||
|
||||||
PytestDjangoTestCase = _build_pytest_django_test_case( | ||||||
test_case_class, | ||||||
reset_sequences=reset_sequences, | ||||||
serialized_rollback=serialized_rollback, | ||||||
databases=databases, | ||||||
available_apps=available_apps, | ||||||
skip_django_testcase_class_setup=True, | ||||||
) | ||||||
|
||||||
from asgiref.sync import sync_to_async | ||||||
|
||||||
await sync_to_async(PytestDjangoTestCase.setUpClass)() | ||||||
|
||||||
test_case = PytestDjangoTestCase(methodName="__init__") | ||||||
await sync_to_async(test_case._pre_setup, thread_sensitive=True)() | ||||||
|
||||||
yield | ||||||
|
||||||
await sync_to_async(test_case._post_teardown, thread_sensitive=True)() | ||||||
|
||||||
await sync_to_async(PytestDjangoTestCase.tearDownClass)() | ||||||
|
||||||
await sync_to_async(PytestDjangoTestCase.doClassCleanups)() | ||||||
|
||||||
|
||||||
def _get_django_db_settings(request: pytest.FixtureRequest) -> _DjangoDb: | ||||||
django_marker = request.node.get_closest_marker("django_db") | ||||||
if django_marker: | ||||||
( | ||||||
transactional, | ||||||
reset_sequences, | ||||||
databases, | ||||||
serialized_rollback, | ||||||
available_apps, | ||||||
) = validate_django_db(django_marker) | ||||||
else: | ||||||
( | ||||||
transactional, | ||||||
reset_sequences, | ||||||
databases, | ||||||
serialized_rollback, | ||||||
available_apps, | ||||||
) = False, False, None, False, None | ||||||
|
||||||
transactional = ( | ||||||
transactional | ||||||
or reset_sequences | ||||||
or ("transactional_db" in request.fixturenames or "live_server" in request.fixturenames) | ||||||
) | ||||||
|
||||||
reset_sequences = reset_sequences or ("django_db_reset_sequences" in request.fixturenames) | ||||||
serialized_rollback = serialized_rollback or ( | ||||||
"django_db_serialized_rollback" in request.fixturenames | ||||||
) | ||||||
return transactional, reset_sequences, databases, serialized_rollback, available_apps | ||||||
|
||||||
|
||||||
@pytest.fixture | ||||||
def _django_db_helper( | ||||||
request: pytest.FixtureRequest, | ||||||
django_db_setup: None, | ||||||
django_db_blocker: DjangoDbBlocker, | ||||||
): | ||||||
asyncio_marker = request.node.get_closest_marker("asyncio") | ||||||
transactional, *_ = _get_django_db_settings(request) | ||||||
if transactional or not asyncio_marker: | ||||||
# add the original sync fixture | ||||||
request.getfixturevalue("_sync_django_db_helper") | ||||||
else: | ||||||
# add the async fixture. Will run it inside the event loop, which will cause the sync to async calls to | ||||||
# start a transaction on the thread safe executor for that loop. This allows us to roll back orm calls made | ||||||
# in that async test context. | ||||||
request.getfixturevalue("_async_django_db_helper") | ||||||
|
||||||
|
||||||
def _django_db_signature( | ||||||
transaction: bool = False, | ||||||
reset_sequences: bool = False, | ||||||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a django requirement, not ours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote this before i saw, you use it in
fixtures.py
now im in two minds... I don't have a strong opinion. I'll leave this up for discussion.