DiscoverRunner does not restore old settings.DEBUG value in teardown

[Chris Jerdonek]

DiscoverRunner.teardown_test_environment() ​does not restore the pre-test value of settings.DEBUG, as for example test.utils.teardown_test_environment() does for ​the values it knows about.

Since test.utils's setup_test_environment() and teardown_test_environment() already have a pattern of storing and restoring environment values, I propose that this be addressed by adding a debug argument to test.utils.setup_test_environment() that DiscoverRunner.setup_test_environment() can then use.

Alternatively, test.utils.setup_test_environment() could take responsibility for setting DEBUG to false.

[Tim Graham]

Is it a practical problem? Typically, more Django code doesn't run at the end of a test run. I guess the argument to setup_test_environment() is okay as long as it defaults to debug=None (i.e. don't change the value) for backwards compatibility.

[Chris Jerdonek]

No, it hasn't been a practical problem for me. But I think it will help conceptually and for consistency to have this logic in one place.

[Chris Jerdonek]

I posted a pull request for this ​here on GitHub.

[Tim Graham]

Left a few comments for improvement.

[Chris Jerdonek]

I'm still thinking about the best way to test (and whether to test for the purposes of this patch).

If possible, I'm thinking it would be better to make the function itself more testable (e.g. by dependency injection) or perhaps by patching. But both of those approaches would be considerably more work than calling teardown_test_environment(), as there are a number of objects in play (e.g. the settings object, the _SavedSettings object, the Template class, the mail and request modules, deactivate(), etc). In a later ticket, it would be straightforward to switch from using the "arbitrary" modules to the _SavedSettings object. That would at least be a simplification, and would make it easier to test directly via patching or dependency injection.

[Tim Graham]

Yes, I think switching things over to _SavedSettings and then using @mock.patch('django.test.utils._SavedSettings') (or whatever the correct incantation of that is) might be the way to go.

[Tim Graham <timograham@…>]

In 7f9fd42:

Fixed #27019 -- Made teardown_test_environment() restore the old DEBUG.