Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#22491 closed Cleanup/optimization (fixed)

TestCase + select_for_update()

Reported by: Andreas Pelme Owned by: nobody
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Since 1.6.3 and the select_for_update() changes, TransactionManagementError is raised when run in autocommit mode.

However, in TestCase subclasses select_for_update() with missing transaction.atomic() blocks are not detected, since the test case itself runs in an atomic block.

This means that select_for_update() errors will be silent in most test suites, and then show up in production.

I think a lot of pain could be saved if it was possible to detect these cases and raise an exception in the tests to point out problems. I am not sure how (if) that could be accomplished, but if it was possible, I think it would be a big win. I would be happy to work on a patch if you think this would be a feasible idea.

Change History (7)

comment:1 by Aymeric Augustin, 11 years ago

That looks hard to implement. How would you tell an atomic block created by a TestCase apart from an atomic block in application code exercised by a TransactionTestCase?

If you can propose an implementation that makes sense, yes, let's do it. Otherwise we'll have to document that code involving select_for_update should be tested in a TransactionTestCase.

comment:2 by Tim Graham, 11 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Accepting at least the documentation route if it's not feasible.

comment:3 by mardini, 10 years ago

Component: Database layer (models, ORM)Documentation
Has patch: set
Version: 1.6master

Since this is an old ticket and there doesn't seem to be a straightforward way to fix it, I was inclined to close this in the documentation front.
PR: https://github.com/django/django/pull/2982
Thanks.

comment:4 by Tim Graham, 10 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 668d432d0a25ef68e101f2c6492dc4d75da931bd:

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

Thanks Andreas Pelme for the report.

comment:6 by Tim Graham <timograham@…>, 10 years ago

In 6c70b1d7df4bc6f666ee9fa55a305e7347862095:

[1.6.x] Fixed #22491 -- documented how select_for_update() should be tested.

Thanks Andreas Pelme for the report.

Backport of 668d432d0a from master

comment:7 by Tim Graham <timograham@…>, 10 years ago

In acbe3fac7c1210aacf2a931d9c5dde4f8fecd6c4:

[1.7.x] Fixed #22491 -- documented how select_for_update() should be tested.

Thanks Andreas Pelme for the report.

Backport of 668d432d0a from master

Note: See TracTickets for help on using tickets.
Back to Top