Skip to content

Commit

Permalink
Limit the cases for E001 to likely scenarios (#1925)
Browse files Browse the repository at this point in the history
Only when the user is customizing both the show toolbar callback
setting and the URLs aren't installed will the underlying
NoReverseMatch error occur.
  • Loading branch information
tim-schilling committed Jul 3, 2024
1 parent c54f898 commit 2d9c6a7
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 36 deletions.
32 changes: 30 additions & 2 deletions debug_toolbar/apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
------------------
Expand Down
10 changes: 10 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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``
Expand Down
99 changes: 65 additions & 34 deletions tests/test_checks.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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={
Expand Down

0 comments on commit 2d9c6a7

Please sign in to comment.