Django

Code

Ticket #4476 (assigned)

Opened 1 year ago

Last modified 1 month ago

Modify test client to follow redirect chains

Reported by: Chris Moffitt <chris.moffitt@gmail.com> Assigned to: telenieko (accepted)
Component: Unit test system Version: SVN
Keywords: Cc: kbussell@gmail.com
Triage Stage: Accepted Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 0

Description (Last modified by russellm)

When using assertRedirects, I find myself doing an assert redirects, then wanting to validate the text where I'm redirecting. The process seems a little clunky and an assertRedirectContains would be useful.

Current process: 

self.assertRedirects(response, '/confirm/', status_code=302, target_status_code=200)
response = self.client.get('/confirm/')
self.assertContains(response, "Total = $54.50", count=1, status_code=200)

It would be nice to have something like: 

self.assertRedirectContains(response, "Total = $54.50", count=1, status_code=302, target_status_code=200)

I'm assuming there's no other shortcut I'm missing.

*** UPDATE (from russellm) ***

Rather that assertRedirectContains, I'm favouring a modification to the test client that will follow redirects. Details in the comment below

Attachments

4476.diff (5.5 kB) - added by telenieko on 12/01/07 17:28:30.
Patch with docs+tests
4476_follow_redirects.diff (14.3 kB) - added by keithb on 12/01/07 20:51:14.
Another version of the fix, since I already did the work
4476_rev7412.diff (5.6 kB) - added by telenieko on 04/10/08 15:12:12.
Updated patch, used my version for being shorter ;))

Change History

07/23/07 16:14:16 changed by John Shaffer <jshaffer2112@gmail.com>

  • owner changed from jacob to adrian.
  • needs_better_patch changed.
  • component changed from Uncategorized to Unit test system.
  • needs_tests changed.
  • needs_docs changed.

07/23/07 20:26:58 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Design decision needed.

(follow-up: ↓ 7 ) 09/08/07 00:24:38 changed by russellm

  • summary changed from Proposal for assertRedirectContains to Modify test client to follow redirect chains.
  • description changed.
  • stage changed from Design decision needed to Accepted.

I'm not a fan of assertRedirectContains, because it sounds like start of aggregating groups of tests together. It works for a single redirect, but then you have a page that redirects to a redirect, so you need to add 'assertRedirectRedirectContains', and then the madness starts :-). So, -1 to the literal idea.

However, I'm going to mark the ticket is as accepted and change the summary, because the general problem is valid.

Rather than assertRedirectContains, I'm going to suggest that the test Client should be modified to do what a normal web client will do - follow redirect trails. So, if you had a site with:

/first -> redirects to /second /second -> redirects to /final /final -> a page of content

response = client.get('/first', follow=True)

you would get back the content at '/final', with a response code of 200. The response should also have some extra meta-data to indicate the paths that were visited along the way. There may also be a need to modify the assertions to allow for checking that a redirect occurred; e.g.,

assertContains(response, "content of /final", count=1, status_code=200, redirected=True)

However, I'll leave the details of any assertion changes as a detail for the implementer.

09/15/07 16:38:35 changed by frasern

  • owner changed from nobody to frasern.
  • status changed from new to assigned.

11/11/07 14:10:08 changed by frasern

  • owner changed from frasern to nobody.
  • status changed from assigned to new.

12/01/07 15:58:27 changed by keithb

  • owner changed from nobody to keithb.

(in reply to: ↑ 3 ) 12/01/07 16:27:54 changed by telenieko

Replying to russellm:

you would get back the content at '/final', with a response code of 200. The response should also have some extra meta-data to indicate the paths that were visited along the way. There may also be a need to modify the assertions to allow for checking that a redirect occurred; e.g.,

What about returning an array of the different response objects?

responses = client.get(..., follow=True)

If follow=True, then always return an array with all the responses got following the redirects.

12/01/07 17:28:30 changed by telenieko

  • attachment 4476.diff added.

Patch with docs+tests

12/01/07 17:30:27 changed by telenieko

  • owner changed from keithb to telenieko.
  • status changed from new to assigned.
  • has_patch set to 1.

Attached patch with docs, tests and the code itself. It works for .post() and .get()

Hope it's fine for all of you ;)

12/01/07 17:33:03 changed by telenieko

keithb, sorry I just noticed you assigned the ticket to yourself. Hope I didn't make you waste too much time! :)

12/01/07 20:51:14 changed by keithb

  • attachment 4476_follow_redirects.diff added.

Another version of the fix, since I already did the work

12/01/07 20:58:14 changed by keithb

No worries. Since I already did the work for the fix, and my solution is slightly different, I figured I'd post it anyway.

The main differences in my version: - instead of returning multiple responses, a redirect_chain attribute is added to the response which holds the intermediate paths and status codes. - follow is True by default - the redirect handling prevents against loops in the chain. - assertRedirects takes an extra parameter if you want to check intermediate redirect steps, but defaults to checking the last

12/01/07 20:59:46 changed by keithb

  • cc set to kbussell@gmail.com.

04/10/08 15:12:12 changed by telenieko

  • attachment 4476_rev7412.diff added.

Updated patch, used my version for being shorter ;))

04/10/08 15:16:39 changed by telenieko

Just attached an updated patch, I took two response codes from keithb patch.

Add/Change #4476 (Modify test client to follow redirect chains)




Change Properties
Action