Skip to content

Commit 2d9c6a7

Browse files
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.
1 parent c54f898 commit 2d9c6a7

File tree

4 files changed

+108
-36
lines changed

4 files changed

+108
-36
lines changed

debug_toolbar/apps.py

+30-2
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
from django.conf import settings
66
from django.core.checks import Error, Warning, register
77
from django.middleware.gzip import GZipMiddleware
8+
from django.urls import NoReverseMatch, reverse
89
from django.utils.module_loading import import_string
910
from django.utils.translation import gettext_lazy as _
1011

11-
from debug_toolbar import settings as dt_settings
12+
from debug_toolbar import APP_NAME, settings as dt_settings
13+
from debug_toolbar.settings import CONFIG_DEFAULTS
1214

1315

1416
class DebugToolbarConfig(AppConfig):
@@ -213,7 +215,33 @@ def debug_toolbar_installed_when_running_tests_check(app_configs, **kwargs):
213215
"""
214216
Check that the toolbar is not being used when tests are running
215217
"""
216-
if not settings.DEBUG and dt_settings.get_config()["IS_RUNNING_TESTS"]:
218+
# Check if show toolbar callback has changed
219+
show_toolbar_changed = (
220+
dt_settings.get_config()["SHOW_TOOLBAR_CALLBACK"]
221+
!= CONFIG_DEFAULTS["SHOW_TOOLBAR_CALLBACK"]
222+
)
223+
try:
224+
# Check if the toolbar's urls are installed
225+
reverse(f"{APP_NAME}:render_panel")
226+
toolbar_urls_installed = True
227+
except NoReverseMatch:
228+
toolbar_urls_installed = False
229+
230+
# If the user is using the default SHOW_TOOLBAR_CALLBACK,
231+
# then the middleware will respect the change to settings.DEBUG.
232+
# However, if the user has changed the callback to:
233+
# DEBUG_TOOLBAR_CONFIG = {"SHOW_TOOLBAR_CALLBACK": lambda request: DEBUG}
234+
# where DEBUG is not settings.DEBUG, then it won't pick up that Django'
235+
# test runner has changed the value for settings.DEBUG, and the middleware
236+
# will inject the toolbar, while the URLs aren't configured leading to a
237+
# NoReverseMatch error.
238+
likely_error_setup = show_toolbar_changed and not toolbar_urls_installed
239+
240+
if (
241+
not settings.DEBUG
242+
and dt_settings.get_config()["IS_RUNNING_TESTS"]
243+
and likely_error_setup
244+
):
217245
return [
218246
Error(
219247
"The Django Debug Toolbar can't be used with tests",

docs/changes.rst

+3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ Pending
66

77
* Fixed overriding font-family for both light and dark themes.
88
* Restored compatibility with ``iptools.IpRangeList``.
9+
* Limit ``E001`` check to likely error cases when the
10+
``SHOW_TOOLBAR_CALLBACK`` has changed, but the toolbar's URL
11+
paths aren't installed.
912

1013
4.4.2 (2024-05-27)
1114
------------------

docs/configuration.rst

+10
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,16 @@ Toolbar options
152152
implication is that it is possible to execute arbitrary SQL through the
153153
SQL panel when the ``SECRET_KEY`` value is leaked somehow.
154154

155+
.. warning::
156+
157+
Do not use
158+
``DEBUG_TOOLBAR_CONFIG = {"SHOW_TOOLBAR_CALLBACK": lambda request: DEBUG}``
159+
in your project's settings.py file. The toolbar expects to use
160+
``django.conf.settings.DEBUG``. Using your project's setting's ``DEBUG``
161+
is likely to cause unexpected results when running your tests. This is because
162+
Django automatically sets ``settings.DEBUG = False``, but your project's
163+
setting's ``DEBUG`` will still be set to ``True``.
164+
155165
.. _OBSERVE_REQUEST_CALLBACK:
156166

157167
* ``OBSERVE_REQUEST_CALLBACK``

tests/test_checks.py

+65-34
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
from unittest.mock import patch
22

3-
from django.core.checks import Error, Warning, run_checks
3+
from django.core.checks import Warning, run_checks
44
from django.test import SimpleTestCase, override_settings
5+
from django.urls import NoReverseMatch
56

6-
from debug_toolbar import settings as dt_settings
77
from debug_toolbar.apps import debug_toolbar_installed_when_running_tests_check
88

99

@@ -239,39 +239,70 @@ def test_check_w007_invalid(self, mocked_guess_type):
239239
],
240240
)
241241

