Re: [PATCH] Test suite

From: Junio C Hamano <junkio@cox.net>
Date: May 12, 2005, 19:48 UTC
Message-ID: <7vll6kfcp8.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20050512192941.GC324@pasky.ji.cz>

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

Show 5 quoted lines

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