Opened 9 years ago

Closed 6 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.

Subtickets

Attachments (3)

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

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by Ben Sturmfels

Owner: set to Ben Sturmfels
Status: newin_progress

comment:2 Changed 8 years ago by Ben Sturmfels

Owner: Ben Sturmfels deleted
Status: in_progressreview

(irrelevant comment removed - was for wrong ticket)

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

comment:3 Changed 8 years ago by Ben Sturmfels

Status: reviewaccepted

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

Changed 8 years ago by Ben Sturmfels

Attachment: trac#687-test_redirect.diff added

Add test for redirect with location.

Changed 8 years ago by Ben Sturmfels

Add url_for_self function to User.

comment:4 Changed 8 years ago by Ben Sturmfels

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 Changed 8 years ago by Ben Sturmfels

Cc: ben@… added

comment:6 Changed 8 years ago by rodney757

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 Changed 8 years ago by ShawnRisk

Keywords: test added

comment:8 Changed 7 years ago by Ben Sturmfels

I'd love some pointers on setting up the second test here. We need to get urlgen attached to the request object.

comment:9 Changed 7 years ago by Elrond

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 Changed 7 years ago by Christopher Allan Webber

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 Changed 6 years ago by Ben Sturmfels

Status: acceptedreview

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

Changed 6 years ago by Ben Sturmfels

Attachment: issue_687.patch added

comment:12 Changed 6 years ago by Christopher Allan Webber

Milestone: 0.9.0
Resolution: fixed
Status: reviewclosed

These tests look great, Sturm! Merged and pushed!

Note: See TracTickets for help on using tickets.