Skip to content

Ensure part_of_a_transaction only runs in tests#159

Open
meshy wants to merge 1 commit into
mainfrom
meshy/part_of_a_transaction-fail-outside-tests
Open

Ensure part_of_a_transaction only runs in tests#159
meshy wants to merge 1 commit into
mainfrom
meshy/part_of_a_transaction-fail-outside-tests

Conversation

@meshy
Copy link
Copy Markdown
Collaborator

@meshy meshy commented May 6, 2026

Before this change, part_of_a_transaction could have been used in production code (or transaction test cases) where it would have actually done a COMMIT, and Django would have run after-commit callbacks. This breaks the expectations of part_of_a_transaction because in those cases it really is a transaction.

Now we'll raise an error if there is no test transaction detected.

Fixes #154

Comment thread src/django_subatomic/test.py Outdated
Comment on lines +71 to +72
if len(connection.atomic_blocks) == 0:
raise _OnlyForUseInTests
Copy link
Copy Markdown
Collaborator Author

@meshy meshy May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Is there any reason why this should be behind a setting? My feeling is that a setting will only enable abuse of this test util.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should put this behind a setting. We can always add one later if a use case is discovered.

Comment thread src/django_subatomic/test.py Outdated
@meshy meshy marked this pull request as ready for review May 7, 2026 16:01
@meshy meshy requested a review from a team as a code owner May 7, 2026 16:01
Comment thread src/django_subatomic/test.py Outdated
Comment thread src/django_subatomic/test.py Outdated
Comment on lines +71 to +72
if len(connection.atomic_blocks) == 0:
raise _OnlyForUseInTests
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should put this behind a setting. We can always add one later if a use case is discovered.

Before this change, `part_of_a_transaction` could have been used in
production code (or transaction test cases) where it would have actually
done a `COMMIT`, and Django would have run after-commit callbacks.
This breaks the expectations of `part_of_a_transaction` because in
those cases it really is a transaction.

Now we'll raise an error if there is no test transaction detected.
@meshy meshy force-pushed the meshy/part_of_a_transaction-fail-outside-tests branch from 46693f1 to 13e8350 Compare May 7, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

part_of_a_transaction should not work in a transaction test case

2 participants