[PATCH] Use WebTest for unit tests

Here is my first attempt at adding WebTest unit testing support to TG1.0.

This patch currently leaves testutil.{call,create_request} alone, and simply adds new TGWebTest and DBWebTest classes. Whether or not we want to port create_request/call to use WebTest, or deprecate them, is still up for discussion. I just want to touch base with my current implementation before I dig deeper into it and start porting it up to 1.1 and 2.0.

The TGWebTest class fires automatically sets up turbogears and hooks up WebTest to the CherryPy WSGI server. 'self.app' can then be used by the unit tester to interact with WebTest. The DBWebTest class does the same thing, while also setting up and tearing down the database as well.

My patch also adds SQLAlchemy support to the testutil.DBTest class. It seems to work fine in all of my testing, but could definitely use some more eyes on it :)

The attach patch allows for the following types of unit tests:

class TestPages(testutil.DBWebTest):

    def test_forbidden(self):
        # Hot action is forbidden
        res = self.app.get('/hot_action', status=403)

    def test_webob_response(self):
        user = User(user_name=u"test", password=u"test")
        self.login_user(user)
        res = self.app.get('/hot_action')
        assert "Hot WSGI action" in res

    def test_response_namespace(self):
        user = User(user_name=u"test", password=u"test")
        self.login_user(user)
        res = self.app.get('/hot_action')
        assert res.namespace['tg_flash'] == u'Hot WSGI action'

Questions, Comments, Suggestions ?

I pulled the testutil.DBTest SQLAlchemy integration out into it's own patch and added it to ticket 1764.

Ok, so the above patch is almost complete, but still has some issues, the main problem being:

• There are a few places, mainly in the identity provider, where we need to deal with things like cherrypy.request.identityProvider. Trying to munge this ourselves to work around it yields AttributeError?: cherrypy.request has no properties outside of an HTTP request..

Any suggestions?

Updated again with some minor cleanups

Here is another issue,

======================================================================
FAIL: flash redirect with chars that can cause troubles in cookies
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/lmacken/tg1.1/turbogears/turbogears/tests/test_controllers.py", line 475, in test_flash_redirect_with_trouble_chars
    assert u'k\xe4se'.encode('utf-8') in value
AssertionError

The orignal test case yields something like this

cherrypy.response.simple_cookie["tg_flash"].value = '%24foo%2C%20k\xc3\xa4se%3B%09bar!'

When porting it to WebTest? and dealing with a WebOb? response, I'm getting this:

response.cookies_set['tg_flash'] = '"%24foo%2C%20k\\303\\244se%3B%09bar!"'

A few things:

• I'm working on cleaning up the patch I submitted against #1466, which does essentially the same things as Luke's patch here. I'm almost done & will post an update to this ticket once I've done a final sanity check, compared it more fully to Luke's approach, & written a test migration guide.

• There were some test isolation issues where one test module was relying on a request being created as a side-effect of another test module. Since requests are (properly) short-lived in a webtest world, these dependencies needed to be fixed prior to the migration. These were fixed in r4584, r4595, and r4596.

• If we end up going with Luke's patch, test_catwalk needs another look. The svn:executable property was set on that module when Luke did his work; since nose skips executables by default, these tests weren't getting run & have some problems yet. (I removed the executable bit in r4583).

• It looks like Luke's latest patch fixes the identity issues he mentioned above. As far as the cookie encoding issue, I think that test is overly specific about how the encoding should happen & is too fragile. The important thing to test is that the "trouble chars" (ie, unicode) can be encoded & decoded & returns the proper result, not that the intermediate result is a particular value. You could probably get it to work using util.quote_cookie & unquote_cookie, but you'd still have a fragile test that has a lot of extra steps without testing anything new.

• We should formally deprecate create_request & createRequest. I added a decorator for this purpose in r4306 (& fixed in r4585).

Anyway, I haven't reviewed Luke's patch too closely yet, but it looks pretty good at first glance in as far as it goes. I think we should add some scope, though, to get rid of other dependencies on CP2 internals. I'm not going to go into detail on that here, since it overlaps what I need to do for my first point above.

Ken's attempt

Attached my attempt at this ticket, and created a "Work In Progress" page at http://docs.turbogears.org/1.1/TestMigration

I'm going to review Luke's version more closely now & see if I can fix the handful of regressions I noticed when testing with it earlier. After that, I'll look into whether a hybrid approach is possible/useful (it should be.. I already have a short list of things I like/don't like from each approach).

Minor tweaks to fix regressions in Luke's version

That was easy. Basically just changed response to response.body for the json loads in test_catwalk, moved the remaining request-based tests in test_identity inside the controllers, and made the cookie-encoding test change I suggested earlier.

"webtest-luke-tweaked.patch" is just Luke's patch with these adjustments.

I'm very happy with the way the merger turned out. The result is cleaner than my original version & more complete than Luke's.

I've updated the TestMigration page that I created earlier, and am about to attach a new patch & a merger report.

There's one minor wart: in widgets/tests/test_request_related_features.py, there's one test that requires the server to be stopped & restarted. The problem is the validate decorator's mangling of parameters is buggy. I think this has just been hidden by all the starting & stopping of the server that's been happening in the tests, so I've just marked this with a FIXME.

If no-one objects, I'm going to commit this in the next couple of days, after I review the changes one more time & do a "final" cleanup of the docs.

A report of which changes were kept from the two approaches.

Patch including a blending of Luke & Ken's approaches.

Nice work, Ken! I'm very pleased with the results as well. The patch looks to be like best of both worlds, gives us a clean consistent API, and shaves about 32 seconds off of the TurboGears test suite. Looks good to me :)

Applied in r4694 & r4695. Thanks for all the work, Luke!

When I merged Luke & my approaches, I failed to update the quickstart templates, so the tests for newly created projects are erroring.

Also, chrisz has suggested that TGWebTest have a root attribute, which setUp would use. That way in the common case it wouldn't be necessary to override setUp & tearDown. I'll fix that while I'm at it.

Finally, I plan on renaming TGWebTest to just TGTest, since the shorter name is more memorable.

Re-opening ticket and raising priority.

Almost complete..

The patch I just attached is almost complete. Identity-enabled quickstarted apps using SQLObject pass tests, but SQLAlchemy & Elixir apps error with "Class '<class 'turbogears.visit.savisit.TG_Visit'>' already has a primary mapper defined"

I'll look into the remaining issues later tonight. We need this fixed by 1.1beta-1.

Confirmed that the SA mapper problem existed before this patch, so going ahead and committed it in r5276 & closing this ticket. Created a new ticket, #1969, for the remaining problem.