[TIP] Test discovery for unittest

[Michael Foord]

I'm happy with a package / module level hook that takes loader and tests 
arguments and is called load_tests or test_suite.

It should return a test suite or None, and if available at the package 
level the current implementation won't continue discovery into the 
package (allowing a different return value at some future point to 
modify discovery for the package would be possible).

Now one of us needs to implement it. :-)

Michael



Robert Collins wrote:
> 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
>   


-- 
http://www.ironpythoninaction.com/
http://www.voidspace.org.uk/blog