242-
def test_debug_toolbar_installed_when_running_tests(self):
243-
with self.settings(DEBUG=True):
244-
# Update the config options because self.settings()
245-
# would require redefining DEBUG_TOOLBAR_CONFIG entirely.
246-
dt_settings.get_config()["IS_RUNNING_TESTS"] = True
247-
errors = debug_toolbar_installed_when_running_tests_check(None)
248-
self.assertEqual(len(errors), 0)
249-
250-
dt_settings.get_config()["IS_RUNNING_TESTS"] = False
251-
errors = debug_toolbar_installed_when_running_tests_check(None)
252-
self.assertEqual(len(errors), 0)
253-
with self.settings(DEBUG=False):
254-
dt_settings.get_config()["IS_RUNNING_TESTS"] = False
255-
errors = debug_toolbar_installed_when_running_tests_check(None)
256-
self.assertEqual(len(errors), 0)
242+
@patch("debug_toolbar.apps.reverse")
243+
def test_debug_toolbar_installed_when_running_tests(self, reverse):
244+
params = [
245+
{
246+
"debug": True,
247+
"running_tests": True,
248+
"show_callback_changed": True,
249+
"urls_installed": False,
250+
"errors": False,
251+
},
252+
{
253+
"debug": False,
254+
"running_tests": False,
255+
"show_callback_changed": True,
256+
"urls_installed": False,
257+
"errors": False,
258+
},
259+
{
260+
"debug": False,
261+
"running_tests": True,
262+
"show_callback_changed": False,
263+
"urls_installed": False,
264+
"errors": False,
265+
},
266+
{
267+
"debug": False,
268+
"running_tests": True,
269+
"show_callback_changed": True,
270+
"urls_installed": True,
271+
"errors": False,
272+
},
273+
{
274+
"debug": False,
275+
"running_tests": True,
276+
"show_callback_changed": True,
277+
"urls_installed": False,
278+
"errors": True,
279+
},
280+
]
281+
for config in params:
282+
with self.subTest(**config):
283+
config_setting = {
284+
"RENDER_PANELS": False,
285+
"IS_RUNNING_TESTS": config["running_tests"],
286+
"SHOW_TOOLBAR_CALLBACK": (
287+
(lambda *args: True)
288+
if config["show_callback_changed"]
289+
else "debug_toolbar.middleware.show_toolbar"
290+
),
291+
}
292+
if config["urls_installed"]:
293+
reverse.side_effect = lambda *args: None
294+
else:
295+
reverse.side_effect = NoReverseMatch()
257296

258-
dt_settings.get_config()["IS_RUNNING_TESTS"] = True
259-
errors = debug_toolbar_installed_when_running_tests_check(None)
260-
self.assertEqual(
261-
errors,
262-
[
263-
Error(
264-
"The Django Debug Toolbar can't be used with tests",
265-
hint="Django changes the DEBUG setting to False when running "
266-
"tests. By default the Django Debug Toolbar is installed because "
267-
"DEBUG is set to True. For most cases, you need to avoid installing "
268-
"the toolbar when running tests. If you feel this check is in error, "
269-
"you can set `DEBUG_TOOLBAR_CONFIG['IS_RUNNING_TESTS'] = False` to "
270-
"bypass this check.",
271-
id="debug_toolbar.E001",
272-
)
273-
],
274-
)
297+
with self.settings(
298+
DEBUG=config["debug"], DEBUG_TOOLBAR_CONFIG=config_setting
299+
):
300+
errors = debug_toolbar_installed_when_running_tests_check(None)
301+
if config["errors"]:
302+
self.assertEqual(len(errors), 1)
303+
self.assertEqual(errors[0].id, "debug_toolbar.E001")
304+
else:
305+
self.assertEqual(len(errors), 0)
275306

276307
@override_settings(
277308
DEBUG_TOOLBAR_CONFIG={

0 commit comments

Comments
 (0)