TestCase: an error in setUpTestData snowballs across the entire test suite

[Adam Johnson]

If a setUpTestData function fails, it causes an error that breaks the current transaction; this is not undone by TestCase, and thus all following database access in all other tests fails, outputting a lot of ERRORs on a test run, and making it hard to debug that it was only the *first* one that has an actual problem. The atomic instances can be exited cleanly on exception fairly easily inside the TestCase code.

[Adam Johnson]

​https://github.com/django/django/pull/5051

I've struggled to add a test for this. I had the idea for something like the below, but the problem is that if you try do any checking in the exception handler in setUpClass you can then trigger tearDownClass even though it shouldn't be run. I don't think there is a sane way of testing it, except from creating tests that run a second set of tests where one has a broken setUpTestData, and then asserting that only the one of the second set failed.

diff --git a/tests/test_utils/tests.py b/tests/test_utils/tests.py
index 7056ea3..ede8cca 100644
--- a/tests/test_utils/tests.py
+++ b/tests/test_utils/tests.py
@@ -2,6 +2,7 @@
 from __future__ import unicode_literals
 
 import unittest
+import sys
 
 from django.conf.urls import url
 from django.contrib.staticfiles.finders import get_finder, get_finders
@@ -744,6 +745,26 @@ class SkippingExtraTests(TestCase):
         pass
 
 
+class TestBadSetupTestData(TestCase):
+
+    class MyException(Exception):
+        pass
+
+    @classmethod
+    def setUpClass(cls):
+        try:
+            super(TestBadSetupTestData, cls).setUpClass()
+        except cls.MyException:
+            assert not connection.in_atomic_block
+
+    @classmethod
+    def setUpTestData(cls):
+        raise cls.MyException()
+
+    def test_failure_in_setUpTestData_should_exit_cleanly(self):
+        pass
+
+
 class AssertRaisesMsgTest(SimpleTestCase):
 
     def test_special_re_chars(self):

[Tim Graham]

For the test, could you store the result of connection.in_atomic_block in a variable and then put the assertion in the test method? I didn't understand what you meant when you said, "you can then trigger tearDownClass even though it shouldn't be run".

[Adam Johnson]

The test method would never be reached - we're triggering an error in setUpClass which means the tests don't get run.

The above test patch catches the breakage and allows the tests to continue - in the "broken" world. I'm not sure this is legit or even if it works now it might not be forwards compatible with future edits .

I also experimented with a broken test case with @expectedFailure wrapped around it but that's not sensitive enough since it allows any exception to be expected. It might be possible to write a custom expectedFailure decorator that checks it was the custom exception raised during the test case and marks it as a pass/skip/expected failure, but when I briefly looked it seemed a bit hacky.

[Aymeric Augustin]

If there is no reasonable way to write a test, let's ship the fix as is. That happens occasionally for changes in the testing infrastructure. The latest iteration of the patch looks good to me.

[Tim Graham <timograham@…>]

In 0abb069:

Fixed #25176 -- Prevented TestCase.setUpTestData() exception from leaking transaction.

[Tim Graham <timograham@…>]

In fd81588:

Refs #25176 -- Fixed typo in tests/test_utils/tests.py

[Tim Graham <timograham@…>]

In b46dad1b:

[1.8.x] Fixed #25176 -- Prevented TestCase.setUpTestData() exception from leaking transaction.

Backport of 0abb06930fc0686cb35079934e5bb40df66f5691 from master