[Django] #21281: trying to override settings in test case setUpClass
Django
-----------------------------------+--------------------
Reporter: anonymous | Owner: nobody
Type: Uncategorized | Status: new
Component: Testing framework | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------
The override_settings decorator doesn't effect the setUpClass method (I
believe this is because class decorators don't effect classmethods) and as
the self.settings() method requires an object instance, there is no easy
way to change the settings for the setUpClass method.
Is there any reason settings() requires an instance and couldn't be class
based?
--
Ticket URL: <https://code.djangoproject.com/ticket/21281>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: anonymous | Owner: nobody
Type: Uncategorized | Status: new
Component: Testing framework | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by alasdair):
* cc: alasdair (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:1>
Django
Reporter: anonymous | Owner: nobody
Type: Uncategorized | Status: new
Component: Testing framework | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by bmispelon):
Hi,
I can reproduce the issue described (using this testcase:
https://gist.github.com/bmispelon/7041561).
I don't know how fixable it is, but if anything, the limitation should at
least be documented, so I'm accepting this ticket.
Thanks for the report.
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:2>
Django
Reporter: anonymous | Owner: nobody
Type: Uncategorized | Status: new
Component: Testing framework | Version: 1.5
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_docs: 0 => 1
* has_patch: 0 => 1
Comment:
The real reason why settings are not overriden in `setUpClass` is that we
want the settings override process to happen before each test, and not
only once for the entire class, so changed settings cannot leak between
tests. I've attached a patch with a possible approach where we both
override settings once in `setUpClass` and again in `_pre_setup` before
each test. This would be a bit redundant and would require to call the
parent `setUpClass` before settings are really overridden (would need to
be documented).
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:3>
Django
Reporter: anonymous | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* version: 1.5 => master
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:4>
Django
Reporter: anonymous | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tchaumeny):
* cc: t.chaumeny@… (added)
* needs_docs: 1 => 0
Comment:
We discussed it with claudep and timograham on #django-dev and came to the
conclusion that overriding settings directly was a mistake and that we
should not rely on `override_settings` to correct that in some cases. A
specific warning was added to tell users not to do so
https://github.com/django/django/commit/3f651b3e88ac1ba8d04acd5a074362866a0a963a.
The proposed PR moves the whole override logic at class level when the
class is decorated. This way, settings are overriden within `setUpClass`
and `tearDownClass` (I included bmispelon's test case above).
A side effect of that change is that users should always call `super` when
defining `setUpClass`/ `tearDownClass`. This side effect has been
discussed on #20392, which this ticket is a prerequisite.
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: anonymous | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Testing framework | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Testing framework | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"d89f56dc4d03f6bf6602536b8b62602ec0d46d2f"]:
{{{
#!CommitTicketReference repository=""
revision="d89f56dc4d03f6bf6602536b8b62602ec0d46d2f"
Fixed #21281 -- Made override_settings act at class level when used as a
TestCase decorator.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:7>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Testing framework | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e77462249339f465395f49f0d0b149b670a696f3"]:
{{{
#!CommitTicketReference repository=""
revision="e77462249339f465395f49f0d0b149b670a696f3"
Moved release note for refs #21281 from "deprecation" to "backwards
incompatible".
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:8>
Django
Reporter: anonymous | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* has_patch: 1 => 0
* resolution: fixed =>
* severity: Normal => Release blocker
* stage: Ready for checkin => Accepted
Comment:
I found a regression here that causes `override_settings()` to leak when
used on a subclass test class. See
`DateTimePickerShortcutsSeleniumFirefoxTests` in `admin_widgets` (we
didn't notice it at the time because that's a selenium test which isn't
run on Jenkins).
Here's a simple test script which fails on the second test.
{{{
from unittest import TestCase
from django.conf import settings
from django.contrib.admin.tests import AdminSeleniumWebDriverTestCase
from django.test import override_settings
@override_settings(TIME_ZONE='Asia/Singapore')
class Test(AdminSeleniumWebDriverTestCase):
def test(self):
assert True
class Test2(TestCase):
def test(self):
self.assertEqual(settings.TIME_ZONE, "America/Chicago")
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:9>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tchaumeny):
After some investigation, that seems to be a consequence of a duplicated
`super(..., cls).setUpClass()` in `LiveServerTestCase`:
https://github.com/django/django/blob/52f0b2b62262743d5f935ddae29428e661b5d8ea/django/test/testcases.py#L1210
and
https://github.com/django/django/blob/52f0b2b62262743d5f935ddae29428e661b5d8ea/django/test/testcases.py#L1258
(looks like I introduced that)
If I remove one of those lines, the test passes fine.
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:10>
Django
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* has_patch: 0 => 1
Comment:
I'd suggest removing the last one
(https://github.com/django/django/pull/3834). OK?
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:11>
Django
Reporter: anonymous | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: master
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
This fixes the test failure I'm seeing.
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:12>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"3bac904607f1999136b97249d9aa220f1db94258"]:
{{{
#!CommitTicketReference repository=""
revision="3bac904607f1999136b97249d9aa220f1db94258"
Removed extraneous super call in LiveServerTestCase
Refs #21281. Thanks Tim Graham and Thomas Chaumeny for investigations.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:13>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization |
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/21281#comment:14>