[MERGE] test framework distinguishes skips

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Aaron Bentley wrote:
> Martin Pool wrote:

...


>>> Suppose we have a test which creates unicode filenames.  We expect it
>>> to be skipped on systems where you can't do that.  But suppose someone
>>> introduces a bug that causes a UnicodeError in code unrelated to the
>>> filesystem - we won't see the failure because it will just be skipped on
>>> all platforms.
> 
> Even expected failures should be more visible than they are now.  I only
> recently discovered that one of my machines was skipping a bunch of
> tests because its console is ISO-8859-1.

Well, I'm actually more concerned that we will have a test which fails
for the wrong reason, and gets skipped.

Specifically because you are catching any UnicodeError that the test may
raise. Rather than just the single one that occurs when setting up the
test. (I just saw that Martin raised the same concern)

> 
>>> The heart of the problem is that builtin exceptions which can be raised
>>> from many places should be caught in tight scopes, otherwise you're
>>> likely to catch something you didn't want.
> 
> Agreed.
> 
>>> So even though the reduction in size is very nice I don't think we
>>> should put that in.
> 
> Okay.  I'll look at addressing those cases with separate methods.
> 
>>> Maybe we can instead have an option to TestCase.build_tree that makes it
>>> skip if theres a unicode error while creating the directory?  That would
>>> cover many cases and be less risky.
> 
> I think that would be nice.
> 
>>> For some of these, like lsprof, I'd think you could just check the
>>> dependency in the test class setup, can't you?
> 
> The two lsprof test cases each import lsprof to get their effect.  They
> share a TestCase with many other tests, so if we're to do handle the
> dependency in setUp, we should split the lsprof tests out into their own
> TestCase.  Using the decorator on the tests seems simpler to me, but we
> can do it that way if you want.
> 
>>> Is it ever really reasonable to catch all UnicodeErrors, rather than
>>> just expecting them for particular calls?
> 
> Perhaps not.  I'll look at reraising these as TestSkipped subclasses
> instead.
> 
>>>> +    def test_expected_failure(self):
>>>> +        self.assertRaises(ExpectedFailure, self.raises_expected)
>>>> +        self.assertRaises(ExpectedFailure, self.raises_expected_match)
>>>> +        self.assertRaises(errors.BzrCommandError, self.raises_expected_nomatch)
>>>> +        self.assertRaises(errors.BzrCommandError, self.raises_unexpected)
>>>> +        self.assertRaises(errors.FailedFailure, self.doesnt_raise)
>>>> +        self.assertEqual("I didn't raise", self.doesnt_raise_permitted())
>>> Very nice to see this tested this way.
> 
> Glad you like it.  Now what's your favourite colour of sheep :-)?

I like the pony myself. :)

> 
>>> I do like the cleanup a lot but I'd like to resolve those issues before
>>> merging it.
> 
> That's fine.  I just wanted to get the ball rolling.  I should really
> have used [RFC], not [MERGE].
> 
> Aaron

For some reason I got Aaron's response but not Martin's mail. But in
general I thought a lot of things looked cleaner, but perhaps a little
too clean for some things. (Like the unicode exception catching).

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFErd+wJdeBCYSNAAMRApnbAKCKGDEg1KdyuhSLXgBni6qsLfdJWgCgwvXW
W6PxaNzmRvENO7rw0myF6U8=
=H5BK
-----END PGP SIGNATURE-----