[TIP] Test discovery for unittest

[Robert Collins]

On Mon, 2009-04-06 at 16:15 +0100, Michael Foord wrote:
> 
> Or just instantiate your custom TestLoader inside test_suite. If you 
> switch to using a custom one from not using a custom one then you
> need 
> to update all of your custom hooks irregardless of whether it is
> spelled 
> test_suite or load_tests. I don't see how providing an additional
> hook 
> makes this any simpler.

If you spell the hook in such a way that a loader is passed to it in the
first place, then only hook implementations that want a custom loader
will have one in the first place.

> >  - 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.
> >   
> 
> Why is that a problem? It sounds like an argument in favour of 
> test_suite - typically they are simple and all look the same.

Don't Repeat Yourself :).

> >  - errors are reported poorly and typically result in the entire
> test
> >    suite not running.
> >   
> 
> How does load_tests alleviate this problem?

We can fix the loader - we have code in bzr to handle import errors
during test loading in such a way that the whole suite isn't aborted. I
would do it differently the second time around, but the code would still
go in the loader. (Also see Olemis' point about test discovery arguably
being a loader responsibility - I agree with him about that. I don't
agree about DiscoveringTestSuite, it is just an adapter from loader to a
memoised test suite, and that's useful)

> I agree that one problem with test discovery is that if it doesn't 
> behave how you expect you can have tests not run at all. You can
> solve 
> this at the TestRunner / TestResult point by collecting information 
> about all tests run and (for example) comparing the total number to 
> previous runs. i.e. it can be solved outside the discovery mechanism
> itself.
> 
> Other suggestions welcomed.

So, I'm not proposing we do recursion control right now. But if we
did...

# controls recursion, and when its true recursion happens before 
# load_tests is called, so load_tests gets the child tests
def load_test_recusion():
    return True

def load_tests(standard_tests, module, loader):
    def test_count(self):
        assertEqual(154, standard_tests.countTestCases())
    standard_tests.addTest(FunctionTestCase(test_count))
    return standard_tests


This breaks horribly if the loader does partial loading [which is one
common performance optimisation on large suites]. So the loader would
need some interface which this test could use.

> >  - cannot customise the TestSuite returned (but loaders know how to
> make
> >    a suite, so customising the loader customises the suite as well).
> >
> >   
> 
> Why can you not customise the suite returned? test_suite is
> responsible 
> for creating the suite and has complete control over which loader it
> uses.

You can't customise it externally. There are several reasons to hook
test loading:
 - fix up what discovery might do
 - customise tests [add parameters, multiply by interfaces]
 - add or remove tests [e.g. bring in tests from a module not in this
   namespace]
 - decorate the tests [add skip decorators for instance]
 - control the test suite to be returned [add a order randomiser for
   instance]

Manually choosing the test suite is only one reason - and hooks that are
not wanting to do this should just inherit any choice the loader has for
suite class. This is permitted by passing in the loader which you've
agreed to already, so its moot.

> We should allow test_suite to return None by the way - meaning no
> tests 
> for the module / package.

Uhm, sure. Returning loader.suiteClass() or TestSuite() is also very
easy, and makes code calling into the hook clearer, but I don't care
either way, other than having a learnt preference for clear ;).

> So lets pass the loader into test_suite. I don't understand the
> purpose 
> of the other two arguments though.

Ok, 1 down two to go :).
in bzr we pass the tests the loader _found_ into the hook so that the
hook doesn't need to ask the loader for them. Why does this matter? Its
a simple factoring out of common code, and you can see the utility in
the sketch I gave about to cross check what the loader loaded, without
having to repeat knowledge the loader has.

As for the module parameter, its probably not needed. I've used it a few
times - its syntactic sugar for os.modules[__name__]. It won't bother me
if its not there, as its probably somewhat too specialised.

> > 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).
> >   
> 
> The loader yes.
> 
> What is the use case for passing in the module? Can't test_suite just
> do 
> '__import__(__name__)' if it wants a reference to its containing
> module?

I don't remember. You may be deeply nested under a top level import
during loading of tests - this happened a lot with the old test_suite()
signature, but a lot less so with the load_tests for some reason.

> At the point at which test_suite is called standard_tests doesn't
> exist 
> as we are delegating to test_suite to *create* the tests. Unless you 
> mean pass in the top level test suite instance that represents all
> tests 
> collected so far. If so, what is the use case?


> Alternatively are you suggesting that we collect tests for the module 
> separately and then pass these in as standard_tests for test_suite to 
> augment or ignore? test_suite can trivially get that itself surely?
> (All 
> TestCase classes in its global namespace.)

Yes, thats exactly what I'm suggested we do. Its something every module
hook does, so factoring it out was natural.

>> FWIW, in bzr we don't have test discovery, so the whole issue of
> > controlling recursion never came up.
> >   
> 
> I quite like the ability of a package to completely control how its 
> tests are loaded. It seems a natural extension of allowing modules to 
> control how their tests are loaded.
> 
> The discovery mechanism as currently implemented completely ignores
> the 
> top level package files (it explicitly skips the __init__.py).

Other than importing each package it finds and inspecting for a hook it
can continue to do so :).

-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/20090407/1a427e22/attachment.pgp