Fixed #22491 -- documented how `select_for_update() should be tested. #2982

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

mmardini commented Jul 27, 2014

Thanks Andreas Pelme for the report.

@timgraham timgraham commented on the diff Jul 28, 2014

docs/ref/models/querysets.txt
@@ -1433,6 +1434,17 @@ Using ``select_for_update`` on backends which do not support
It is now an error to execute a query with ``select_for_update()`` in
autocommit mode. With earlier releases in the 1.6 series it was a no-op.
+.. warning::
+
+ Although `select_for_update() normally fails in autocommit mode, since
@timgraham

timgraham Jul 28, 2014

Owner

please use 4 space indent
missing after select_for_update() on this line (but actually all instances of single should be `` for proper formatting)

@timgraham timgraham commented on the diff Jul 28, 2014

docs/ref/models/querysets.txt
@@ -1433,6 +1434,17 @@ Using ``select_for_update`` on backends which do not support
It is now an error to execute a query with ``select_for_update()`` in
autocommit mode. With earlier releases in the 1.6 series it was a no-op.
+.. warning::
+
+ Although `select_for_update() normally fails in autocommit mode, since
+ :class:`~django.test.TestCase` automatically wraps each test in a transaction,
+ calling `select_for_update()` in a `TestCase` even without an
+ :func:`~django.db.transaction.atomic()` block will unexpectedly
+ pass without raising a `TransactionManagementError`. To properly test
+ `select_for_update()` you should use
+ :class:`~django.test.TransactionTestCase`.
+
@timgraham

timgraham Jul 28, 2014

Owner

1 newline between sections

Owner

timgraham commented Jul 28, 2014

Will make minor edits and commit.

Owner

timgraham commented Jul 28, 2014

merged in 668d432, thanks.

timgraham closed this Jul 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment