#18417 closed Bug (fixed)
override_settings doesn't work with unittest.TestCase subclasses
Reported by: | Claude Paroz | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | 1.4 |
Severity: | Normal | Keywords: | |
Cc: | asendecka@… | Triage Stage: | Design decision needed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Attachments (2)
Change History (11)
by , 12 years ago
Attachment: | 18417-1.diff added |
---|
comment:1 by , 12 years ago
Cc: | added |
---|---|
Triage Stage: | Accepted → Ready for checkin |
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.
follow-up: 3 comment:2 by , 12 years ago
Triage Stage: | Ready for checkin → Design decision needed |
---|
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.
follow-up: 4 comment:3 by , 12 years ago
Replying to aaugustin:
override_settings
is Django-specific; I believe it's fair to restrict it to subclasses ofdjango.test.TransactionTestCase
orTestCase
, 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.
comment:4 by , 12 years ago
Replying to claudep:
Replying to aaugustin:
override_settings
is Django-specific; I believe it's fair to restrict it to subclasses ofdjango.test.TransactionTestCase
orTestCase
, 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.
by , 12 years ago
Attachment: | 18417-2.diff added |
---|
Raise if decorated class is not subclass of SimpleTestCase
comment:5 by , 12 years ago
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).
comment:6 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Allow unittest.TestCase to be decorated by override_settings