[TIP] Why do we need loadTestsFromPackage ?

[Michael Foord]

Olemis Lang wrote:
> On Tue, Apr 14, 2009 at 12:11 PM, Michael Foord
> <fuzzyman at voidspace.org.uk> wrote:
>   
>> Olemis Lang wrote:
>>     
>>>> what would the interface to the
>>>> discovery mechanism look like?
>>>>         
>>> {{{
>>> #!python
>>> simpleloader = Loader1()
>>> s1 = simpleloader.loadTestsFromModule(pkg)
>>> discoloader = DiscoLoader(simpleloader)
>>> s2 = discoloader.loadTestsFromModule(pkg)
>>> }}}
>>>       
>> Shoehorning discovery into loadTestsFromModule is worse - it is less
>> amenable to customization as you can't customize loading tests from a module
>> whilst still reusing the discovery mechanism.
>>
>>     
>
> I'll try to show you the way to refine a loader in the approach I
> mentioned. Let's say that Loader1 loads «blue» tests and Loader2 loads
> «dark blue» tests. In the following snippet Loader2 refines Loader1
> and the code needed for test discovery is almost the same
>
> {{{
> class Loader2(Loader1):
>   def loadTestsFromModule(self, *args):
>       # Specific code
>       Loader1.loadTestsFromModule(self, *args)
>       # Specific code
>   # More overrides if needed
>
> simpleloader = Loader2()
> s1 = simpleloader.loadTestsFromModule(pkg)
> discoloader = DiscoLoader(simpleloader)
> s2 = discoloader.loadTestsFromModule(pkg)
> }}}
>
> Please if you feel that I didnt understand what you were saying, I'd
> appreciate if you could provide a somehow concrete example of the
> specific limitation you 'r trying to highlight.
>
>   

Except that with a module that is also a package namespace, delegating 
to loadTestsFromModule will invoke discovery.

If discovery is in a separate method which calls loadTestsFromModule 
then you can overload module loading separately from the discovery 
mechanism. It would be a mistake to conflate them.


>> Whether it is provided on the standard loader or on a subclass is not a very
>> interesting question.
>>
>>     
>
> Please, just to be sure. The «it» above in «Whether it is provided on
> » refers to what ?
>   

It being discovery.
>   
>>>> You say you don't understand why, I don't understand why not.
>>>>         
>>> I understand that loadTestsFromPackage is a candidate. I'm not sure
>>> about its benefits, especially because AFAICS and IMHO there is no
>>> need to include loadTestsFromPackage in order to allow discovery. I
>>> think that the option I mentioned above is more dynamic and is
>>> suitable for diverse discovery strategies, varying apart of the code
>>> for the real | concrete loader.
>>>
>>> Could we pls keep focused on the pros & cons of both approaches with
>>> concrete arguments ?
>>>       
>> "is more dynamic and is suitable for diverse discovery strategies, varying
>> apart of the code for the real | concrete loader." is not a concrete
>> argument and as far as I can tell doesn't actually say anything.
>>
>>     
>
> Provided that DiscoLoader holds a reference to the real loader to use,
> by more dynamic I mean
>
> {{{
> l1 = Loader1()
> l2 = Loader2()
> discoloader = DiscoLoader(l1)
> s1 = discoloader.loadTestsFromModule(pkg)   # Blue Tests
>
>      # in whole package
> discoloader.loader = l2
> s2 = discoloader.loadTestsFromModule(pkg)   # Dark blue Tests
>
>      # in whole package
>
> # And the instance of DiscoLoader is the same
You're still using two different loader instances (three in point of 
fact). How is the above better than:

loader1 = Loader1()
loader2 = Loader2()

s1 = loader1.loadTestsFromPackage(pkg)   # Blue Tests
s2 = loader2.loadTestsFromPackage(pkg)   # Dark Blue Tests


If both Loader1 and Loader2 inherit from the default loader and override 
loadTestsFromModule to implement their different semantics then it is 
easier to understand and you have lost nothing in terms of dynamism.

In actual fact your pattern of subclassing loader, implementing 
different functionality in an overridden method but delegating to 
another instance is functionally identical to (and would be clearer 
expressed) as a single function that takes a loader instance as one of 
the arguments. Personally I find your suggested pattern bizarre and an 
abuse of OO.

> .
> }}}
>
> By «suitable for diverse discovery strategies, varying apart of the
> code for the real | concrete loader» I'm talking about changing the
> way modules are enumerated. If the idea is to override the discovery
> mechanism, then the code'd look like this :
>   

Whereas with loadTestsFromPackage it is much clearer that the method to 
override to vary discovery is loadTestsFromPackage. The programmer is 
also free to override loadTestsFromModule or not.

Michael
> {{{
> class DiscoLoader1(DiscoLoader):
>   def loadTestsFromModule(self, *args):
>       # Specific code
>       self.loader.loadTestsFromModule(self, *args)
>       # Specific code
>   # More overrides if needed, in fact loadTestsFromModule
>   # may be kept just the same, and only more specific
>   # methods refined (i.e. similar to find_files and
>   # module_names_from_path in DiscoveryTestSuite)
>
> loader_class = Loader1 if cond else Loader2
> simpleloader = loader_class()
> s1 = simpleloader.loadTestsFromModule(pkg) # Tests in module
> discoloader = DiscoLoader1(simpleloader)
> s2 = discoloader.loadTestsFromModule(pkg)   # Tests in whole package
> }}}
>
> Therefore each feature changes in a single place and has its own well-
> established hierarchy, which can be combined using loader field in
> DiscoLoader (i.e. actually decorator pattern).
>
>   


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