How am I supposed to think about that? Where is the flaw?
Thank you,
Jiri
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 %-)
FYI, I added these arguments in [5179].
Yours,
Russ Magee %-)
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
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 %-)
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
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
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 %-)
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?
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 %-)
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