Opened 11 years ago

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

trac#687-test_redirect.diff (781 bytes ) - added by Ben Sturmfels 11 years ago.
Add test for redirect with location.
trac#686-user.url_for_self.diff (1 byte ) - added by Ben Sturmfels 11 years ago.
Add url_for_self function to User.
issue_687.patch (3.3 KB ) - added by Ben Sturmfels 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 by Ben Sturmfels, 11 years ago

Owner: set to Ben Sturmfels
Status: newin_progress

comment:2 by Ben Sturmfels, 11 years ago

Owner: Ben Sturmfels removed
Status: in_progressreview

(irrelevant comment removed - was for wrong ticket)

Last edited 11 years ago by Ben Sturmfels (previous) (diff)

comment:3 by Ben Sturmfels, 11 years ago

Status: reviewaccepted

Whoops! I came back and noticed that I'd adding the attachment and commend to the wrong ticket. Please ignore the above.

by Ben Sturmfels, 11 years ago

Attachment: trac#687-test_redirect.diff added

Add test for redirect with location.

by Ben Sturmfels, 11 years ago

Add url_for_self function to User.

comment:4 by Ben Sturmfels, 11 years ago

Status: acceptedreview

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 Ben Sturmfels, 11 years ago

Cc: ben@… added

comment:6 by rodney757, 11 years ago

Status: reviewaccepted

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 ShawnRisk, 10 years ago

Keywords: test added

comment:8 by Ben Sturmfels, 10 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 Elrond, 10 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 Christopher Allan Webber, 10 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 Ben Sturmfels, 9 years ago

Status: acceptedreview

I've attached a patch to test redirect and redirect_obj.

by Ben Sturmfels, 9 years ago

Attachment: issue_687.patch added

comment:12 by Christopher Allan Webber, 8 years ago

Milestone: 0.9.0
Resolution: fixed
Status: reviewclosed

These tests look great, Sturm! Merged and pushed!

Note: See TracTickets for help on using tickets.