Code

Opened 6 months ago

Closed 6 months ago

#21263 closed Bug (fixed)

override_settings in inherited classes should take precedence

Reported by: Sephi Owned by: nobody
Component: Testing framework Version: master
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.

Attachments (0)

Change History (6)

comment:1 Changed 6 months ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 months ago by claudep

Unfortunately, it's not as simple as exchanging order of lines (original_pre_setup vs enable). 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. timezone AdminTests) depend on settings being overriden during fixture loading.

comment:3 Changed 6 months ago by Sephi

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 Changed 6 months ago by claudep

  • Has patch set

I've tried another approach to customized settings to solve this issue: https://github.com/django/django/pull/1732

comment:5 Changed 6 months ago by mjtamlyn

  • Triage Stage changed from Accepted to Ready for checkin

Looks good to me. Tests all pass locally and code looks sane.

comment:6 Changed 6 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 949076eb11011736f9e827adfde03c842bc1f1dd:

Fixed #21263 -- Fixed issue with override_settings in inherited classes

When both parent and child classes are decorated with override_settings,
child class settings should take precedence.
Thanks Sephi for the report and Marc Tamlyn for the review.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.