[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Add more tests to the svn python bindings

From: Madan U Sreenivasan <madan_at_collab.net>
Date: 2006-06-24 02:45:39 CEST

On Sat, 24 Jun 2006 00:15:05 +0530, David James <djames@collab.net> wrote:

> On 6/23/06, Madan U Sreenivasan <madan@collab.net> wrote:
>> Hi,
>>
>> Pl. find attached some more tests for the svn.wc python bindings.
>
> Nice patch, Madan! A few minor issues:
>
> 1. Instead of using failUnless and failUnlessRaises, please use
> assert_ and assertRaises. This is more consistent with our existing
> test cases.

I had a discussion with Jelmer on this on irc (We had discussed a bit of
this on the dev@ list too, in the thread starting
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=116865, and
specifically at
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=116911)

The summary : Jelmer wanted to use assert...() functions, and I wanted to
use fail...() functions wherever available. My rationale behind it is
simple: fail...() makes more sense when we write a test case. Its self
explanatory to say failIf() than to say assert_(). The statment "A test
case failed" is more natural than the computing jargon "A test case
asserted". :)

> 2. Instead of saying 'failUnless(x == y)", use "assertEqual(x, y)".

didn't we decide not to use assertEqual in favor of assert_() because its
available only in the recent versions of python?

> This outputs a better error message if the test fails.

Really? I thought these were synonyms.
http://docs.python.org/lib/testcase-objects.html uses the same description
for both these functions.

> 3. For the test case which throws SubversionException, please add a
> comment. It should say something like "Ensure that setting svn:entry
> props is not allowed". I trust that this behavior is correct (if not,
> add a TODO explaining how it should normally behave.)

Okay, shall I assume this has to be specifically done for all failure test
cases in future?

> After these issues are fixed, +1 to commit.

Cool. Thanks for the review.

>> btw, I almost completed implementing XFail() for the bindings, when
>> I
>> realized that assertRaises()/failUnlessRaises() do the same thing. Why
>> do
>> we need XFail for the python bindings tests?
>
> Good point. Actually, now that I think about it, assertRaises is
> probably better than XFail, because you can test what exception is
> raised. All tests which test for incorrect behaviour should contain a
> TODO marker explaining what's wrong and how they should be fixed.

yeah, the TODO marker is important. Sometime next week, I will write a
small convention document for the python tests (or maybe add it to the
existing README) including this point.

Regards,
Madan U S
www.symonds.net/~madan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jun 23 21:16:25 2006

This is an archived mail posted to the Subversion Dev mailing list.