Opened 13 years ago
Closed 10 years ago
#687 closed enhancement (fixed)
Write unit tests for redirect and redirect_obj
| Reported by: | Elrond | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | 0.9.0 |
| Component: | programming | Keywords: | bitesized, test |
| Cc: | ben@… | Parent Tickets: |
Description
Well, the title says it all. Those methods could use some unit tests.
For redirect two cases should be tested: with location= given, and without.
redirect_obj has big magic.
Attachments (3)
Change History (15)
comment:1 by , 12 years ago
| Owner: | set to |
|---|---|
| Status: | new → in_progress |
comment:2 by , 12 years ago
| Owner: | removed |
|---|---|
| Status: | in_progress → review |
comment:3 by , 12 years ago
| Status: | review → accepted |
|---|
Whoops! I came back and noticed that I'd adding the attachment and commend to the wrong ticket. Please ignore the above.
by , 12 years ago
| Attachment: | trac#687-test_redirect.diff added |
|---|
Add test for redirect with location.
by , 12 years ago
| Attachment: | trac#686-user.url_for_self.diff added |
|---|
Add url_for_self function to User.
comment:4 by , 12 years ago
| Status: | accepted → review |
|---|
I've blanked out the irrelevant attachment with an empty file (trac#686-user.url_for_self.diff) and have attached one initial test for request in trac#687-test_redirect.diff. This is the easy test out of the ones requested above. Feedback appreciated since I haven't written py.test tests before.
The other test requested are a bit more complicated, partly I think because of this urlgen stuff that's generated on the fly. This is where the magic is, not in redirect_obj. Hmm. Not quite sure where to go there.
comment:5 by , 12 years ago
| Cc: | added |
|---|
comment:6 by , 12 years ago
| Status: | review → accepted |
|---|
Okay, so for this test, I don't think that we need to mock anything. You can just set res = redirect(...) and then assert res.location == ....
As for testing redirect without location kwarg, we need to get urlgen attached to the request object and I'm not sure of the best way to do that at the moment. Maybe Elrond or cwebb knows? I'll get back to you.
The other test should be fairly simple once we get urlgen attached to the request object. Just get a user object, pass it into redirect_obj and check that res.location is the users homepage.
Thanks for starting this.
comment:7 by , 12 years ago
| Keywords: | test added |
|---|
comment:8 by , 11 years ago
I'd love some pointers on setting up the second test here. We need to get urlgen attached to the request object.
comment:9 by , 11 years ago
For testing redirect_obj, testing this with User is one good thing.
Maybe we should test this with all supported objects?
But I would encourage to also have a test with a fake object, that has an url_for_self method. Just let it call urlgen with some simple args, or something made up. I don't know yet.
comment:10 by , 11 years ago
tl;dr: There's no nice way at present, might become easier once we mandate having a host configuration in the config, but in the meanwhile, you could fake the request.
We don't really have a nice way to create nice "context"/request objects without making a real request. It's something I've been thinking of putting together... I have a prototype of this inn another application. But in this case, it's kind of important to have a request object because we don't know what the domain is unless it's coming in off of a request. This may change soon... we might *require* setting a host/domain in the config (a pretty big change) due to changes necessary for federation stuff.
In the meanwhile, one hack would be to create a werkzeug request manually (see app.py for how this is done), only attach what is needed for the method to complete, and then pass it into the relevant method.
I could probably walk through the relevant parts of app.py if needed on IRC.
comment:11 by , 10 years ago
| Status: | accepted → review |
|---|
I've attached a patch to test redirect and redirect_obj.
by , 10 years ago
| Attachment: | issue_687.patch added |
|---|
comment:12 by , 10 years ago
| Milestone: | → 0.9.0 |
|---|---|
| Resolution: | → fixed |
| Status: | review → closed |
These tests look great, Sturm! Merged and pushed!

(irrelevant comment removed - was for wrong ticket)