Comment 3 for bug 637674

On Tue, Sep 14, 2010 at 7:27 PM, Andrew Bennetts
<email address hidden> wrote:
> Robert Collins wrote:
>> > bzr at one point has this code:
>> >
>> >        # Only check for thread leaks if the test case supports cleanups
>> >        addCleanup = getattr(test, "addCleanup", None)
>> >        if addCleanup is not None:
>> >            addCleanup(self._check_leaked_threads, test)
>>
>> Thats buggy code though.
>
> I'm not surprised.  Care to elaborate though?  (I think I have a fair idea
> why, but it'd prefer not to guess.)

TestCase offers two contracts:
 - the runner contract (run, debug, id, description, countTests)
 - the testcase contract (only for use by a subclass on self)

The code that bzr has is not in the runner contact, and by definition
not running under the testcase contract.

So its being fundamentally naughty.

> Regardless, RemotedTestCase inheriting from TestCase without up-calling
> seems pretty fragile, and likely to be a source of trouble as the standard
> unittest.TestCase implementation keeps evolving.

This is the first issue seen, subclassing matters to some clients
(also buggy) and its not wrong to not upcall when you don't -call at
all-.

The bzr code path shown will break with other third party
implementations as well - it has nothing to do with upcalling per se.