Code

Opened 6 years ago

Closed 2 years ago

#8797 closed Bug (worksforme)

django password reset tests assume hardcoded urls

Reported by: teh Owned by: SmileyChris
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

Description

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

response = self.client.post('/password_reset/', {'email': 'not_a_real_email@email.com'})

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

Attachments (1)

8797.diff (4.0 KB) - added by SmileyChris 5 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by jacob

  • milestone 1.0 deleted
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 5 years ago by anonymous

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

reverse('django-password-reset') 

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

comment:3 Changed 5 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

comment:4 Changed 5 years ago by SmileyChris

  • Triage Stage changed from Design decision needed to Accepted

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 5 years ago by SmileyChris

(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 5 years ago by SmileyChris

comment:6 Changed 5 years ago by SmileyChris

  • Triage Stage changed from Accepted to Design 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 5 years ago by SmileyChris

New ticket opened as #10818

comment:8 Changed 3 years ago by SmileyChris

  • Easy pickings unset
  • Owner changed from nobody to SmileyChris
  • Severity set to Normal
  • Type set to 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 3 years ago by mlavin

  • 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 2 years ago by aaugustin

  • Resolution set to worksforme
  • Status changed from new to closed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.