Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ addopts =
# https://pytest-django.readthedocs.io/
-p django

markers =
# See Note [Running after-commit callbacks in tests]
deprecated_ignore_after_commit_callbacks: require after-commit callbacks to be explicitly captured and called

# Ensure passing xfailed (aka "xpass") tests cause the test suite to fail.
# https://docs.pytest.org/en/latest/skipping.html#strict-parameter
xfail_strict = true
17 changes: 10 additions & 7 deletions src/django_subatomic/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@
from typing import NoReturn


# See Note [Running after-commit callbacks in tests]
_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS = True


def dbs_with_open_transactions() -> frozenset[str]:
"""
Get the names of databases with open transactions.
Expand Down Expand Up @@ -224,7 +220,8 @@ def _execute_on_commit_callbacks_in_tests(using: str | None = None) -> Iterator[
"""
yield
if (
_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS # See Note [Running after-commit callbacks in tests]
# See Note [Running after-commit callbacks in tests]
getattr(settings, "SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS", True)
and _innermost_atomic_block_wraps_testcase(using=using)
):
connection = django_transaction.get_connection(using)
Expand Down Expand Up @@ -335,6 +332,9 @@ class _UnexpectedDanglingTransaction(Exception):
# To avoid that problem, we capture after-commit callbacks and execute them when we exit the
# outermost `transaction` or `transaction_if_not_already` context. This emulates how the application
# will behave when deployed and means that our tests are testing realistic application behaviour.
#
# To help projects migrate to this behaviour, it can be disabled with
# the Django setting `SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS`.


def run_after_commit(
Expand Down Expand Up @@ -368,8 +368,11 @@ def run_after_commit(
if needs_transaction and not in_transaction(using=using):
raise _MissingRequiredTransaction(database=using)

# See Note [Running after-commit callbacks in tests]
if _RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS and only_in_testcase_transaction:
if (
# See Note [Running after-commit callbacks in tests]
getattr(settings, "SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS", True)
and only_in_testcase_transaction
):
callback()
else:
django_transaction.on_commit(callback, using=using)
Expand Down
45 changes: 11 additions & 34 deletions tests/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import re
from typing import TYPE_CHECKING
from unittest import mock

import pytest
from django import db as django_db
Expand All @@ -14,7 +13,7 @@

if TYPE_CHECKING:
import contextlib
from collections.abc import Callable, Generator
from collections.abc import Callable
from typing import Protocol

import pytest_django
Expand Down Expand Up @@ -60,28 +59,6 @@ def _parametrize_transaction_testcase(func: Callable[..., None]) -> MarkDecorato
return parametrize(usefixtures(func)) # type: ignore[no-any-return]


@pytest.fixture(autouse=True)
def _prevent_after_commit_callbacks(
request: pytest.FixtureRequest,
) -> Generator[None, None, None]:
"""
Disable automatically running after-commit callbacks in marked tests.

If a test relies on after-commit callbacks never being called, it may be
marked with `deprecated_ignore_after_commit_callbacks` to disable test simulation
of how after-commit callbacks are naturally run after transactions.

TODO: Allow users of this library to do this in their own tests.

See Note [Running after-commit callbacks in tests]
"""
if "deprecated_ignore_after_commit_callbacks" in request.keywords:
with mock.patch.object(db, "_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS", new=False):
yield
else:
yield


class Counter:
def __init__(self) -> None:
self.count = 0
Expand Down Expand Up @@ -463,25 +440,24 @@ def test_not_executed_if_rolled_back(self) -> None:
assert counter.count == 0


class TestRunAfterCommitDeprecatedTestBehaviour:
@pytest.mark.deprecated_ignore_after_commit_callbacks
class TestRunAfterCommitCallbacksSettingBehaviour:
@override_settings(SUBATOMIC_AFTER_COMMIT_NEEDS_TRANSACTION=False)
def test_does_not_execute_when_in_testcase_transaction_if_callbacks_disabled(
self,
) -> None:
"""
`run_after_commit` should not ignore testcase transactions when test marked with `deprecated_ignore_after_commit_callbacks`.
`run_after_commit` should not ignore testcase transactions when SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS is False.

We have to disable `SUBATOMIC_AFTER_COMMIT_NEEDS_TRANSACTION`, because
otherwise an error would be raised when trying to register the callback.
"""
counter = Counter()

db.run_after_commit(counter.increment)
with override_settings(SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS=False):
db.run_after_commit(counter.increment)

assert counter.count == 0

@pytest.mark.deprecated_ignore_after_commit_callbacks
@pytest.mark.parametrize(
"transaction_context",
(
Expand All @@ -493,16 +469,17 @@ def test_does_not_execute_after_commit_if_decorated(
self, transaction_context: DBContextManager
) -> None:
"""
`run_after_commit` should not execute the callback when test marked with `deprecated_ignore_after_commit_callbacks`.
`run_after_commit` should not execute the callback when SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS is False.
"""
counter = Counter()

with transaction_context():
db.run_after_commit(counter.increment)
with override_settings(SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS=False):
with transaction_context():
db.run_after_commit(counter.increment)

assert counter.count == 0
assert counter.count == 0

assert counter.count == 0
assert counter.count == 0


class TestInTransaction:
Expand Down