[MERGE] test framework distinguishes skips

Aaron Bentley aaron.bentley at utoronto.ca
Fri Jul 7 04:40:48 BST 2006


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

Martin Pool wrote:
> On  6 Jul 2006, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
>> The dependency_failure decorator...
> 
> Nice function, but maybe not quite the right name.  How about
> @test_requires or @test_dependency?

Yeah, I did think about that.  It's a bit awkward, because the parameter
is an exception, e.g. ParamikoNotPresent.  I thought
@dependency_failure(ParamikoNotPresent) made more sense than
@test_requires(ParamikoNotPresent).

But the exception does actually name the missing dependency, so it would
be possible to do @test_requires("paramiko").  Would you prefer that?

>> The platform_requirement decorator indicates that this test requires
>> facilities that are not supported by some platforms.  This class of
>> skips can be run on another platform.
> 
> I think this might be simpler as a regular method, not a decorator.

Okay, I may have gotten carried away there.  My main desire is to make
the exceptions consistent, but that can be done just as easily with a
normal function as with a decorator.

> This smells like this method is trying to do too many things.  Why not
> just make three of them?  (Well, because stacking three decorators might
> look bad, but that's kind of a point against decorators.)

I think by this point, everything looked like a nail.  I can't justify
it-- there were no test cases that required more than 1 of these flags
to be on at a time.

> 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.

> 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 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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFErde/0F+nu1YWqI0RAqYtAJ431mGUdzZSiHja9doGcxquXrCHVwCdEOjs
VsXQgg3uIjhZFwVrafqwspQ=
=i8fu
-----END PGP SIGNATURE-----




More information about the bazaar mailing list