Opened 4 years ago
Last modified 2 years ago
#32939 assigned Cleanup/optimization
Permit override_settings to work with test class mixins that don't inherit from unittest.TestCase
Description (last modified by ) ¶
Currently, the @override_settings
decorator raises a ValueError
if it's used to decorate a class that doesn't inherit from SimpleTestCase
:
https://github.com/django/django/blob/56f9579105c324ff15250423bf9f8bdf1634cfb4/django/test/utils.py#L519-L521
However, this prevents the decorator from being useable as a test-class mixin for a number of SimpleTestCase
subclasses. When creating shared test-class functionality, it's often better to use a mixin rather than a concrete TestCase
class. This way the mixin won't have its setUp
methods run unnecessarily on a test-case class with no tests, and it won't count towards the parallel test runner's test-case class count, etc.
The check could instead be done lazily, e.g. inside setUpClass()
, while still having the same desired effect.
I noticed this when seeing if I could convert AuthViewsTestCase into a mixin. This AuthViewsTestCase
change could be done in a nicer way once this ticket is implemented, perhaps even as part of this ticket.
According to the ticket's flags, the next step(s) to move this issue forward are:
- To provide a patch by sending a pull request. Claim the ticket when you start working so that someone else doesn't duplicate effort. Before sending a pull request, review your work against the patch review checklist. Check the "Has patch" flag on the ticket after sending a pull request and include a link to the pull request in the ticket comment when making that update. The usual format is:
[https://github.com/django/django/pull/#### PR]
.
Change History (9)
comment:1 by , 4 years ago
comment:2 by , 4 years ago
Description: | modified (diff) |
---|
comment:3 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Summary: | Change override_settings to do its subclass check lazily → Permit override_settings to work with test class mixins that don't inherit from unittest.TestCase |
comment:4 by , 4 years ago
I realize now that the "non-lazy" check could be kept by modifying it to be done only when the class inherits from unittest.TestCase
(i.e. when the class is a concrete test class):
--- a/django/test/utils.py +++ b/django/test/utils.py @@ -515,7 +515,7 @@ class override_settings(TestContextDecorator): def decorate_class(self, cls): from django.test import SimpleTestCase - if not issubclass(cls, SimpleTestCase): + if issubclass(cls, TestCase) and not issubclass(cls, SimpleTestCase): raise ValueError( "Only subclasses of Django SimpleTestCase can be decorated " "with override_settings")
comment:5 by , 4 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:6 by , 4 years ago
This way the mixin won't have its setUp methods run unnecessarily on a test-case class with no tests, and it won't count towards the parallel test runner's test-case class count, etc.
Investigating further, it turns out neither of these seem to happen in practice, since test classes without tests get filtered out. Nevertheless, being able to apply the decorator to test mixins would still be a useful enhancement.
comment:7 by , 4 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:8 by , 3 years ago
Cc: | added |
---|
comment:11 by , 2 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
PS - should this be a
TypeError
instead ofValueError
?