id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
31944	Use addCleanup() to register TestContextDecorator cleanups	François Freitag	nobody	"`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?
"	Cleanup/optimization	new	Testing framework	dev	Normal				Unreviewed	0	0	0	0	0	0