Opened 8 years ago

Closed 8 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

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, 8 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

The message could suggest using the fetch_redirect_response=False argument.

comment:2 by Tobias McNulty, 8 years ago

Owner: changed from nobody to Tobias McNulty
Status: newassigned

comment:3 by Tobias McNulty, 8 years ago

Has patch: set

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

Resolution: fixed
Status: assignedclosed

In c7b1b81:

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

comment:5 by Tim Graham, 8 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@…>, 8 years ago

In 78a0ca6:

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

comment:7 by Tobias McNulty, 8 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, 8 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, 8 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@…>, 8 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, 8 years ago

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