Skip to content

Commit 649d124

Browse files
meshyLilyFirefly
andcommitted
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 <code@lilyf.org>
1 parent a63b4d0 commit 649d124

4 files changed

Lines changed: 66 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
1010
- Added MariaDB and SQLite to the test matrix.
1111
- Disallowed nesting of `part_of_a_transaction` to prevent nonsense
1212
implication of nested partial transactions in tests. Fixes #150.
13+
- `part_of_a_transaction` now raises an error if unhandled callbacks are detected when it starts.
14+
This makes it more similar to `transaction`.
15+
This avoids polluting the test's transaction with callbacks
16+
that were explicitly ignored by using `part_of_a_transaction`.
17+
The error can be silenced by setting the `SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS` setting to `False`
1318

1419
## [1.0.0] - 2026-04-16
1520

docs/reference/settings.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,13 @@ on a per-test basis.
3535
(default: `True`)
3636

3737
[`transaction`][django_subatomic.db.transaction]
38-
will raise `_UnhandledCallbacks` in tests
38+
will raise `subatomic.db._UnhandledCallbacks` in tests
3939
if it detects any lingering unhandled after-commit callbacks
4040
when it's called.
41-
Note: because this exception represents a programming error,
42-
it starts with an underscore to discourage anyone from catching it.
41+
[`part_of_a_transaction`][django_subatomic.test.part_of_a_transaction]
42+
will raise `subatomic.test._UnhandledCallbacks` instead.
43+
Note: because these exceptions each represent a programming error,
44+
they start with an underscore to discourage anyone from catching them.
4345

4446
This highlights order-of-execution issues in tests
4547
caused by after-commit callbacks having not been run.

src/django_subatomic/test.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,33 @@
33
import contextlib
44
from typing import TYPE_CHECKING
55

6+
import attrs
7+
from django.conf import settings
68
from django.db import transaction
79

810

911
if TYPE_CHECKING:
10-
from collections.abc import Generator
12+
from collections.abc import Callable, Generator
1113

1214
__all__ = [
1315
"part_of_a_transaction",
1416
]
1517

1618

19+
@attrs.frozen
20+
class _UnhandledCallbacks(Exception):
21+
"""
22+
Raised in tests when unhandled callbacks are found before calling `part_of_a_transaction`.
23+
24+
This happens when after-commit callbacks are registered
25+
but not run before trying to open a database transaction.
26+
27+
The best solution is to ensure the after-commit callbacks are handled first.
28+
"""
29+
30+
callbacks: tuple[Callable[[], object], ...]
31+
32+
1733
@contextlib.contextmanager
1834
def part_of_a_transaction(using: str | None = None) -> Generator[None]:
1935
"""
@@ -33,6 +49,12 @@ def part_of_a_transaction(using: str | None = None) -> Generator[None]:
3349
use [`transaction`][django_subatomic.db.transaction] instead.
3450
"""
3551
connection = transaction.get_connection(using)
52+
if getattr(
53+
settings, "SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS", True
54+
):
55+
callbacks = connection.run_on_commit
56+
if callbacks:
57+
raise _UnhandledCallbacks(tuple(callback for _, callback, _ in callbacks))
3658

3759
with transaction.atomic(using=using, durable=True):
3860
yield

tests/test_test.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import pytest
77
from django.db import transaction as django_transaction
8+
from django.test import override_settings
89

910
from django_subatomic import db, test
1011

@@ -46,6 +47,38 @@ def test_callbacks_not_executed_in_normal_test_case(self) -> None:
4647
with test.part_of_a_transaction():
4748
db.run_after_commit(_callback_which_should_not_be_called)
4849

50+
def test_dangling_callbacks_cause_an_error_on_enter(self) -> None:
51+
"""
52+
Pre-existing callbacks will be detected and cause an error.
53+
"""
54+
# Django's `atomic` leaves dangling after-commit callbacks
55+
# on the test case's transaction.
56+
with django_transaction.atomic():
57+
django_transaction.on_commit(_callback_which_should_not_be_called)
58+
59+
# Ignoring private API here because it's the only way to test this guardrail.
60+
with pytest.raises(test._UnhandledCallbacks) as exc_info: # noqa: SLF001
61+
with test.part_of_a_transaction():
62+
...
63+
64+
assert exc_info.value.callbacks == (_callback_which_should_not_be_called,)
65+
66+
def test_dangling_callbacks_detection_can_be_disabled(self) -> None:
67+
"""
68+
Pre-existing callbacks can be ignored with a setting.
69+
"""
70+
# Django's `atomic` leaves dangling after-commit callbacks
71+
# on the test case's transaction.
72+
with django_transaction.atomic():
73+
django_transaction.on_commit(_callback_which_should_not_be_called)
74+
75+
# This setting suppresses the guardrail.
76+
with override_settings(
77+
SUBATOMIC_CATCH_UNHANDLED_AFTER_COMMIT_CALLBACKS_IN_TESTS=False
78+
):
79+
with test.part_of_a_transaction():
80+
...
81+
4982
def test_remaining_callbacks_cleared_on_exit(self) -> None:
5083
"""
5184
Any callbacks left at the end of the block are cleared out.

0 commit comments

Comments
 (0)