Opened 10 years ago

Closed 7 years ago

#8797 closed Bug (worksforme)

django password reset tests assume hardcoded urls

Reported by: teh Owned by: Chris Beaven
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


in django/contrib/auth/tests/ there are lines like e.g.:

response ='/password_reset/', {'email': ''})

When one associates the password views with different urls in these tests will fail because of the hardcoded '/password_reset/'.

Attachments (1)

8797.diff (4.0 KB) - added by Chris Beaven 9 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 years ago by Jacob

milestone: 1.0

Not a 1.0 bug.

I'm actually not sure if it's a bug at all -- we can't anticipate every possible test environment in the test suite -- but I suspect in this particular case we should be doing something different.

comment:2 Changed 10 years ago by anonymous

We could use a reverse lookup to check which URL to use for testing


If the user hasn't named his URL then we fall back to the normal '/password_reset/'

comment:3 Changed 10 years ago by Jacob

Triage Stage: UnreviewedDesign decision needed

comment:4 Changed 10 years ago by Chris Beaven

Triage Stage: Design decision neededAccepted

It's a valid bug, and one which has actually irked me for quite a while.

The problem is due to the now loudly failing {% url %} tag. If you override a registration template with one which uses a URL tag, you'll end up with your project tests failing loudly for tde contrib.auth tests.

I think the solution would be to put default templates in the contrib/auth/tests/template dir (I'm assuming this gets used somehow but didn't see the code which activates this).

comment:5 Changed 10 years ago by Chris Beaven

(ah, I see now that it does it specifically for views.ChangePasswordTests. So, would also need to abstract that code to work with all the contrib tests.

Changed 9 years ago by Chris Beaven

Attachment: 8797.diff added

comment:6 Changed 9 years ago by Chris Beaven

Triage Stage: AcceptedDesign decision needed

Hrm... looking at the original issue, I realize that I have hijacked this ticket with a separate issue (test failures due to overridden templates). Ignore the attachment, I'm putting this back to design decision where I found it.

comment:7 Changed 9 years ago by Chris Beaven

New ticket opened as #10818

comment:8 Changed 7 years ago by Chris Beaven

Easy pickings: unset
Owner: changed from nobody to Chris Beaven
Severity: Normal
Type: Bug

Assigning to myself to have a look at whether this is still an issue (but if someone wants to jump in and confirm it before I get the chance to do that, feel free) :)

comment:9 Changed 7 years ago by Mark Lavin

UI/UX: unset

While the urls are hard coded the testcase, the testcase also defines the urls in AuthViewsTestCase. I'm not convinced there is (still) a bug here.

comment:10 Changed 7 years ago by Aymeric Augustin

Resolution: worksforme
Status: newclosed

I agree with mlavin — since the test cases set urls = '...', having hardcoded URLs is fine. I just checked that all auth tests do.

It seems that the original report was based on code inspection, not on an actual problem — there is not traceback or test run output.

Note: See TracTickets for help on using tickets.
Back to Top