Opened 10 years ago

Closed 9 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@… 10 years ago.
Diff of django/http/init.py
docs_request_response.diff (912 bytes) - added by egmanoj@… 10 years ago.
Documentation for the suggested code change for HttpResponseRedirect and HttpResponsePermanentRedirect.

Download all attachments as: .zip

Change History (9)

Changed 10 years ago by egmanoj@…

Attachment: HttpResponseRedirect.diff added

Diff of django/http/init.py

comment:1 Changed 10 years ago by Chris Beaven

Needs documentation: set
Triage Stage: UnreviewedAccepted

Looks good, could probably do with some docs

comment:2 in reply to:  1 Changed 10 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 10 years ago by egmanoj@…

Attachment: docs_request_response.diff added

Documentation for the suggested code change for HttpResponseRedirect and HttpResponsePermanentRedirect.

comment:3 Changed 10 years ago by anonymous

Needs documentation: unset

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

Cc: egmanoj@… added

comment:5 Changed 9 years ago by Malcolm Tredinnick

Resolution: wontfix
Status: newclosed

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 9 years ago by Manoj Govindan <egmanoj@…>

Resolution: wontfix
Status: closedreopened

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 9 years ago by Malcolm Tredinnick

Resolution: wontfix
Status: reopenedclosed

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