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

> Jeff King venit, vidit, dixit 13.06.2014 13:46:
>> On Fri, Jun 13, 2014 at 12:42:46PM +0200, Michael J Gruber wrote:
>>
>>> When t7510 was introduced, the author made sure that a for loop in
>>> a subshell would return with the appropriate error code.
>>>
>>> Make sure this is true also the for the first line in each loop, which
>>> was missed.
>>>
>>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>>> ---
>>>  t/t7510-signed-commit.sh | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
>>> index 5ddac1a..a5ba48e 100755
>>> --- a/t/t7510-signed-commit.sh
>>> +++ b/t/t7510-signed-commit.sh
>>> @@ -49,7 +49,7 @@ test_expect_success GPG 'show signatures' '
>>>  	(
>>>  		for commit in initial second merge fourth-signed fifth-signed sixth-signed master
>>>  		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
>>
>> Hrm. The original is:
>>
>>   X &&
>>   Y || exit 1
>>
>> Won't that still exit (i.e., it is already correct)? Doing:
>>
>>   for X in true false; do
>>     for Y in true false; do
>>       ($X && $Y || exit 1)
>>       echo "$X/$Y: $?"
>>     done
>>   done
>>
>> yields:
>>
>>   true/true: 0
>>   true/false: 1
>>   false/true: 1
>>   false/false: 1
>>
>> (and should still short-circuit Y, because we go from left-to-right).
>>
>> I do not mind changing it to keep the style of each line consistent,
>> though. I would have written it as a series of "&&"-chains, with a
>> single exit at the end, but I think that is just a matter of preference.
> 
> If I remember correctly, I put something failing on the first line of
> the original version, and the test succeeded. I think the point is that
> we have a for loop in a subshell, and we need to make sure that the
> false of one iteration is not overwritten by the true of the next one -
> "exit 1" makes sure to "break" the for loop and exit the subshell.
> (The chain should do that as well, I'll recheck.)