Write unit tests for redirect and redirect_obj

[Elrond]

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.

[Ben Sturmfels]

I've attached a patch to add the User.url_for_self function. This is my first patch to MediaGoblin, so I'd appreciate some feedback.

I'm a bit unsure about the urlgen function. I can see that it's a reference to the build_proxy function of a Request in mediagoblin.app.call_backend, but it's not clear to me what it actually is. build_proxy could do with some docs in this regard.

I'm also not sure whether it's useful to allow **kwargs on url_to_self - I'm doing this because this is how it's done in mixin.MediaEntryMixin.url_for_self.

[Ben Sturmfels]

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

[Ben Sturmfels]

Add test for redirect with location.

[Ben Sturmfels]

Add url_for_self function to User.

[Ben Sturmfels]

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.

[rodney757]

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.

[Ben Sturmfels]

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

[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.

[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.

[Ben Sturmfels]

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

[Christopher Allan Webber]

These tests look great, Sturm! Merged and pushed!