From: Junio C Hamano <junkio@cox.net>
Date: Thu, 12 May 2005 19:48:03 GMT
Subject: Re: [PATCH] Test suite
Message-ID: <7vll6kfcp8.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20050512192941.GC324@pasky.ji.cz>

>>>>> "PB" == Petr Baudis <pasky@ucw.cz> writes:

>> --- /dev/null
>> +++ b/t/test-lib.sh
>> +# For repeatability, reset the environment to known value.
>> +export LANG C
>> +export TZ UTC

These are just "Duh".  Sorry.

>> +
>> +test_description () {
>> +	while case "$#" in 0) break;; esac

PB> Duh. This looks mysterious - why not a simple test?

Because I am old fashioned and am used to this kind of Bourne
shell idioms.

PB> This branch makes no sense, I think.

Because...?

PB> Again, why not a simple test?

Again using case when test is not needed is a good old Bourne
shell idiom.

PB> [ "$debug" ] && eval "$*"

PB> (Actually, eval will do the wrong thing here - it just concatenates the
PB> arguments. Just "$@" would do, I guess.)

No, I wrote "$*" because that was what I wanted and not "$@".  I
wanted to give eval a _single_ string.

PB> Please clean up after yourself in this case.

I would not mind, but I'd mildly disagree.  The next test will
start by cleaning up test-repo/ anyway.  What we are lacking is
t/Makefile that drives all the tests and clean-up can be done
there.

>> +git-init-db 2>/dev/null || error "cannot run git-init-db"

PB> But there's no 'error' thing.

Yes, and that way you can get an error message ;-).  Simple
oversight.

PB> The testcases currently utterly fail,

That is something I told you in the cover letter (or commit log).