Prevent check from erroring when ROOT_URLCONF is not defined

[scott-8]

Description

The check debug_toolbar_installed_when_running_tests_check checks if the toolbar's URLs are installed by calling reverse. This can cause issues when ROOT_URLCONF is not defined. In this case, reverse will throw an AttributeError. The check should ignore this error, since if there is no ROOT_URLCONF then it means toolbar_urls_installed should be false.

This comes into play when you're using separate settings files for things like management commands, where it's not necessary to define a ROOT_URLCONF.

For reference, the exception in Django gets raised from this line.

Checklist:

• I have added the relevant tests for this change.
• I have added an item to the Pending section of docs/changes.rst.

AI/LLM Usage

• This PR includes code generated with the help of an AI/LLM

[matthiask]

I wonder if we shouldn't do the same thing Django's own checks do and check getattr(settings, "ROOT_URLCONF", None) first. Catching any AttributeError feels a bit too broad to be honest, but if there are additional reasons for that or if my suggestion doesn't make sense I'm good with the change as it is right now.

[scott-8]

@matthiask I had thought about that, the reason I went more broad is because there are uses where a Django app might not use ROOT_URLCONF. From the Django docs:

Django determines the root URLconf module to use. Ordinarily, this is the value of the ROOT_URLCONF setting, but if the incoming HttpRequest object has a urlconf attribute (set by middleware), its value will be used in place of the ROOT_URLCONF setting.

I wasn't sure if there was anything in Django explicitly requiring that ROOT_URLCONF to be set, so I leaned towards overly broad.

Alternatively, we could check that django.urls.resolvers.get_resolver does not error, since that is where the error gets raised, but it would make the logic a bit more complicated. Open to suggestions.

[scott-8]

Could do something like this, but it seems a bit messy

try:
    get_resolver()
except AttributeError:
    toolbar_urls_installed = False
else:
    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

[matthiask]

You're raising good points. That said, I have never encountered a Django project until now where ROOT_URLCONF wasn't set. Of course someone could run set_urlconf some other way, which they would have to do anyway since request.urlconf doesn't exist without a current request -- which doesn't exist when running system checks. So, I have a hard time imagining an environment where someone would do advanced things like configuring the root URLconf completely without ROOT_URLCONF while not being OK with silencing the offending system check.

So, long story short: I think I lean towards the simple solution of skipping the check if settings.ROOT_URLCONF isn't defined. What do you think?

(Context: I'm definitely one of the people messing around with request.urlconf in feincms3 and I'm finding it very useful. ROOT_URLCONF is always set to the base URLconf though.)

[scott-8]

I agree with you. If someone is messing around with urlconf and really didn't want a valid ROOT_URLCONF, they could also set it to None.

I'll push up new changes.

[matthiask]

Thank you!

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
debug_toolbar
apps.py					255
Project Total

This report was generated by python-coverage-comment-action