Opened 18 years ago
Closed 18 years ago
#5538 closed (fixed)
assertRedirects should assume http://testserver if it isn't provided
| Reported by: | Russell Keith-Magee | Owned by: | anonymous |
|---|---|---|---|
| Component: | Testing framework | Version: | dev |
| Severity: | Keywords: | assertRedirects testserver | |
| Cc: | Triage Stage: | Design decision needed | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
[6164] fixed #987, a problem with redirects allowing relative URIs. The fix is correct, but it makes the testing API slightly unwieldy; all calls to assertRedirects() now require that 'http://testserver' be part of the asserted URL.
The vast majority of redirection tests will be internal to the site, so requiring the full dummy host prefix only serves to muddy the tests. This is made worse by the fact that urlresolvers.reverse() doesn't include the testserver component, so assertRedirects(reverse('some_name')) doesn't work.
assertRedirects should be modified to inspect the test URL; if no ':' doesn't occur in the test url, 'http://testserver' should be prepended for test purposes. i.e.,
assertRedirects('/some/url') is equivalent to assertRedirects('http://testserver/some/url')
assertRedirects('http://otherserver/some/url') would be untouched
Attachments (1)
Change History (6)
comment:1 by , 18 years ago
| Description: | modified (diff) |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 18 years ago
| Triage Stage: | Accepted → Design decision needed |
|---|
comment:3 by , 18 years ago
comment:4 by , 18 years ago
| Has patch: | set |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
I attached a solution to only match parts of the url that were defined in the expected_url. It fixes our tests but I'm not sure if its good/clear/readable/up to django standards.
comment:5 by , 18 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Uh, I don't think I changed the requirement on assertRedirect tests, since the test server doesn't set
HTTP_HOSTorSERVER_NAMEsee http://code.djangoproject.com/changeset/6164#file4