From 2d9c6a7cdd448429047623ab4985c9ef984b9735 Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Wed, 3 Jul 2024 11:06:58 -0500 Subject: [PATCH] Limit the cases for E001 to likely scenarios (#1925) Only when the user is customizing both the show toolbar callback setting and the URLs aren't installed will the underlying NoReverseMatch error occur. --- debug_toolbar/apps.py | 32 +++++++++++++- docs/changes.rst | 3 ++ docs/configuration.rst | 10 +++++ tests/test_checks.py | 99 +++++++++++++++++++++++++++--------------- 4 files changed, 108 insertions(+), 36 deletions(-) diff --git a/debug_toolbar/apps.py b/debug_toolbar/apps.py index a2e977d84..a49875bac 100644 --- a/debug_toolbar/apps.py +++ b/debug_toolbar/apps.py @@ -5,10 +5,12 @@ from django.conf import settings from django.core.checks import Error, Warning, register from django.middleware.gzip import GZipMiddleware +from django.urls import NoReverseMatch, reverse from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ -from debug_toolbar import settings as dt_settings +from debug_toolbar import APP_NAME, settings as dt_settings +from debug_toolbar.settings import CONFIG_DEFAULTS class DebugToolbarConfig(AppConfig): @@ -213,7 +215,33 @@ def debug_toolbar_installed_when_running_tests_check(app_configs, **kwargs): """ Check that the toolbar is not being used when tests are running """ - if not settings.DEBUG and dt_settings.get_config()["IS_RUNNING_TESTS"]: + # Check if show toolbar callback has changed + show_toolbar_changed = ( + dt_settings.get_config()["SHOW_TOOLBAR_CALLBACK"] + != CONFIG_DEFAULTS["SHOW_TOOLBAR_CALLBACK"] + ) + try: + # Check if the toolbar's urls are installed + reverse(f"{APP_NAME}:render_panel") + toolbar_urls_installed = True + except NoReverseMatch: + toolbar_urls_installed = False + + # If the user is using the default SHOW_TOOLBAR_CALLBACK, + # then the middleware will respect the change to settings.DEBUG. + # However, if the user has changed the callback to: + # DEBUG_TOOLBAR_CONFIG = {"SHOW_TOOLBAR_CALLBACK": lambda request: DEBUG} + # where DEBUG is not settings.DEBUG, then it won't pick up that Django' + # test runner has changed the value for settings.DEBUG, and the middleware + # will inject the toolbar, while the URLs aren't configured leading to a + # NoReverseMatch error. + likely_error_setup = show_toolbar_changed and not toolbar_urls_installed + + if ( + not settings.DEBUG + and dt_settings.get_config()["IS_RUNNING_TESTS"] + and likely_error_setup + ): return [ Error( "The Django Debug Toolbar can't be used with tests", diff --git a/docs/changes.rst b/docs/changes.rst index 2eaece240..6dbfe829a 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -6,6 +6,9 @@ Pending * Fixed overriding font-family for both light and dark themes. * Restored compatibility with ``iptools.IpRangeList``. +* Limit ``E001`` check to likely error cases when the + ``SHOW_TOOLBAR_CALLBACK`` has changed, but the toolbar's URL + paths aren't installed. 4.4.2 (2024-05-27) ------------------ diff --git a/docs/configuration.rst b/docs/configuration.rst index 04694aceb..80e44984f 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -152,6 +152,16 @@ Toolbar options implication is that it is possible to execute arbitrary SQL through the SQL panel when the ``SECRET_KEY`` value is leaked somehow. + .. warning:: + + Do not use + ``DEBUG_TOOLBAR_CONFIG = {"SHOW_TOOLBAR_CALLBACK": lambda request: DEBUG}`` + in your project's settings.py file. The toolbar expects to use + ``django.conf.settings.DEBUG``. Using your project's setting's ``DEBUG`` + is likely to cause unexpected results when running your tests. This is because + Django automatically sets ``settings.DEBUG = False``, but your project's + setting's ``DEBUG`` will still be set to ``True``. + .. _OBSERVE_REQUEST_CALLBACK: * ``OBSERVE_REQUEST_CALLBACK`` diff --git a/tests/test_checks.py b/tests/test_checks.py index 827886db1..27db92a9d 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -1,9 +1,9 @@ from unittest.mock import patch -from django.core.checks import Error, Warning, run_checks +from django.core.checks import Warning, run_checks from django.test import SimpleTestCase, override_settings +from django.urls import NoReverseMatch -from debug_toolbar import settings as dt_settings from debug_toolbar.apps import debug_toolbar_installed_when_running_tests_check @@ -239,39 +239,70 @@ def test_check_w007_invalid(self, mocked_guess_type): ], ) - def test_debug_toolbar_installed_when_running_tests(self): - with self.settings(DEBUG=True): - # Update the config options because self.settings() - # would require redefining DEBUG_TOOLBAR_CONFIG entirely. - dt_settings.get_config()["IS_RUNNING_TESTS"] = True - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual(len(errors), 0) - - dt_settings.get_config()["IS_RUNNING_TESTS"] = False - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual(len(errors), 0) - with self.settings(DEBUG=False): - dt_settings.get_config()["IS_RUNNING_TESTS"] = False - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual(len(errors), 0) + @patch("debug_toolbar.apps.reverse") + def test_debug_toolbar_installed_when_running_tests(self, reverse): + params = [ + { + "debug": True, + "running_tests": True, + "show_callback_changed": True, + "urls_installed": False, + "errors": False, + }, + { + "debug": False, + "running_tests": False, + "show_callback_changed": True, + "urls_installed": False, + "errors": False, + }, + { + "debug": False, + "running_tests": True, + "show_callback_changed": False, + "urls_installed": False, + "errors": False, + }, + { + "debug": False, + "running_tests": True, + "show_callback_changed": True, + "urls_installed": True, + "errors": False, + }, + { + "debug": False, + "running_tests": True, + "show_callback_changed": True, + "urls_installed": False, + "errors": True, + }, + ] + for config in params: + with self.subTest(**config): + config_setting = { + "RENDER_PANELS": False, + "IS_RUNNING_TESTS": config["running_tests"], + "SHOW_TOOLBAR_CALLBACK": ( + (lambda *args: True) + if config["show_callback_changed"] + else "debug_toolbar.middleware.show_toolbar" + ), + } + if config["urls_installed"]: + reverse.side_effect = lambda *args: None + else: + reverse.side_effect = NoReverseMatch() - dt_settings.get_config()["IS_RUNNING_TESTS"] = True - errors = debug_toolbar_installed_when_running_tests_check(None) - self.assertEqual( - errors, - [ - Error( - "The Django Debug Toolbar can't be used with tests", - hint="Django changes the DEBUG setting to False when running " - "tests. By default the Django Debug Toolbar is installed because " - "DEBUG is set to True. For most cases, you need to avoid installing " - "the toolbar when running tests. If you feel this check is in error, " - "you can set `DEBUG_TOOLBAR_CONFIG['IS_RUNNING_TESTS'] = False` to " - "bypass this check.", - id="debug_toolbar.E001", - ) - ], - ) + with self.settings( + DEBUG=config["debug"], DEBUG_TOOLBAR_CONFIG=config_setting + ): + errors = debug_toolbar_installed_when_running_tests_check(None) + if config["errors"]: + self.assertEqual(len(errors), 1) + self.assertEqual(errors[0].id, "debug_toolbar.E001") + else: + self.assertEqual(len(errors), 0) @override_settings( DEBUG_TOOLBAR_CONFIG={