Opened 7 years ago

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


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 Changed 7 years ago by Tim Graham

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

The message could suggest using the fetch_redirect_response=False argument.

comment:2 Changed 7 years ago by Tobias McNulty

Owner: changed from nobody to Tobias McNulty
Status: newassigned

comment:3 Changed 7 years ago by Tobias McNulty

Has patch: set

comment:4 Changed 7 years ago by Markus Holtermann <info@…>

Resolution: fixed
Status: assignedclosed

In c7b1b81:

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

comment:5 Changed 7 years ago by Tim Graham

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

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

comment:6 Changed 7 years ago by Markus Holtermann <info@…>

In 78a0ca6:

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

comment:7 Changed 7 years ago by Tobias McNulty

  • The original fix to this ticket ( 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 ( 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 Changed 7 years ago by Simon Charette

Has patch: set

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

comment:9 Changed 7 years ago by Philip James

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 Changed 7 years ago by Tim Graham <timograham@…>

In 17e66164:

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

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

comment:11 Changed 7 years ago by Tim Graham

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