[TIP] Unittest Changes

[Andrew Bennetts]

Michael Foord wrote:
[...]
> Right - although virtually every project I've worked on with unittest 
> had ended up with an ad-hoc test collection mechanism, so I think the 
> 'discover_tests' features I've discussed are prima-facie needed.
> 
> It looks like Twisted and Bzr both have prior-art in these areas.

I also think a test discovery mechanism should be part of the included
batteries.  It's a big barrier to people using unittest, because every new user
of unittest bumps into this very shortly after they start using unittest.

[...]
> I've never had a need for per-module / per-class cleanup methods myself. 
> I tend to prefer creating needed resources by decorating test-methods, 
> even if this means creating them 'per-test'. As such I'm not so 
> convinced about 'addCleanup' etc.

addCleanup isn't related to per-module or per-class cleanup, and I think it's
more elegant than decorating test-methods.  I'll quickly try to explain why,
although perhaps Jono can explain better.

Imagine a test like:

    def test_foo(self):
        branch = self.make_branch('source')
        branch.lock_write()
        ...do stuff...
        ...assert stuff...
        branch.unlock()

This is a fairly common sort of thing to see in the bzrlib test suite, but it's
applicable to any kind of resource you might acquire.  An immediate problem is
that if the test fails, the unlock never happens.  You can wrap most of the test
in a try-finally, but that's ugly.  Now imagine that you acquire multiple
resources (in bzrlib we often need to write tests involving multiple branches),
or similarly global state changes (like modifying os.environ).

If you use try-finally, you get lots of ugly nesting.

If you use decorators, you get a large stack of:

    @cleans_environ
    @needs_locked_branch('source')
    @needs_locked_branch('target')
    def test_foo(self):
        ...

Plus you need to write a decorator first! (so you'll probably write a decorator
factory for convenience!).  And when you do refactoring, decorators like that
are hard to move between test methods and the setUp method (with the obvious
implementations anyway, unless you build them on addCleanup... I'd be interested
to see what your decorators look like so we can compare).

If you have addCleanup, then it's easy and requires no special decorators or
other infrastructure to be defined:

    def test_foo(self):
        branch = self.make_branch('source')
        branch.lock_write()
        self.addCleanup(branch.unlock)
        ...

It's trivial to move that logic into (or out of) a setUp method, or even into
the creation method itself:

    def make_locked_branch(self, name):
        branch = self.make_branch(name)
        branch.lock_write()
        self.addCleanup(branch.unlock)
        return branch

    def test_foo(self):
        branch = self.make_locked_branch('source')
        ...

This is pretty close to perfect: the test method has a single line devoted to
constructing that object, and the noise about how cleaning it up is confined to
a simple test helper.

The fact that addCleanup works in setUp, even if the setUp raises an exception
half-way through, is also very handy.  You basically don't need to think about
it: if you've called addCleanup, then it will be cleaned-up.

Probably the best argument though is that they've proved immensely simple and
useful to use in practice in bzrlib's tests. :)

-Andrew.