[MERGE] test framework distinguishes skips

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> Then we parameterize TestSkipped to include *why* the test was skipped.
>>> Which can be 'dependency missing', 'unicode failure', etc.
> 
> That's double work.  I've already added subclasses for the particular
> Skip rationales.

Well, you could have the decorator raise the right exception based on
what its parameters where. So:

skip_on_exception(ParamikoNotPresent, dependency=True)

would raise DependencyNotPresent

> 
>>> I also kind of which this type of decorator would let you pass in a
>>> message string to give more information:
>>>
>>> skip_on_exception(ParamikoNotPresent, msg='Test requires paramiko')
>>>
>>> But if we can include all that information in the parameters, we don't
>>> need another parameter for msg.
> 
> But in the DependencyNotPresent, I'm holding a copy of the original
> exception, which says """Unable to import paramiko (required for sftp
> support): %(error)s""", which I think will convey the same thing.

I think so.

> 
>>>> (Hm, there may still be restrictions about default encoding
>>>> or locale, but perhaps they'd be ok in utf-8?)
>>>
>>> Well, the full test suite runs with 'LANG=C', which causes lots of
>>> things to skip because of Unicode issues.
> 
> Failure to support Unicode is a platform deficiency.  But perhaps
> unicode skips should have their own toggle, so we can assert that there
> are no unicode skips when LANG=.*UTF-8

sounds about right.

> 
>>> Except for things that are skipped because they are adapted to formats
>>> that don't support them. (Old-format branches that can't be initialized,
>>> LocalTransport doesn't have 'bogus_url' support, and really can't, etc).
> 
> Those are ExpectedFailures, which is a different class from PlatformDefect.
> 
>>> But those shouldn't really be TestSkipped, since they are invalid tests.
> 
> I'm not sure what you're saying.  They're not TestSkipped anymore,
> they're a subclass: ExpectedFailure.
> 
> The determination of whether or not these tests are valid is most easily
> done by running the tests.  If they're not valid, they should be
> skipped.  It seems reasonable to raise a subclass of TestSkipped in
> order to skip them.
> 
>>> However, for a while, I think SFTP didn't have bogus_url support, and it
>>> really should (as well for ftp, etc).
>>> So those should be TestSkipped() indicating that we should fix them.
>>> While LocalTransport should be a TestNotPossible().
> 
> Feel free to specialize it further once we get it in.
> 
> Aaron

The problem is that our current framework makes it difficult to do this.
I suppose LocalTransport.get_bogus_url() could raise TestNotPossible(),
and then other ones could raise TestSkipped('implement me')

But this is the difficulty with just using a catch-all decorator. You
really want the tests themselves to raise the most specific exception
they can. Since they are the only ones that know what is going on inside.

I don't know what your current state is, though. And it sounds like you
might have implemented some of this stuff. So I'll wait to see your next
patch before commenting too much further.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060707/85b4b377/attachment.pgp