Use addCleanup() to register TestContextDecorator cleanups

[François Freitag]

unittest.TestCase offers an ​addCleanup() function that is guaranteed to run even if unittest.TestCase.setUp() fails.

In #29024, the cleanup was implemented by ​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:

class ChangeCSRFCookieName(TestContextDecorator):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.stack = contextlib.ExitStack()

    def enable(self):
        # 1. Before setUp, TestContextDecorator runs enable().
        cm = override_settings(CSRF_COOKIE_NAME="cross_site_request_forgery_token")
        self.stack.enter_context(cm)

    def disable(self):
        # 3. After tearDown, TestContextDecorator runs disable(), which restores settings captured in 1.
        # This should be step 4.
        self.stack.close()


class ChangeSettingsTestCase(SimpleTestCase):
    def setUp(self):
        super().setUp()
        # 2. Capture settings after they’ve been changed in 1.
        settings_cm = self.settings(CACHES={})
        settings_cm.enable()
        self.addCleanup(settings_cm.disable)  # 4. Restore settings captured in 2. Should be executed at step 3.


@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?

[François Freitag]

Clarified ordering issue with comments in the example code.