diff --git a/CHANGELOG.md b/CHANGELOG.md index 9da8280..f416dfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Added - Search plugin for MkDocs +- Raise an error when entering `db.transaction()` (and `transaction_if_not_already()` when it opens) **if** the testcase transaction already has pending `on_commit` callbacks. + Opt-out via `SUBATOMIC_RAISE_IF_PENDING_TESTCASE_ON_COMMIT_ON_ENTER` (default: `True`). (#31) ### Fixed diff --git a/docs/reference/settings.md b/docs/reference/settings.md index 29b4d23..a5eb858 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -32,3 +32,13 @@ on a per-test basis. [override_settings]: https://docs.djangoproject.com/en/stable/topics/testing/tools/#django.test.override_settings [django-settings]: https://docs.djangoproject.com/en/stable/topics/settings/ + + +## `SUBATOMIC_RAISE_IF_PENDING_TESTCASE_ON_COMMIT_ON_ENTER` + +**Default:** `True` + +When `True`, entering `db.transaction()` (and `transaction_if_not_already()` when it opens) **raises** if the test suite’s transaction already has pending `on_commit` callbacks. +This avoids accidentally flushing callbacks that were queued **before** opening an application transaction in tests. + +Set to `False` to keep the previous behaviour during migration. diff --git a/src/django_subatomic/db.py b/src/django_subatomic/db.py index 25a3a77..42e6a98 100644 --- a/src/django_subatomic/db.py +++ b/src/django_subatomic/db.py @@ -27,6 +27,39 @@ "transaction_required", ] +@attrs.frozen +class _PendingTestcaseAfterCommitCallbacks(Exception): + pending: int + database: str | None = None + + def __str__(self) -> str: + return ( + f"{self.pending} pending on_commit() callback(s) already queued on the " + f"test-case transaction (db={self.database!r}). " + "Register on_commit() inside a real transaction or set " + "SUBATOMIC_RAISE_IF_PENDING_TESTCASE_ON_COMMIT_ON_ENTER=False to opt out." + ) + + +@contextlib.contextmanager +def _error_if_pending_testcase_on_commit_callbacks(*, using: str | None = None): + if not getattr( + settings, + "SUBATOMIC_RAISE_IF_PENDING_TESTCASE_ON_COMMIT_ON_ENTER", + True, # strict par défaut + ): + yield + return + + if _innermost_atomic_block_wraps_testcase(using=using): + connection = django_transaction.get_connection(using=using) + pending = getattr(connection, "run_on_commit", None) + if pending: # non vide => des callbacks ont été enregistrés avant + raise _PendingTestcaseAfterCommitCallbacks( + pending=len(pending), + database=getattr(connection, "alias", None), + ) + yield @contextlib.contextmanager def transaction(*, using: str | None = None) -> Generator[None]: @@ -43,11 +76,11 @@ def transaction(*, using: str | None = None) -> Generator[None]: """ # Note that `savepoint=False` is not required here because # the `savepoint` flag is ignored when `durable` is `True`. - with ( - _execute_on_commit_callbacks_in_tests(using), - django_transaction.atomic(using=using, durable=True), - ): - yield + with _error_if_pending_testcase_on_commit_callbacks(using=using): + with _execute_on_commit_callbacks_in_tests(using), django_transaction.atomic( + using=using, durable=True + ): + yield @contextlib.contextmanager @@ -81,13 +114,17 @@ def transaction_if_not_already(*, using: str | None = None) -> Generator[None]: # If the innermost atomic block is from a test case, we should create a SAVEPOINT here. # This allows for a rollback when an exception propagates out of this block, and so # better simulates a production transaction behaviour in tests. - savepoint = _innermost_atomic_block_wraps_testcase(using=using) - - with ( - _execute_on_commit_callbacks_in_tests(using), - django_transaction.atomic(using=using, savepoint=savepoint), - ): + if in_transaction(using=using): + # Already in a transaction; don't open another. yield + else: + # If the innermost atomic block comes from the testcase, create a SAVEPOINT. + savepoint = _innermost_atomic_block_wraps_testcase(using=using) + with _error_if_pending_testcase_on_commit_callbacks(using=using): + with _execute_on_commit_callbacks_in_tests(using), django_transaction.atomic( + using=using, savepoint=savepoint + ): + yield @_utils.contextmanager diff --git a/tests/test_on_commit_guard.py b/tests/test_on_commit_guard.py new file mode 100644 index 0000000..af34d6c --- /dev/null +++ b/tests/test_on_commit_guard.py @@ -0,0 +1,39 @@ +# tests/test_on_commit_guard.py + +from django.test import TestCase, override_settings +from django.db import transaction as dj_tx + +from django_subatomic import db +from django_subatomic.db import _PendingTestcaseAfterCommitCallbacks + + +class TestOnCommitGuardWithTransaction(TestCase): + def test_raises_on_enter_if_testcase_has_pending_callbacks(self): + dj_tx.on_commit(lambda: None) + with self.assertRaises(_PendingTestcaseAfterCommitCallbacks): + with db.transaction(): + pass + + @override_settings(SUBATOMIC_RAISE_IF_PENDING_TESTCASE_ON_COMMIT_ON_ENTER=False) + def test_opt_out_keeps_previous_behaviour(self): + calls = [] + dj_tx.on_commit(lambda: calls.append("ok")) + with db.transaction(): + self.assertEqual(calls, []) + self.assertEqual(calls, ["ok"]) + + +class TestOnCommitGuardWithTransactionIfNotAlready(TestCase): + def test_raises_on_enter_in_transaction_if_not_already(self): + dj_tx.on_commit(lambda: None) + with self.assertRaises(_PendingTestcaseAfterCommitCallbacks): + with db.transaction_if_not_already(): + pass + + @override_settings(SUBATOMIC_RAISE_IF_PENDING_TESTCASE_ON_COMMIT_ON_ENTER=False) + def test_opt_out_keeps_previous_behaviour_in_transaction_if_not_already(self): + calls = [] + dj_tx.on_commit(lambda: calls.append("ok")) + with db.transaction_if_not_already(): + self.assertEqual(calls, []) + self.assertEqual(calls, ["ok"])