[Django] #35548: An error in TestCase.setUpTestData() leaks data on databases without transactions

10 views
Skip to first unread message

Django

unread,
Jun 21, 2024, 10:54:56 AM (8 days ago) Jun 21
to django-...@googlegroups.com
#35548: An error in TestCase.setUpTestData() leaks data on databases without
transactions
---------------------------------------------+------------------------
Reporter: Tim Graham | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+------------------------
While working on [https://github.com/mongodb-labs/django-mongodb a backend
for MongoDB], I found that an exception on
[https://github.com/django/django/blob/72b7aecbbfbec0ceb1a829eef82a68d7283df604/tests/expressions/tests.py#L1445
the fourth line] of
`expressions.tests.ExpressionsNumericTests.setUpTestData` left `Number`
rows in the database such that a later test failed like this:

{{{
======================================================================
ERROR: test_F_reuse (expressions.tests.ExpressionsTests.test_F_reuse)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/tim/code/django/tests/expressions/tests.py", line 1217, in
test_F_reuse
self.assertEqual(n_qs.get(), n)
^^^^^^^^^^
File "/home/tim/code/django/django/db/models/query.py", line 652, in get
raise self.model.MultipleObjectsReturned(
expressions.models.Number.MultipleObjectsReturned: get() returned more
than one Number -- it returned more than 20!
}}}

This is the same problem as #25176 but for databases with
`DatabaseFeatures.supports_transactions = False`.
--
Ticket URL: <https://code.djangoproject.com/ticket/35548>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jun 21, 2024, 12:30:18 PM (8 days ago) Jun 21
to django-...@googlegroups.com
#35548: An error in TestCase.setUpTestData() leaks data on databases without
transactions
-----------------------------------+------------------------------------
Reporter: Tim Graham | Owner: nobody
Type: Bug | Status: new
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+------------------------------------
Changes (by Simon Charette):

* stage: Unreviewed => Accepted

Comment:

I think the following should do and also avoid a lack of rolling back
transaction when they are supported

{{{#!diff
diff --git a/django/test/testcases.py b/django/test/testcases.py
index f1c6b5ae9c..5cd90a7415 100644
--- a/django/test/testcases.py
+++ b/django/test/testcases.py
@@ -7,7 +7,7 @@
import threading
import unittest
from collections import Counter
-from contextlib import contextmanager
+from contextlib import contextmanager, suppress
from copy import copy, deepcopy
from difflib import get_close_matches
from functools import wraps
@@ -1125,6 +1125,11 @@ def _pre_setup(self):
try:
self._fixture_setup()
except Exception:
+ # Attempt to teardown fixtures on exception during setup as
+ # `_post_teardown` won't be triggered to cleanup state when
an
+ # an exception is surfaced to `SimpleTestCase._pre_setup`.
+ with suppress(Exception):
+ self._fixture_teardown()
if self.available_apps is not None:
apps.unset_available_apps()
setting_changed.send(
}}}

Only lightly tested though.
--
Ticket URL: <https://code.djangoproject.com/ticket/35548#comment:1>

Django

unread,
Jun 21, 2024, 11:50:01 PM (8 days ago) Jun 21
to django-...@googlegroups.com
#35548: An error in TestCase.setUpTestData() leaks data on databases without
transactions
-----------------------------------+------------------------------------
Reporter: Tim Graham | Owner: Rish
Type: Bug | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+------------------------------------
Changes (by Rish):

* owner: nobody => Rish
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35548#comment:2>

Django

unread,
Jun 26, 2024, 9:21:46 AM (3 days ago) Jun 26
to django-...@googlegroups.com
#35548: An error in TestCase.setUpTestData() leaks data on databases without
transactions
-----------------------------------+------------------------------------
Reporter: Tim Graham | Owner: Rish
Type: Bug | Status: assigned
Component: Testing framework | Version: dev
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 Rish):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35548#comment:3>

Django

unread,
Jun 26, 2024, 10:22:11 AM (3 days ago) Jun 26
to django-...@googlegroups.com
#35548: An error in TestCase.setUpTestData() leaks data on databases without
transactions
-----------------------------------+------------------------------------
Reporter: Tim Graham | Owner: Rish
Type: Bug | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+------------------------------------
Changes (by Tim Graham):

* needs_tests: 0 => 1

Comment:

The [https://github.com/django/django/pull/18312 PR] lacks any tests.
Perhaps inspiration could be taken from
0abb06930fc0686cb35079934e5bb40df66f5691.
--
Ticket URL: <https://code.djangoproject.com/ticket/35548#comment:4>
Reply all
Reply to author
Forward
0 new messages