An error in TestCase.setUpTestData() leaks data on databases without transactions

[Tim Graham]

While working on ​a backend for MongoDB, I found that an exception on ​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.

[Simon Charette]

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

django/test/testcases.py

@@ -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 +1125 @@
         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.

[Tim Graham]

The ​PR lacks any tests. Perhaps inspiration could be taken from 0abb06930fc0686cb35079934e5bb40df66f5691.

[Hisham Mahmood]

​PR

[JaeHyuckSa]

As I also mentioned on the other ticket, please refrain from modifying the Triage Stage directly.

[blighj]

In their defence, there is guidance that says members can mark tickets ready for checkin
​https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#ready-for-checkin

The ticket was reviewed by any member of the community other than the person who supplied the patch and found to meet all the requirements for a commit-ready contribution. A merger now needs to give a final review prior to being committed.

​https://forum.djangoproject.com/t/django-ticket-status-change/16239

It does sort of elude that if a member things a patch meets all the requirements they can mark it ready for checkin
I know it's not the usual practise though.

[Jacob Walls]

Right, I like it when people set Ready for Checkin, because it clarifies that I'm not the blocker for everything. The other mergers and I are only blockers on the stuff marked RFC. So when someone does that, it floats to the top of my list as a way of respecting the time the reviewer donated.