Merge pull request #23 from kraken-tech/transaction-callback-simulation-setting

Control after-commit calls in tests with a setting

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: