Opened 7 years ago
Closed 7 years ago
#29673 closed Bug (fixed)
Thread urlconf isn't reset after response complete
| Reported by: | tpict | Owned by: | Matthew Power |
|---|---|---|---|
| Component: | Core (URLs) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Keryn Knight, Herbert Fortes, Adam Johnson | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
When setting the urlconf on a request (e.g. in middleware for handling multiple domains pointing to the same Django app), it's not reset until the start of the next request. Since urlconf is threadlocal, this causes problems when running a suite of tests, even if the tests pass when ran individually. For example:
- Django test client makes a request that triggers a middleware to change the urlconf
-
reverseis called with nourlconfkwarg, expecting to be given the urlconf specified byROOT_URLCONF - test throws
NoReverseMatch
I took this problem to the IRC and found that another person recently messaged about the same thing: https://botbot.me/freenode/django/2018-08-07/?msg=103000008&page=3
Change History (13)
comment:1 by , 7 years ago
| Version: | 2.1 → 2.0 |
|---|
comment:2 by , 7 years ago
comment:3 by , 7 years ago
| Cc: | added |
|---|
comment:4 by , 7 years ago
There are a number of places in Django's test suite that set request.urlconf so I think a test is indeed needed to demonstrate the issue.
follow-up: 6 comment:5 by , 7 years ago
| Resolution: | → needsinfo |
|---|---|
| Status: | new → closed |
comment:6 by , 7 years ago
Replying to Tim Graham:
Sorry about the delay. I've just written a test case: https://github.com/tpict/django-29673-test-case
comment:7 by , 7 years ago
| Resolution: | needsinfo |
|---|---|
| Status: | closed → new |
comment:8 by , 7 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Version: | 2.0 → master |
Hi Tom.
Thanks for the example project. It's a curious one:
(tmp-d8650c87792fd55) ~/Downloads/django-29673-test-case-master $ ./manage.py test
..E.
======================================================================
ERROR: test_primary (django29673.secondary.tests.SecondaryTestCase)
Do stuff with the primary site.
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/carlton/Downloads/django-29673-test-case-master/django29673/secondary/tests.py", line 12, in test_primary
response = self.client.get(reverse("primary-site"))
File "/Users/carlton/ve/tmp-d8650c87792fd55/lib/python3.6/site-packages/django/urls/base.py", line 90, in reverse
return iri_to_uri(resolver._reverse_with_prefix(view, prefix, *args, **kwargs))
File "/Users/carlton/ve/tmp-d8650c87792fd55/lib/python3.6/site-packages/django/urls/resolvers.py", line 622, in _reverse_with_prefix
raise NoReverseMatch(msg)
django.urls.exceptions.NoReverseMatch: Reverse for 'primary-site' not found. 'primary-site' is not a valid view function or pattern name.
----------------------------------------------------------------------
Ran 4 tests in 0.017s
FAILED (errors=1)
(tmp-d8650c87792fd55) ~/Downloads/django-29673-test-case-master $ ./manage.py test django29673.secondary.tests.SecondaryTestCase
..
----------------------------------------------------------------------
Ran 2 tests in 0.009s
OK
The workaround you demonstrate in the the middleware.py file is interesting:
# To make tests pass, comment the return statement and uncomment this
# block:
try:
return self.get_response(request)
finally:
if getattr(request, "urlconf", None) is not None:
set_urlconf(None)
If you'd like to push this forward, that would be great. Next step I guess I would be to bring the test case into the Django test suite in a PR.
comment:9 by , 7 years ago
| Cc: | added |
|---|
comment:10 by , 7 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:11 by , 7 years ago
| Cc: | added |
|---|
Any chance you could put this into a test case?