diff --git a/CHANGELOG.md b/CHANGELOG.md index 6944d88..367d549 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,9 +7,17 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added - Added MariaDB and SQLite to the test matrix. +- `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` + +### Fixed - Disallowed nesting of `part_of_a_transaction` to prevent nonsense implication of nested partial transactions in tests. Fixes #150. +- `part_of_a_transaction` now clears after-commit callbacks from the transaction before it exits. + This avoids polluting the test's transaction with callbacks. ## [1.0.0] - 2026-04-16 diff --git a/docs/reference/settings.md b/docs/reference/settings.md index 1ba5241..9240880 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -35,11 +35,14 @@ on a per-test basis. (default: `True`) [`transaction`][django_subatomic.db.transaction] -will raise `_UnhandledCallbacks` in tests -if it detects any lingering unhandled after-commit callbacks -when it's called. -Note: because this exception represents a programming error, -it starts with an underscore to discourage anyone from catching it. +and [`transaction_if_not_already`][django_subatomic.db.transaction_if_not_already] +will raise `subatomic.db._UnhandledCallbacks` in tests +if they detect any lingering unhandled after-commit callbacks +when they are called. +[`part_of_a_transaction`][django_subatomic.test.part_of_a_transaction] +will raise `subatomic.test._UnhandledCallbacks` instead. +Note: because these exceptions each represent a programming error, +they start with an underscore to discourage anyone from catching them. This highlights order-of-execution issues in tests caused by after-commit callbacks having not been run. diff --git a/src/django_subatomic/test.py b/src/django_subatomic/test.py index 3fde7b5..ed75a06 100644 --- a/src/django_subatomic/test.py +++ b/src/django_subatomic/test.py @@ -3,17 +3,33 @@ import contextlib from typing import TYPE_CHECKING +import attrs +from django.conf import settings from django.db import transaction if TYPE_CHECKING: - from collections.abc import Generator + from collections.abc import Callable, Generator __all__ = [ "part_of_a_transaction", ] +@attrs.frozen +class _UnhandledCallbacks(Exception): + """ + Raised in tests when unhandled callbacks are found before calling `part_of_a_transaction`. + + This happens when after-commit callbacks are registered + but not run before trying to open a database transaction. + + The best solution is to ensure the after-commit callbacks are handled first. + """ + + callbacks: tuple[Callable[[], object], ...] + + @contextlib.contextmanager def part_of_a_transaction(using: str | None = None) -> Generator[None]: """ @@ -32,5 +48,19 @@ def part_of_a_transaction(using: str | None = None) -> Generator[None]: Note that this does not handle after-commit callback simulation. If you need that, use [`transaction`][django_subatomic.db.transaction] instead. """ + connection = transaction.get_connection(using) + if getattr( + settings, "SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS", True + ): + callbacks = connection.run_on_commit + if callbacks: + raise _UnhandledCallbacks(tuple(callback for _, callback, _ in callbacks)) + with transaction.atomic(using=using, durable=True): yield + + # Throw away any callbacks that were registered during the partial transaction, + # so that they don't pollute later code. + # We don't need to do this in `try: ... finally:` because Django's roll + # back logic already clears the callbacks when an exception is raised. + connection.run_on_commit = [] diff --git a/tests/test_test.py b/tests/test_test.py index 1dda72b..0bdea43 100644 --- a/tests/test_test.py +++ b/tests/test_test.py @@ -5,6 +5,7 @@ import pytest from django.db import transaction as django_transaction +from django.test import override_settings from django_subatomic import db, test @@ -43,13 +44,68 @@ def test_callbacks_not_executed_in_normal_test_case(self) -> None: """ Callbacks aren't executed when tests manage the transaction. """ + with test.part_of_a_transaction(): + db.run_after_commit(_callback_which_should_not_be_called) + + def test_dangling_callbacks_cause_an_error_on_enter(self) -> None: + """ + Pre-existing callbacks will be detected and cause an error. + """ + # Django's `atomic` leaves dangling after-commit callbacks + # on the test case's transaction. + with django_transaction.atomic(): + django_transaction.on_commit(_callback_which_should_not_be_called) - def _callback_which_should_not_be_called() -> None: - pytest.fail("Callback should not have been called.") # pragma: no cover + # Ignoring private API here because it's the only way to test this guardrail. + with pytest.raises(test._UnhandledCallbacks) as exc_info: # noqa: SLF001 + with test.part_of_a_transaction(): + ... + assert exc_info.value.callbacks == (_callback_which_should_not_be_called,) + + def test_dangling_callbacks_detection_can_be_disabled(self) -> None: + """ + Pre-existing callbacks can be ignored with a setting. + """ + # Django's `atomic` leaves dangling after-commit callbacks + # on the test case's transaction. + with django_transaction.atomic(): + django_transaction.on_commit(_callback_which_should_not_be_called) + + # This setting suppresses the guardrail. + with override_settings( + SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS=False + ): + with test.part_of_a_transaction(): + ... + + def test_remaining_callbacks_cleared_on_exit(self) -> None: + """ + Any callbacks left at the end of the block are cleared out. + """ with test.part_of_a_transaction(): db.run_after_commit(_callback_which_should_not_be_called) + # If the callbacks weren't cleared, this would raise an error. + with db.transaction(): + ... + + def test_remaining_callbacks_cleared_on_error(self) -> None: + """ + Callbacks left at the end of the block are cleared out when an error is raised. + """ + + class _ArbitraryError(Exception): ... + + with pytest.raises(_ArbitraryError): + with test.part_of_a_transaction(): + db.run_after_commit(_callback_which_should_not_be_called) + raise _ArbitraryError + + # If the callbacks weren't cleared, this would raise an error. + with db.transaction(): + ... + @pytest.mark.parametrize( "transaction_manager", ( @@ -74,3 +130,7 @@ def test_fails_when_nested_inside_an_atomic_block( ): with test.part_of_a_transaction(): ... + + +def _callback_which_should_not_be_called() -> None: + pytest.fail("Callback should not have been called.") # pragma: no cover