Opened 13 years ago
Last modified 13 years ago
#16412 closed Cleanup/optimization
Disabling an 'django.contrib.sites' app, causes Try disabling an 'django.contrib.sites' app, (which as it is a contrib app, should be an optional), and you'll get an error in tests with django.contrib.auth.tests.forms.PasswordResetFormTest — at Version 11
Reported by: | Matt Harasymczuk | Owned by: | nobody |
---|---|---|---|
Component: | contrib.auth | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | djangoproject.com@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Disabling 'django.contrib.sites' app causes error in django.contrib.auth.tests.forms.PasswordResetFormTest when running manage.py test
Change History (12)
comment:1 by , 13 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 13 years ago
I am not sure, I have been using contrib.auth without contrib.sites for a few years now.
comment:3 by , 13 years ago
Hmm, maybe I'm wrong then. Feel free to re-open but you should provide more information including the traceback from the failing test.
comment:4 by , 13 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
Creating test database for alias 'default'... ..............................................................E................. ................................................................................ ................................................................................ ................................................................................ ...... ====================================================================== ERROR: test_custom_email_subject (django.contrib.auth.tests.forms.PasswordResetFormTest) ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\Python27\Lib\site-packages\django\contrib\auth\tests\forms.py", line 264, in test_custom_email_subject form.save() File "C:\Python27\Lib\site-packages\django\contrib\auth\forms.py", line 138, in save current_site = get_current_site(request) File "C:\Python27\Lib\site-packages\django\contrib\sites\models.py", line 94, in get_current_site current_site = RequestSite(request) File "C:\Python27\Lib\site-packages\django\contrib\sites\models.py", line 74, in __init__ self.domain = self.name = request.get_host() AttributeError: 'NoneType' object has no attribute 'get_host' ---------------------------------------------------------------------- Ran 326 tests in 5.113s FAILED (errors=1) Destroying test database for alias 'default'...
comment:5 by , 13 years ago
it is fully functional without django.contrib.sites app, no errors, but this one test fails.
comment:6 by , 13 years ago
What depends on the contrib.sites contrib apps is the contrib.auth.forms module. It changed in [13980] but it it already depended on it.
What tests are you running?
comment:8 by , 13 years ago
Easy pickings: | unset |
---|---|
Has patch: | set |
Severity: | Release blocker → Normal |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
django.contrib.auth
uses the get_current_site
method of django.contrib.sites
, which is safe to use, whether the sites framework is or isn't enabled.
However, when the sites framework isn't enabled, this method must receive the request
in argument. In test_custom_email_subject
, there is no request available, and the test crashes. Attached patch fixes this (and avoids relying on the default name of the default site).
by , 13 years ago
Attachment: | 16412.diff added |
---|
comment:9 by , 13 years ago
Needs tests: | set |
---|
comment:10 by , 13 years ago
Why does this need tests when the diff fixes a failing test (TDD: "...first the developer writes a failing automated test case..."), and the change is 1 line.
comment:11 by , 13 years ago
Description: | modified (diff) |
---|
While the contrib apps should be optional, they do depend on each other. contrib.auth requires contrib.sites.