override_settings doesn't work with unittest.TestCase subclasses

[Claude Paroz]

Currently, if you decorate with override_settings a subclass of unittest.TestCase, all tests in the class are silently skipped. This is happening because of the issubclass(TransactionTestCase) test here: ​https://github.com/django/django/blob/master/django/test/utils.py#L188

We cannot simply change the issubclass test, because ut.TestCase doesn't have any _pre_setup mechanism.

I can see two ways forward:

• Document that override_settings cannot decorate unittest.TestClass subclasses, and raise an error when the decorator is applied to anything other than TransactionTestCase subclasses.
• Instead of using _pre_setup, use setUpClass to enable the overriden settings.

I'd really like to see the second alternative implemented. However, this means that the override_settings machinery will only run at the start of the TestClass tests, and no more between each test (unless the specific test itself is decorated by override_settings). This shouldn't disturb the test system more than that.

[Claude Paroz]

Allow unittest.TestCase to be decorated by override_settings

[Aleksandra Sendecka]

Patch works nice for me. I tested that by adding new unittest TestCase with assert False. Before applying the patch, the test was skipped and after - the test failed (as expected). That's a pity it's no good way to add a real test for that.

[Aymeric Augustin]

override_settings is Django-specific; I believe it's fair to restrict it to subclasses of django.test.TransactionTestCase or TestCase, provided an exception is raised when it's misused.

Currently, override_settings guarantees that the settings are reset before each test in the test case. Altering them in a test won't affect other tests. This isn't documented but it's likely that some people are relying on this behavior.

So the current patch will lead to test failures for some users after upgrading Django. These test failures won't happen if the failing test is run in isolation, making them hard to track down.

In my eyes this is as a choice between:

• raising an exception for people who've written tests that never ran; and
• taking the risk to introduce unexpected test failures for people who've written working tests.

I'm +1 on the first option and -0 on the second.

[Claude Paroz]

Replying to aaugustin:

override_settings is Django-specific; I believe it's fair to restrict it to subclasses of django.test.TransactionTestCase or TestCase, provided an exception is raised when it's misused.

I don't follow you here. There are clearly use-cases currently in our test suite that would need override_settings for unittest.TestCase. It does not appear so unnatural to me.

Currently, override_settings guarantees that the settings are reset before each test in the test case. Altering them in a test won't affect other tests. This isn't documented but it's likely that some people are relying on this behavior.

That's right, and that's why I didn't commit it until now.

(...)

In my eyes this is as a choice between:

• raising an exception for people who've written tests that never ran; and
• taking the risk to introduce unexpected test failures for people who've written working tests.

I'm +1 on the first option and -0 on the second.

There might be a third option, use _pre_setup/_post_teardown for Django test subclasses, and setUpClass/tearDownClass for unittest.TestCase subclasses. It's a bit peculiar to have different behaviours depending on the test class, but at least compatibility is kept, and we can document it.

[Aymeric Augustin]

Replying to claudep:

Replying to aaugustin:

override_settings is Django-specific; I believe it's fair to restrict it to subclasses of django.test.TransactionTestCase or TestCase, provided an exception is raised when it's misused.

I don't follow you here. There are clearly use-cases currently in our test suite that would need override_settings for unittest.TestCase. It does not appear so unnatural to me.

Right -- unittest.TestCase is sometimes preferred because it's faster.

The current practice in Django's test suite is to use django.test.(Transaction)TestCase when the test cases requires some Django-specific behavior (fixtures, test client, etc.) and unittest.TestCase when it doesn't.

Since override_settings is Django-specific, I suggest switching test cases decorated with @override_settings to django.test.(Transaction)TestCase.

If override_settings raises an exception for unsupported classes, we'll immediately know which ones need this change.

[Claude Paroz]

Raise if decorated class is not subclass of SimpleTestCase

[Claude Paroz]

The attached patch raises an exception if the decorated class is not a subclass of SimpleTestCase. Note that currently this works only for TransactionTestCase, but the current documentation tells that SimpleTestCase can be decorated by override_settings (and I also think it should) (​https://docs.djangoproject.com/en/dev/topics/testing/#django.test.SimpleTestCase).

[Claude Paroz <claude@…>]

In 9f7cefd5059e78d75f1e1a852b51a6286ecec728:

Fixed #18417 -- Raised exception when unittest.TestCase is decorated with override_settings

[Claude Paroz <claude@…>]

In a5d47415f47025b014e580af1533e9023dc32945:

Enabled SimpleTestCase to be decorated by override_settings

Refs #18417. Also fixed some test case classes which subclassed
the wrong parent.

[Claude Paroz <claude@…>]

In 4389b51fabf6d5d98d3d59681a527e0b4fbe4331:

[1.5.x] Fixed #18417 -- Raised exception when unittest.TestCase is decorated with override_settings

Backport of 9f7cefd5 from master.

[Claude Paroz <claude@…>]

In 6945f60c2b2406ba06d889020d50aaba80e7d317:

[1.5.x] Enabled SimpleTestCase to be decorated by override_settings

Refs #18417. Also fixed some test case classes which subclassed
the wrong parent.
Backport of a5d47415f from master.