From d81b778b16c0b571c238c70124bfb713fa0b2bc4 Mon Sep 17 00:00:00 2001 From: tschilling Date: Mon, 27 May 2024 19:14:51 -0500 Subject: [PATCH 1/2] Limit the cases for E001 to likely scenarios 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 | 4 ++ docs/configuration.rst | 10 +++++ tests/test_checks.py | 99 +++++++++++++++++++++++++++--------------- 4 files changed, 109 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 a0215d09c..8f4951f95 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -4,6 +4,10 @@ Change log Pending ------- +* 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..df6337adf 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 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={ From 385262be327dac102a8618521868dc00713fe88c Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Wed, 3 Jul 2024 10:58:59 -0500 Subject: [PATCH 2/2] Update docs/configuration.rst --- docs/configuration.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration.rst b/docs/configuration.rst index df6337adf..80e44984f 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -158,7 +158,7 @@ Toolbar options ``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 when running your tests. This is because + 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``.