[Django] #22491: TestCase + select_for_update()
Django
----------------------------------------------+--------------------
Reporter: andreas_pelme | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22491>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: andreas_pelme | Owner: nobody
Type: Uncategorized | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22491#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version: 1.6
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Accepting at least the documentation route if it's not feasible.
--
Ticket URL: <https://code.djangoproject.com/ticket/22491#comment:2>
Django
--------------------------------------+------------------------------------
Reporter: andreas_pelme | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mardini):
* has_patch: 0 => 1
* version: 1.6 => master
* component: Database layer (models, ORM) => Documentation
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22491#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization | Version: master
Component: Documentation | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/22491#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Documentation | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"668d432d0a25ef68e101f2c6492dc4d75da931bd"]:
{{{
#!CommitTicketReference repository=""
revision="668d432d0a25ef68e101f2c6492dc4d75da931bd"
Fixed #22491 -- documented how select_for_update() should be tested.
Thanks Andreas Pelme for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22491#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Documentation | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"6c70b1d7df4bc6f666ee9fa55a305e7347862095"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22491#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Documentation | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"acbe3fac7c1210aacf2a931d9c5dde4f8fecd6c4"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22491#comment:7>