#31944: Use addCleanup() to register TestContextDecorator cleanups
------------------------------------------------+------------------------
Reporter: François Freitag | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
`unittest.TestCase` offers an
[https://docs.python.org/3/library/unittest.html#unittest.TestCase.addCleanup
addCleanup()] function that is guaranteed to run even if
`unittest.TestCase.setUp()` fails.
In #29024, the cleanup was implemented by
[https://github.com/django/django/blob/0b0658111cba538b91072b9a133fd5545f3f46d1/django/test/utils.py#L357-L361
catching Exception, calling TestContextDecorator.disable() and re-
raising]. Using `addCleanup()` allows a small simplification. More
importantly, it prevents an ordering issue with the cleanups that can
cause tests to bleed when subclasses of Django `TestCase` relying of
`addCleanup()` have their cleanups occurring after the
`TestContextDecorator.disable()`. To illustrate:
{{{#!python
class ChangeCSRFCookieName(TestContextDecorator):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.stack = contextlib.ExitStack()
def enable(self):
cm =
override_settings(CSRF_COOKIE_NAME="cross_site_request_forgery_token")
self.stack.enter_context(cm)
def disable(self):
self.stack.close()
class ChangeSettingsTestCase(SimpleTestCase):
def setUp(self):
super().setUp()
settings_cm = self.settings(CACHES={})
settings_cm.enable()
self.addCleanup(settings_cm.disable)
@ChangeCSRFCookieName()
class ChangeCSRFCookieNameTest(ChangeSettingsTestCase):
def test_initial(self):
pass
class LeakTest(SimpleTestCase):
def test_csrf_cookie_name_has_leaked(self):
# Should use the default value.
self.assertEqual("csrftoken", settings.CSRF_COOKIE_NAME)
# Fails with:
#======================================================================
#FAIL: test_csrf_cookie_name_has_leaked (test_utils.tests.LeakTest)
#----------------------------------------------------------------------
#Traceback (most recent call last):
# File "/home/freitafr/dev/django-side/tests/test_utils/tests.py", line
1514, in test_csrf_cookie_name_has_leaked
# self.assertEqual("csrftoken", settings.CSRF_COOKIE_NAME)
#AssertionError: 'csrftoken' != 'cross_site_request_forgery_token'
#- csrftoken
#+ cross_site_request_forgery_token
}}}
The cleanups from `addCleanup()` are scheduled to happen in reverse order
to the order they are added (LIFO). That should make sure each cleanup is
executed from the innermost to the outermost.
I am happy to submit a patch for the fix, but I am concerned the change
may be backward incompatible. Perhaps people relied on this ordering, and
“fixing” the ordering will trigger test bleeding in their test suite. Is
`TestContextDecorator` considered private enough to be changed with a
mention in the release note?
--
Ticket URL: <https://code.djangoproject.com/ticket/31944>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/058.1a50cbc4a122afba45b9528b4b9187e5%40djangoproject.com.
