Opened 9 years ago

Closed 9 years ago

#26666 closed Cleanup/optimization (fixed)

assertRedirects gives unhelpful error message expected URL is external

Reported by: Peter Inglesby Owned by: Tobias McNulty
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no
Pull Requests:6702 unmerged, 6701 merged, 6696 merged, 6713 merged

Description

assertRedirects uses the test client and so cannot fetch external URLs. When the expected URL is set to an external URL, assertRedirects fails with something like:

AssertionError: 404 != 200 : Couldn't retrieve redirection page '/some-path/': response code was 404 (expected 200)

It would be better if assertRedirects could detect that it was being used incorrectly, and gave a more helpful error message.

Change History (11)

comment:1 by Tim Graham, 9 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

The message could suggest using the fetch_redirect_response=False argument.

comment:2 by Tobias McNulty, 9 years ago

Owner: changed from nobody to Tobias McNulty
Status: newassigned

comment:3 by Tobias McNulty, 9 years ago

Has patch: set

comment:4 by Markus Holtermann <info@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In c7b1b81:

Fixed #26666 -- Added more useful error message to assertRedirects

comment:5 by Tim Graham, 9 years ago

Has patch: unset
Resolution: fixed
Status: closednew
Version: 1.9master

Tobias is working on fixing the test failures that the patched introduced.

comment:6 by Markus Holtermann <info@…>, 9 years ago

In 78a0ca6:

Refs #26666 -- Fixed test failures caused by assertRedirects changes (#6701)

comment:7 by Tobias McNulty, 9 years ago

  • The original fix to this ticket (https://github.com/django/django/pull/6696) broke unit testing of custom hostnames in non-core Django projects when the hostname was passed as part of the path to client.get()
  • A new proposed change (https://github.com/django/django/pull/6713) leverages ALLOWED_HOSTS to facilitate support for custom domain testing in 3rd party packages while increasing the rigor which with we validate ALLOWED_HOSTS (now also in tests)

comment:8 by Simon Charette, 9 years ago

Has patch: set

The code part LGTM but the doc could benefit from a second look of a native speaker.

comment:9 by Philip James, 9 years ago

As a native speaker I took a look over the docs, and they make a lot of sense minus one potentially confusing bit. I left a comment on the PR.

comment:10 by Tim Graham <timograham@…>, 9 years ago

In 17e66164:

Refs #26666 -- Added ALLOWED_HOSTS validation when running tests.

Also used ALLOWED_HOSTS to check for external hosts in assertRedirects().

comment:11 by Tim Graham, 9 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top