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 Ramiro Morales)

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 Dougal Matthews, 13 years ago

Resolution: invalid
Status: newclosed

While the contrib apps should be optional, they do depend on each other. contrib.auth requires contrib.sites.

comment:2 by Matt Harasymczuk, 13 years ago

I am not sure, I have been using contrib.auth without contrib.sites for a few years now.

comment:3 by Dougal Matthews, 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 Matt Harasymczuk, 13 years ago

Resolution: invalid
Status: closedreopened
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'...
Last edited 13 years ago by Ramiro Morales (previous) (diff)

comment:5 by Matt Harasymczuk, 13 years ago

it is fully functional without django.contrib.sites app, no errors, but this one test fails.

comment:6 by Ramiro Morales, 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:7 by Matt Harasymczuk, 13 years ago

python manage.py test in my project root directory

comment:8 by Aymeric Augustin, 13 years ago

Easy pickings: unset
Has patch: set
Severity: Release blockerNormal
Triage Stage: UnreviewedAccepted
Type: BugCleanup/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 Aymeric Augustin, 13 years ago

Attachment: 16412.diff added

comment:9 by Julien Phalip, 13 years ago

Needs tests: set

comment:10 by anonymous coward, 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 Ramiro Morales, 13 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top