Re: [PATCHv2 4/6] t7510: exit for loop with test result

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Jun 13, 2014, 13:06 UTC
Message-ID: <539AF748.3000901@drmicha.warpmail.net>
In-Reply-To: <539AF49F.3010309@viscovery.net>

Johannes Sixt venit, vidit, dixit 13.06.2014 14:54:

Show 23 quoted lines

> Am 6/13/2014 14:33, schrieb Michael J Gruber:
>> .... with this loop, sorry:
>>
>> for X in true false; do
>>      for Y in false true; do
>>        ($X && $Y || exit 1)
>>      done
>>      echo "$X/last inner $Y: $?"
>> done
>>
>> gives
>>
>> true/last inner true: 0
>> false/last inner true: 1
>>
>> even though on both cases we have at least one failure of Y. (failure of
>> one subtest = failure of the test)
> 
> Place the loop(s) inside the subshell, and you observe termination on the
> first failure, and a failure exit code of the subshell.
> 
> The change proposed in this patch should not be necessary. Something else
> must be wrong with your tests.

I know I started this (or Jeff did, who knows ;) ), but we keep confusing each other more and more:

Show 9 quoted lines

> Ah, here it is:
> 
> @@ -58,7 +58,7 @@ test_expect_success GPG 'show signatures' '
>  	(
>  		for commit in merge^2 fourth-unsigned sixth-unsigned seventh-unsigned
>  		do
> -			git show --pretty=short --show-signature $commit >actual &&
> +			git show --pretty=short --show-signature $commit >actual || exit 1
>  			grep "Good signature from" actual && exit 1

This is as in the original, it tests invalid signatures, so "Good signature" should not be in the response.

Show 6 quoted lines

>  			! grep "BAD signature from" actual || exit 1
>  			echo $commit OK
> 
> Notice the '&& exit 1'! It should be '|| exit 1', right?
> 
> -- Hannes

In other words, the original tests already had

grep foo && exit 1 ! grep bar || exit 1

to test that we have neither foo nor bar. The reason is (supposedly) to have this portion of the test mostly analogous to the previous one, where we want foo and do want bar.

So this is completely unrelated.

Otoh, it seems the original test could have had

a && b && c || exit 1

or

a || exit 1 b || exit 1 c || exit 1

rather than

a && b || exit 1 c || exit 1

which I thought was incorrect (but I can't recreate the proof right now). I'd say both of the former versions are preferable to the last one unless there is a difference that neither Jeff nor I see.

I need a break before looking at this again ;)

Michael