From 2522aec8b2176d1acee8c9d267a64822dac5818a Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Fri, 1 May 2026 12:54:51 +0100 Subject: [PATCH 1/5] Make test helper available to more tests --- tests/test_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_test.py b/tests/test_test.py index 1dda72b..875b165 100644 --- a/tests/test_test.py +++ b/tests/test_test.py @@ -43,10 +43,6 @@ def test_callbacks_not_executed_in_normal_test_case(self) -> None: """ Callbacks aren't executed when tests manage the transaction. """ - - def _callback_which_should_not_be_called() -> None: - pytest.fail("Callback should not have been called.") # pragma: no cover - with test.part_of_a_transaction(): db.run_after_commit(_callback_which_should_not_be_called) @@ -74,3 +70,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 From 835f15da1335d3917337a3f6c0445bceb142475a Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Fri, 1 May 2026 15:07:01 +0100 Subject: [PATCH 2/5] Clear callbacks when exiting part_of_a_transaction Before this change, after-commit callbacks would pollute the transaction in which tests run, and would sometimes be run by later code. This change wipes the list of after-commit callbacks from the connection before `part_of_a_transaction` completes. Co-authored-by: Lily --- CHANGELOG.md | 2 ++ src/django_subatomic/test.py | 8 ++++++++ tests/test_test.py | 27 +++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6944d88..1a35599 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Added MariaDB and SQLite to the test matrix. - 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/src/django_subatomic/test.py b/src/django_subatomic/test.py index 3fde7b5..9652fff 100644 --- a/src/django_subatomic/test.py +++ b/src/django_subatomic/test.py @@ -32,5 +32,13 @@ 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) + 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 875b165..8120657 100644 --- a/tests/test_test.py +++ b/tests/test_test.py @@ -46,6 +46,33 @@ def test_callbacks_not_executed_in_normal_test_case(self) -> None: with test.part_of_a_transaction(): db.run_after_commit(_callback_which_should_not_be_called) + 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", ( From 3e53ddc95215c288786574dbf3e73e40abf7ae67 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Fri, 1 May 2026 16:03:31 +0100 Subject: [PATCH 3/5] Catch and raise more dangling unhandled callbacks The test util `part_of_a_transaction` didn't raise errors when dangling callbacks were detected in the same way in which `transaction` does. It is now strict in the same way, and shares the same setting to silence the error, `settings.SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS`. Co-authored-by: Lily --- CHANGELOG.md | 3 +++ docs/reference/settings.md | 8 +++++--- src/django_subatomic/test.py | 24 +++++++++++++++++++++++- tests/test_test.py | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a35599..300db4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Added MariaDB and SQLite to the test matrix. - Disallowed nesting of `part_of_a_transaction` to prevent nonsense implication of nested partial transactions in tests. Fixes #150. +- `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` - `part_of_a_transaction` now clears after-commit callbacks from the transaction before it exits. This avoids polluting the test's transaction with callbacks. diff --git a/docs/reference/settings.md b/docs/reference/settings.md index 1ba5241..66c7f12 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -35,11 +35,13 @@ on a per-test basis. (default: `True`) [`transaction`][django_subatomic.db.transaction] -will raise `_UnhandledCallbacks` in tests +will raise `subatomic.db._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. +[`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 9652fff..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]: """ @@ -33,6 +49,12 @@ def part_of_a_transaction(using: str | None = None) -> Generator[None]: 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 diff --git a/tests/test_test.py b/tests/test_test.py index 8120657..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 @@ -46,6 +47,38 @@ def test_callbacks_not_executed_in_normal_test_case(self) -> None: 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) + + # 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. From 9bd91c6fe21ed609c7a1b666e781d7e2898177c1 Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Fri, 1 May 2026 15:51:11 +0100 Subject: [PATCH 4/5] Document possible transaction_if_not_already error This function can raise an error, but that was not previously documented. Co-authored-by: Lily --- docs/reference/settings.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/reference/settings.md b/docs/reference/settings.md index 66c7f12..9240880 100644 --- a/docs/reference/settings.md +++ b/docs/reference/settings.md @@ -35,9 +35,10 @@ on a per-test basis. (default: `True`) [`transaction`][django_subatomic.db.transaction] +and [`transaction_if_not_already`][django_subatomic.db.transaction_if_not_already] will raise `subatomic.db._UnhandledCallbacks` in tests -if it detects any lingering unhandled after-commit callbacks -when it's called. +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, From 4835517bea754c27b2bd3c1f6f89de6e4ef6737a Mon Sep 17 00:00:00 2001 From: Charlie Denton Date: Fri, 1 May 2026 16:10:13 +0100 Subject: [PATCH 5/5] Arrange latest changes into sections --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 300db4c..367d549 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,15 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added - Added MariaDB and SQLite to the test matrix. -- Disallowed nesting of `part_of_a_transaction` to prevent nonsense - implication of nested partial transactions in tests. Fixes #150. - `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.