Merge pull request #159 from kraken-tech/meshy/part_of_a_transaction-fail-outside-tests

Ensure `part_of_a_transaction` only runs in tests

Author: meshy

CHANGELOG.md

@@ -15,6 +15,9 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Added
 - Added MariaDB and SQLite to the test matrix.
+- `part_of_a_transaction` now raises an error if it is called outside of tests.
+  This prevents code which misleadingly runs after-commit callbacks.
+  For the same reason, it will also fail when called within transaction test cases.
 - `part_of_a_transaction` now raises an error if unhandled callbacks are detected when it starts.
   This makes it more similar to `transaction`.
   The error can be silenced by setting the `SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS` setting to `False`

src/django_subatomic/test.py

@@ -30,6 +30,17 @@ class _UnhandledCallbacks(Exception):
     callbacks: tuple[Callable[[], object], ...]
 
 
+class _OnlyForUseInDjangoTestTransaction(Exception):
+    """
+    Raised when `part_of_a_transaction` is used without a transaction created by Django tests.
+
+    This can also be raised in tests which handle their own transaction
+    instead of allowing the testsuite to wrap the test in a transaction.
+    (These kinds of tests are called "transaction testcases".)
+    This prevents `part_of_a_transaction` from running after-commit callbacks.
+    """
+
+
 @contextlib.contextmanager
 def part_of_a_transaction(using: str | None = None) -> Generator[None]:
     """
@@ -41,18 +52,25 @@ def part_of_a_transaction(using: str | None = None) -> Generator[None]:
     This works by entering a new "atomic" block, so that the inner-most "atomic"
     isn't the one created by the test-suite.
 
-    In "transaction testcases" this will create a transaction, but if you're writing
-    a transaction testcase, you probably want to manage transactions more explicitly
-    than by calling this.
-
     Note that this does not handle after-commit callback simulation. If you need that,
     use [`transaction`][django_subatomic.db.transaction] instead.
+
+    In production code and "transaction testcases" this will raise an error
+    to ensure we don't misleadingly run after-commit callbacks.
     """
     connection = transaction.get_connection(using)
+
     raise_unhandled_callbacks = getattr(
         settings, "SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS", True
     )
 
+    # We must be called from inside an atomic block created by the test suite
+    # to avoid running after-commit callbacks on exit.
+    # We don't check that the atomic block is from the test suite though,
+    # because if it's created elsewhere we'll see an error from `durable=True` below.
+    if len(connection.atomic_blocks) == 0:
+        raise _OnlyForUseInDjangoTestTransaction
+
     if raise_unhandled_callbacks:
         callbacks = connection.run_on_commit
         if callbacks:

tests/test_test.py

@@ -131,6 +131,17 @@ def test_fails_when_nested_inside_an_atomic_block(
                 with test.part_of_a_transaction():
                     ...
 
+    @pytest.mark.django_db(transaction=True)
+    def test_fails_when_test_suite_not_managing_transactions(self) -> None:
+        """
+        `part_of_a_transaction` cannot be used if the test suite isn't managing transactions.
+        """
+        with (
+            pytest.raises(test._OnlyForUseInDjangoTestTransaction),  # noqa: SLF001
+            test.part_of_a_transaction(),
+        ):
+            ...
+
 
 def _callback_which_should_not_be_called() -> None:
     pytest.fail("Callback should not have been called.")  # pragma: no cover