[TIP] Test discovery for unittest

Robert Collins robertc at robertcollins.net
Mon Apr 6 04:27:12 PDT 2009


On Mon, 2009-04-06 at 11:52 +0100, Michael Foord wrote:
> Robert Collins wrote:
> Hmm... in our framework we have a custom TestResult, but also a 
> TestRunner that overrides _makeResult.
> 
> Our call to actually run tests looks like:
> 
>     PublishingTestRunner(verbosity=2).run(suite)
> 
> This pushes results (and tracebacks) into the database as the tests run. 
> If we want the TextTestRunner to use our custom result objects then as 
> far as I can tell we have to do this?

With the current TextTestRunner yes you do; what I mean to say is that
TextTestRunner currently does things that are IMO properly the
responsibility of the Result object.


> > So it acts as a hook to control what tests are found.
> >
> >   
> 
> So you're suggesting a protocol for specifying a test suite to use and 
> not 'manually' collect tests from a module / package. That seems like 
> the right level of customization to provide.

Yup.

> > The load_tests hook, which I added to bzr, is an attempt to reduce
> > duplication from the test_suite hook, while adding in support for custom
> > loaders and so on.
> >
> > Compare:
> > def test_suite():
> >     loader = unittest.TestLoader()
> >     names = [__name__] + [__name__ +  ".tests.test_" + name for name in
> >         [
> >         "bar",
> >         "foo",
> >         "gam",
> >         ]]
> >     tests = loader.loadTestsFromNames(names)
> >     # do things to tests here
> >     return tests
> >
> >   
> 
> So if a module provides 'test_suite' it is a callable that provides the 
> tests for that module.
> 
> The code above implies that it loads the tests for the whole package - 
> so if test_suite is found in a package (the __init__.py) then we stop 
> recursing into the package. If test_suite is found in a module then we 
> use it for that module but continue searching the package / sub-packages.
> 
> (Note: I see below that you suggest controlling whether we recurse into 
> packages by having load_tests return two values. I think this is 
> starting to get overly complex for an initial version. See below for my 
> suggestions.)

Well, the problem with standardising is that there is immediate pressure
to 'get it right' because upgrading a protocol is harder. (e.g. you need
a new hook point with a slightly different name, docs about the
differences etc).

That said there are many spellings that could be done - e.g. one hook
for 'get the suite' and another for 'control recursion'. 

> > with
> > def load_tests(standard_tests, module, loader):
> >     names = ["tests.test_" + name or name in 
> > 	[
> > 	"bar",
> >         "foo",
> >         "gam",
> > 	]]
> >     standard_tests.addTest(loader.loadTestsFromNames(
> >         names, module=module))
> >     # Do things to standard_tests here
> >     return standard_tests
> >
> > The second one removes all the machinery for choosing a loader (allows
> > custom loader) 
> 
> Why is test_suite not enough? test_suite allows you to use a custom 
> loader and anything you do in load_tests to reduce duplication could 
> also be done in test_suite.

bugs in test_suite:
 - cannot control the loader from outside the hook (which means that
   when you have a better loader you have to change all your test_suite
   hooks manually to use it, or monkeypatch unittest.TestLoader. Yes, 
   I've seen that done).
 - about 80% of all test_suite methods look the same; package scope ones
   are a list of imports and recursive returns, module ones construct
   a loader and use it.
 - errors are reported poorly and typically result in the entire test
   suite not running.
 - cannot customise the TestSuite returned (but loaders know how to make
   a suite, so customising the loader customises the suite as well).

> My initial goal is to provide a simple system that works. Having the 
> test_suite protocol seems like it should be enough to allow for 
> customisation of module / package loading. Why not have test_suite take 
> a loader argument and the implementation of test_suite is free to use it 
> or ignore it.

Well, that makes it closer to 'load_tests'. Indeed, for a load_tests
hook you can ignore the loader object if you want to. But often you
don't need to, and thats the nice thing about it :). The reason
load_tests is not called 'test_suite' is simply API compatibility, bzr's
test loader has supported test_suite for a long time, when we added
load_tests we didn't want to cause any incompatibility.

..

> Here you are changing the suggested protocol above so that load_tests 
> allows individual *packages* to customize how discovery is done inside 
> them (it doesn't seem like that would make sense for a module as you 
> could then get multiple different contradictory rulesets within a single 
> package).

Well, it might make sense for a module if the package didn't customise
anything.
 
> Anyway, at this point we're getting beyond what I want to achieve with 
> an initial version. The test_suite protocol seems to allow modules and 
> packages to completely customize how tests are loaded from them. Don't 
> forget that they can also use their own DiscoveringTestSuite if they 
> want to change the filter rules.
> 
> So in summary +1 on the test_suite protocol (a callable taking a loader 
> as an argument) and -1 on load_tests and the more complex rules.
> 
> We can always add load_tests as a next step, whereas if we add it now we 
> are tied to forever remaining compatible with the API signature we choose.

So test_suite, if you want to be compatible with the folk using it is
just 'def test_suite():'. If you're not worried about that, I'd spell it
as
def test_suite(standard_tests, module, loader):
    "Return a TestSuite for this module."""

Avoiding all the concerns about controlling recursion, and just disable
recursion when you encounter one of these. This is broadly what you
describe above - the only modification I'm making is to pass in the
module and loader (and then because I've found them both extremely
useful to pass in).

FWIW, in bzr we don't have test discovery, so the whole issue of
controlling recursion never came up.

-Rob

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : http://lists.idyll.org/pipermail/testing-in-python/attachments/20090406/bfa81033/attachment.pgp 


More information about the testing-in-python mailing list