From: Thomas Rast <trast@inf.ethz.ch>
Date: Tue, 18 Jun 2013 08:45:59 GMT
Subject: Re: [PATCH v2 3/6] test-lib: verbose mode for only tests matching a pattern
Message-ID: <8761xbdcxk.fsf@linux-k42r.v.cablecom.net>
In-Reply-To: <20130618053710.GB5916@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Mon, Jun 17, 2013 at 11:18:48AM +0200, Thomas Rast wrote:
>
>> As suggested by Jeff King, this takes care to wrap the entire test_expect_*
>> block, but nothing else, in the verbose toggling.  To that end we use
>> a new pair of hook functions.  The placement is a bit weird because we
>> need to wait until the beginning of test_skip for $test_count to be
>> incremented.
[...]
>   test_start ;# increment number, run setup hooks
>   if ! test_skip
>   then
>     ...
>   fi
>   test_finish ;# teardown hooks
>
> Then it is a bit easier to see that each start has a finish (whereas in
> the current version, the setups in test_skip are matched by individual
> teardowns in each caller). I did not look too hard at it, though, so I
> wouldn't be surprised if there is some other hidden order dependency
> that makes that not work. :)

No, I think that's actually very reasonable.  I'll do it that way in v3.

> But then what is this hunk doing:
>
>>  test_eval_ () {
>>  	# This is a separate function because some tests use
>>  	# "return" to end a test_expect_success block early.
>> @@ -358,9 +399,7 @@ test_run_ () {
>>  
>>  	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
>>  	then
>> -		setup_malloc_check
>>  		test_eval_ "$test_cleanup"
>> -		teardown_malloc_check
>>  	fi
>>  	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"
>>  	then

Thanks for catching this -- it's just a mis-edit that would effectively
revert 1/6.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch