[Python-Dev] patch for review: unittest ImportError handling

[Michael Foord]

On 14/04/2010 12:54, Chris Jerdonek wrote:
> On Wed, Apr 14, 2010 at 2:07 AM, Michael Foord
> <fuzzyman at voidspace.org.uk>  wrote:
>    
>> I'm still not convinced that this isn't a backwards incompatible change - up
>> until now, however horrible it may be, TestLoader.loadTestsFromName only
>> raised an AttributeError when it failed to load a test. Changing it to allow
>> it propagate ImportError means that code catching errors by handling
>> AttributeError will be potentially broken by the fix. I'm certainly happy to
>> discuss this here though.
>>      
> Thanks for your response. Michael.  To understand your position
> better, would you view changing the AttributeError in *any* way an
> incompatible change (e.g. changing just the error message), or is it
> only changing the error type that you view as backwards incompatible?
>
> The reason I ask is that I think it's the change in what the error
> contains (e.g. the error message and stack trace) that is the more
> important part of this change -- rather than whether that information
> be wrapped in an ImportError versus an AttributeError.  It is the
> information rather than the particular error type that will assist an
> end-developer in diagnosing future unit-test failures.
>    

Changing the error message to provide more useful information, possibly 
including the original traceback, would certainly avoid the potential 
for incompatibility. I'd be interested in seeing what other folks here 
on python-dev think.

>    
>> An alternative fix would be for a new API and deprecation of
>> loadTestFromName. A new API could return a placeholder test that raises the
>> original error when run - that way individual errors don't break the test
>> collection phase but are still reported. This *could* be added to
>> loadTestsFromName with an optional argument.
>>      
> I'm not opposed to adding a new API, but I think it would be valuable
> if we could find a way for people to enjoy the benefit of not having
> the stack trace swallowed -- without having to change their code.

It's a double edged sword though - it means existing users depending on 
the fact that this API only raises AttributeError have to change *their* 
code.

For a new API I like Rob Collin's suggestion of a keyword argument to 
loadTestsFromNames that returns an error placeholder instead of raising 
an exception. That couldn't be put into 2.7 now though.

> I'm
> currently part of a project where the current behavior makes it harder
> to track down unit test failures.  I imagine many developers are in a
> similar situation.  This seems to be a bug, and such developers may be
> in a position where they can't change how their unit tests are run,
> but they're nevertheless stuck diagnosing the failures.
>
> I'm writing on the assumption that there is a way to preserve the
> stack trace and error message of an ImportError while re-raising it as
> an AttributeError.
>
>    

The original stacktrace could be included as part of the error message. 
Pretty horrible though.


>> By the way, in general please don't assign unittest bugs *away* from me on
>> the tracker.
>>      
> Will do.  I hadn't come across any guidance re: assigning in the
> development workflow documentation.  Maybe we should add a cautionary
> note about this in the documentation somewhere.  In general, do all
> reports stay assigned to the maintainer (if a maintainer exists), or
> is it a per-maintainer preference on how that is handled?
>
>    
Where an issue is assigned to an existing maintainer I would wait for a 
response on the issue tracker or raise it here on python-dev rather than 
re-assigning the issue. In general issues corresponding to modules that 
have an active maintainer should be assigned to the maintainer.

It is particularly an issue for me with unittest because I want to track 
changes in unittest in the unittest2 package and need to know about all 
changes. I'm often around on #python-dev to discuss things.

All the best,

Michael Foord


> Thanks,
> --Chris
>    


-- 
http://www.ironpythoninaction.com/