Opened 8 years ago

Closed 8 years ago

#3499 closed (wontfix)

Add a "redirect_to" attribute (similar to status_code) to HttpResponseRedirect and HttpResponsePermanentRedirect to help with testing

Reported by: egmanoj@… Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords: HttpResponseRedirect, redirect_to
Cc: egmanoj@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Consider this test:

response = self.client.post(url, params)

self.failUnlessEqual(response.status_code, 302)
self.failUnlessEqual(response.headers['Location'], some_other_url)

It would be easier to do the following instead, treating redirect_to as an attribute.

self.failUnlessEqual(response.redirect_to, some_other_url)

Attachments (2)

HttpResponseRedirect.diff (722 bytes) - added by egmanoj@… 8 years ago.
Diff of django/http/init.py
docs_request_response.diff (912 bytes) - added by egmanoj@… 8 years ago.
Documentation for the suggested code change for HttpResponseRedirect and HttpResponsePermanentRedirect.

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by egmanoj@…

Diff of django/http/init.py

comment:1 follow-up: Changed 8 years ago by SmileyChris

  • Needs documentation set
  • Triage Stage changed from Unreviewed to Accepted

Looks good, could probably do with some docs

comment:2 in reply to: ↑ 1 Changed 8 years ago by egmanoj@…

Replying to SmileyChris:

Looks good, could probably do with some docs


I am not sure how to go about adding the necessary documentation.
Any guidance would be appreciated.

Thanks,
Manoj

Changed 8 years ago by egmanoj@…

Documentation for the suggested code change for HttpResponseRedirect and HttpResponsePermanentRedirect.

comment:3 Changed 8 years ago by anonymous

  • Needs documentation unset

comment:4 Changed 8 years ago by egmanoj@…>

  • Cc egmanoj@… added

comment:5 Changed 8 years ago by mtredinnick

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

I don't think this is worth it. Firstly, it's a core change just for tests and for something you can already do. Secondly, you would still need to test that it's some kind of redirect, because redirect_to will only be on certain types of responses.

Finally, assertRedirect() seems to already provide a handy helper for all this.

comment:6 Changed 8 years ago by Manoj Govindan <egmanoj@…>

  • Resolution wontfix deleted
  • Status changed from closed to reopened

assertRedirect() requires tests to be derived from django.test.TestCase, something that slows down test execution considerably. There is also the known problem with assertRedirect() that messes up tests.

comment:7 Changed 8 years ago by mtredinnick

  • Resolution set to wontfix
  • Status changed from reopened to closed

Those are things that can be fixed in other ways and should be directed at the appropriate tickets and components. Adding a new attribute to a core data structure that is only used by the test suite and is to work around limitations in the test framework (#5538 isn't even a limitation; it's just asking for a shortcut) is poor design. Plus you can already accomplish what you need here.

Please don't reopen. The goal is always to fix the root problems, not add workarounds.

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