#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 , 10 years ago
comment:2 by , 10 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Cleanup/optimization |
Accepting at least the documentation route if it's not feasible.
comment:3 by , 10 years ago
| Component: | Database layer (models, ORM) → Documentation |
|---|---|
| Has patch: | set |
| Version: | 1.6 → master |
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 , 10 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:5 by , 10 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
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.