Control after-commit calls in tests with a setting

By default, in (non transaction testcase) tests, we simulate the
execution of after-commit callbacks, to reproduce Django's normal
behaviour when outside of tests.

To allow projects to migrate their existing code over to this new
paradigm gradually, we allow this behaviour to be disabled. This was
previously controlled by monkey-patching a module scoped value in tests,
but is now exposed with a Django setting:
`SUBATOMIC_RUN_AFTER_COMMIT_CALLBACKS_IN_TESTS`.

Co-authored-by: Samuel Searles-Bryant <sam.searles-bryant@kraken.tech>

Author: meshy

pytest.ini

@@ -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

src/django_subatomic/db.py

@@ -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.
@@ -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)
@@ -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(
@@ -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)

tests/test_db.py

@@ -2,7 +2,6 @@
 
 import re
 from typing import TYPE_CHECKING
-from unittest import mock
 
 import pytest
 from django import db as django_db
@@ -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
@@ -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
@@ -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",
         (
@@ -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: