assertRedirects

277 views
Skip to first unread message

Jiri Barton

unread,
May 9, 2007, 12:46:00 PM5/9/07
to Django users
Why does assertRedirects of TestCase succeeds only if the response
code is 302? One would think any 3xx should do - they are *redirects*
at all. At least, I would think 301 were okay because that is what
django.views.generic.simple.redirect_to generates.

How am I supposed to think about that? Where is the flaw?

Thank you,
Jiri

Russell Keith-Magee

unread,
May 9, 2007, 7:44:19 PM5/9/07
to django...@googlegroups.com

Omission, not design. The reason it checks 302 is that this is what is
HttpResponseRedirect raises, which is the most common form of redirect
(as it is required by form submission views to operate).

I'll add a configurable argument that allows you to specify alternate
status codes (defaulting to 302). Thanks for the suggestion.

Yours,
Russ Magee %-)

Russell Keith-Magee

unread,
May 10, 2007, 7:28:26 AM5/10/07
to django...@googlegroups.com
On 5/10/07, Russell Keith-Magee <freakb...@gmail.com> wrote:
> On 5/10/07, Jiri Barton <whis...@gmail.com> wrote:
> >
> > Why does assertRedirects of TestCase succeeds only if the response
> > code is 302? One would think any 3xx should do - they are *redirects*
> > at all. At least, I would think 301 were okay because that is what
> > django.views.generic.simple.redirect_to generates.
> >
> I'll add a configurable argument that allows you to specify alternate
> status codes (defaulting to 302). Thanks for the suggestion.

FYI, I added these arguments in [5179].

Yours,
Russ Magee %-)

Jiri Barton

unread,
May 11, 2007, 11:45:32 AM5/11/07
to Django users
Thank you Russ, that has fixed the problem.

I have a problem with the fix though; what if there were multiple
redirects? This just happened when I jumped to use your fix today:

/usermanagement

redirects to

/usermanagement/

which redirects to

/usermanagement/users/

in my application. I don't know how to use assertRedirect in this
scenario:

response = self.client.get('/usermanagement')

(1)
self.assertRedirects(response, '/usermanagement//users/',
status_code=301)

This will print
AssertionError: Response redirected to '/usermanagement/', expected '/
usermanagement/users/'

(2)
(1)
self.assertRedirects(response, '/usermanagement/', status_code=301)

This will print
AssertionError: Couldn't retrieve redirection page '/usermanagement/':
response code was 301 (expected 301)

This calls for some loop in the code of assertRedirects but I don't
know where to place it and how to check the redirect chain has
finished (return code starts with '3'?) - and detect infinte loops
possibly.

I hate to bother you but please explain this to me or, could you fix
it?
Thank you,
Jiri

Russell Keith-Magee

unread,
May 11, 2007, 10:05:44 PM5/11/07
to django...@googlegroups.com
On 5/11/07, Jiri Barton <whis...@gmail.com> wrote:
>
> Thank you Russ, that has fixed the problem.
>
> I have a problem with the fix though; what if there were multiple
> redirects? This just happened when I jumped to use your fix today:
...

> This calls for some loop in the code of assertRedirects but I don't
> know where to place it and how to check the redirect chain has
> finished (return code starts with '3'?) - and detect infinte loops
> possibly.
>
> I hate to bother you but please explain this to me or, could you fix
> it?

I thought about this, and decided that it would be better to require
the user to do multiple checks. Tests should always be atomic, and
although they have the appearance to the user of being a single
action, they are two very distinct actions on the part of the server.

I suppose one option would be to make assertRedirects return the
response from GETting the redirect page; that way you could chain the
tests easily:

response = self.client.get('/page')
response = self.assertRedirects(response, '/first/')
response = self.assertRedirects(response, '/second'/)
...

but I'm not sure how I feel about assertRedirects being the only
assert method that returns a value. Opinions?

Yours,
Russ Magee %-)

Malcolm Tredinnick

unread,
May 12, 2007, 12:31:53 AM5/12/07
to django...@googlegroups.com

I agree (with you). This sort of API feels like you're smashing
unrelated things together. Maybe an explicit response.follow_redirect()
method?

Thinking out loud, this use case suggests you might want some way to
easily check that you've come to the end of a redirect chain. So running
the sorts of tests Jiri is asking about would involve a loop to get to
the end of the chain and then check the location to see that they end up
where they wanted to be.

Except that that isn't possible in the current setup: the ClientHandler
class doesn't have a concept of "current location" -- and I'm not sure
that adding one is the right idea (yet) -- and you can't implement a
method that asks "is the next redirect the last one?" without executing
the redirect.

Hmmm... imaginary code that would work:

while response.is_redirect():
old_response, response = response,
response.follow_redirect()
self.assertRedirect(old_response, '/final_destination/')

It's not winning prizes for beauty, though. This needs more thought.

(Wait! I thought I wasn't going to get involved in the testing
framework? How did I get to this point, posting a reply?!)

Malcolm

Jiri Barton

unread,
May 12, 2007, 8:56:15 AM5/12/07
to Django users
I could live with doing multiple checks. In fact, I may want to check
every URL in the redirecting chain to make sure the flow is just what
I wanted.

Then, I can just hard code the URLs in the chain:

response = self.client.get('/page')

self.assertRedirects(response, '/first/') # no need to return
anything
response = self.client.get('/first/') # asserted by the previous line
self.assertRedirects(response, '/second'/) # no need to return
anything
response = self.client.get('/second/') # asserted by the previous
line

Guys, what do you think about this?

Russ, the first implementation of assertRedirects wasn't that bad
after all... I just missed the status_code argument.

Cheers,
Jiri

Russell Keith-Magee

unread,
May 12, 2007, 10:58:46 AM5/12/07
to django...@googlegroups.com
On 5/12/07, Jiri Barton <whis...@gmail.com> wrote:
>
> I could live with doing multiple checks. In fact, I may want to check
> every URL in the redirecting chain to make sure the flow is just what
> I wanted.

That was my point - the fact that /second redirects to /third is just
as important as the fact that /first redirects to /third; each
redirect should be an atomic check.

> Guys, what do you think about this?

The code you provided is legal and does what it needs to do - I was
just trying to work out if there was a way to make the syntax cleaner
and do a little optimization. After all, each call to assertRedirect
is already doing a get; if there is a way to avoid doing the get
twice, it's probably worth implementing.

Yours,
Russ Magee %-)

Jiri Barton

unread,
May 14, 2007, 12:15:56 PM5/14/07
to Django users
Another thing. I don't know if that's legal or not but I'd also need
to check the query:

self.client.post('/usermanagement/users/create/', {
'firstname': 'Sherlock',
'lastname': 'Holmes',
})

redirects to

/usermanagement/users/filter/?edit=1

(I use some search filters to display only certain group (groups=), or
entries (edit=?))

I don't know if my URL space is okay but it seems assertRedirect could
look like:

def assertRedirects(self, response, expected_path,
status_code=302):
"""Assert that a response redirected to a specific URL, and
that the
redirect URL can be loaded.

"""
self.assertEqual(response.status_code, status_code,
"Response didn't redirect: Reponse code was %d (expected
%d)" %
(response.status_code, status_code))

setattr(response, 'redirected', dict(
zip(('scheme', 'netloc', 'path', 'params', 'query',
'fragment'),
urlparse(response['Location']))))

path = response.redirected['path']
self.assertEqual(path, expected_path,
"Response redirected to '%s', expected '%s'" % (path,
expected_path))

So, I would like to use the urlparse the way Russ uses it in the
implementation. There would be no way for me to check the QUERY part
of the redirected url.

self.assertEqual(response.redirected['query'], 'edit=1')

Or, do you think this is too specific for an interface? Do I have to
parse the response URL by myself?

Jiri

P.S. Should we move this topic to the django-developers group so that
it gets more attention from the right audience?

Russell Keith-Magee

unread,
May 14, 2007, 8:50:46 PM5/14/07
to django...@googlegroups.com
On 5/15/07, Jiri Barton <whis...@gmail.com> wrote:
>
> Or, do you think this is too specific for an interface? Do I have to
> parse the response URL by myself?

Redirecting to a GET with data is a fairly eclectic edge case, and I
can't say I'm a huge fan of an assertion statement modifying the
variables it is asserting.

I'm inclined to put this in the 'not a common use case' basket. If you
need to test this sort of thing, you can always write your own
modified redirect assertion that suits your purposes.

Yours,
Russ Magee %-)

Jiri Barton

unread,
May 15, 2007, 5:45:48 AM5/15/07
to Django users
> Redirecting to a GET with data is a fairly eclectic edge case, and I
> can't say I'm a huge fan of an assertion statement modifying the
> variables it is asserting.

I have but to agree with both statements. Let's leave it the way it
is.

> I'm inclined to put this in the 'not a common use case' basket. If you
> need to test this sort of thing, you can always write your own
> modified redirect assertion that suits your purposes.

I won't do that because I feel now too, my URL pattern space is poor
in this aspect.

Thank you for discussing this!
Jiri

Reply all
Reply to author
Forward
0 new messages