Opened 12 years ago
Closed 12 years ago
#21263 closed Bug (fixed)
override_settings in inherited classes should take precedence
| Reported by: | Sylvain Fankhauser | Owned by: | nobody | 
|---|---|---|---|
| Component: | Testing framework | Version: | dev | 
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no | 
| Needs tests: | no | Patch needs improvement: | no | 
| Easy pickings: | no | UI/UX: | no | 
Description
Consider the following test case:
@override_settings(TEST='super')
class TestSettingsOverrideInheritanceSuper(TestCase):
    def test_override_settings_inheritance(self):
        self.assertEqual(settings.TEST, 'super')
@override_settings(TEST='child')
class TestSettingsOverrideInheritanceChild(TestSettingsOverrideInheritanceSuper):
    def test_override_settings_inheritance(self):
        self.assertEqual(settings.TEST, 'child')
The expected behaviour is that the 2 tests pass. Instead, this will fail because the _pre_setup method of override_settings enables the current class settings before running the original_pre_setup method (which will enable the parent class settings).
You can easily reproduce the issue by using the code above in any test suite.
Change History (6)
comment:1 by , 12 years ago
| Triage Stage: | Unreviewed → Accepted | 
|---|
comment:2 by , 12 years ago
comment:3 by , 12 years ago
That's the issue I just faced and I was about to comment on the exact same tests.
On the other hand, the current implementation also causes issues with other existing tests, we just can't see it because of the way these are written. For example you couldn't use override_settings in TestServeDisabled (in staticfiles_tests) to set DEBUG = False because it would then use the value from the base class, StaticFilesTestCase.
comment:4 by , 12 years ago
| Has patch: | set | 
|---|
I've tried another approach to customized settings to solve this issue: https://github.com/django/django/pull/1732
comment:5 by , 12 years ago
| Triage Stage: | Accepted → Ready for checkin | 
|---|
Looks good to me. Tests all pass locally and code looks sane.
comment:6 by , 12 years ago
| Resolution: | → fixed | 
|---|---|
| Status: | new → closed | 
Unfortunately, it's not as simple as exchanging order of lines (
original_pre_setupvsenable). If you override settings after pre_setup, settings are not overriden during that phase where for instances fixtures are loaded. Some test cases (i.e. timezoneAdminTests) depend on settings being overriden during fixture loading.