git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michael J Gruber <git@drmicha.warpmail.net>
To: Johannes Sixt <j.sixt@viscovery.net>, Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCHv2 4/6] t7510: exit for loop with test result
Date: Fri, 13 Jun 2014 15:06:16 +0200	[thread overview]
Message-ID: <539AF748.3000901@drmicha.warpmail.net> (raw)
In-Reply-To: <539AF49F.3010309@viscovery.net>

Johannes Sixt venit, vidit, dixit 13.06.2014 14:54:
> 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:

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

>  			! 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

  reply	other threads:[~2014-06-13 13:06 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-06 14:15 [PATCH 0/3] verify-commit: verify commit signatures Michael J Gruber
2014-06-06 14:15 ` [PATCH 1/3] pretty: free the gpg status buf Michael J Gruber
2014-06-06 14:15 ` [PATCH 2/3] gpg-interface: provide access to the payload Michael J Gruber
2014-06-13  7:55   ` Jeff King
2014-06-13  9:44     ` Michael J Gruber
2014-06-13 10:34       ` Jeff King
2014-06-06 14:15 ` [PATCH 3/3] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-11 19:48   ` Michael J Gruber
2014-06-13  8:02   ` Jeff King
2014-06-13  9:55     ` Michael J Gruber
2014-06-13 11:09       ` Jeff King
2014-06-13 17:06         ` Junio C Hamano
2014-06-16  9:21           ` Michael J Gruber
2014-06-16 19:54           ` Jeff King
2014-06-16 20:34             ` Junio C Hamano
2014-06-16 20:39               ` Jeff King
2014-06-27 12:31                 ` Michael J Gruber
2014-06-27 12:49                   ` Michael J Gruber
2014-06-27 13:06                     ` Michael J Gruber
2014-06-27 13:18                       ` [PATCH] log: correctly identify mergetag signature verification status Michael J Gruber
2014-06-28  0:44                         ` Jeff King
2014-07-10 22:27                           ` Junio C Hamano
2014-06-27 13:50                     ` [PATCH 3/3] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-27 18:55                       ` Junio C Hamano
2014-06-27 18:36                     ` Junio C Hamano
2014-06-28  0:32                       ` Jeff King
2014-06-30  6:14                         ` Junio C Hamano
2014-06-13 10:42 ` [PATCHv2 0/6] verify-commit: verify commit signatures Michael J Gruber
2014-06-13 10:42   ` [PATCHv2 1/6] pretty: free the gpg status buf Michael J Gruber
2014-06-13 11:39     ` Jeff King
2014-06-13 10:42   ` [PATCHv2 2/6] gpg-interface: provide access to the payload Michael J Gruber
2014-06-13 10:42   ` [PATCHv2 3/6] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-13 11:19     ` Jeff King
2014-06-13 11:45       ` Michael J Gruber
2014-06-13 11:50         ` Jeff King
2014-06-13 12:12           ` Michael J Gruber
2014-06-13 10:42   ` [PATCHv2 4/6] t7510: exit for loop with test result Michael J Gruber
2014-06-13 11:46     ` Jeff King
2014-06-13 12:04       ` Michael J Gruber
2014-06-13 12:22         ` Michael J Gruber
2014-06-13 12:33           ` Michael J Gruber
2014-06-13 12:45             ` Jeff King
2014-06-13 12:54             ` Johannes Sixt
2014-06-13 13:06               ` Michael J Gruber [this message]
2014-06-13 13:21                 ` Johannes Sixt
2014-06-13 13:30                   ` Jeff King
2014-06-13 13:31                   ` Michael J Gruber
2014-06-13 13:42                     ` Johannes Sixt
2014-06-13 18:23       ` Junio C Hamano
2014-06-13 10:42   ` [PATCHv2 5/6] t7510: test verify-commit Michael J Gruber
2014-06-13 11:51     ` Jeff King
2014-06-13 12:14       ` Michael J Gruber
2014-06-13 18:16         ` Junio C Hamano
2014-06-13 10:42   ` [PATCHv2 6/6] gpg-interface: provide clear helper for struct signature_check Michael J Gruber
2014-06-23  7:05   ` [PATCHv3 0/5] verify-commit: verify commit signatures Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 1/5] gpg-interface: provide clear helper for struct signature_check Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 2/5] gpg-interface: provide access to the payload Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 3/5] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 4/5] t7510: exit for loop with test result Michael J Gruber
2014-06-23  7:05     ` [PATCHv3 5/5] t7510: test verify-commit Michael J Gruber
2014-06-23 23:02       ` Junio C Hamano
2014-06-23 17:28     ` [PATCHv3 0/5] verify-commit: verify commit signatures Jeff King
2014-06-23 17:52       ` Junio C Hamano
2014-06-23 21:09         ` Jeff King
2014-06-23 21:23           ` Junio C Hamano
2014-06-27 14:13             ` [PATCHv4 0/4] " Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 1/4] gpg-interface: provide clear helper for struct signature_check Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 2/4] gpg-interface: provide access to the payload Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 3/4] verify-commit: scriptable commit signature verification Michael J Gruber
2014-06-27 14:13               ` [PATCHv4 4/4] t7510: test verify-commit Michael J Gruber
2014-06-27 19:32                 ` Junio C Hamano
2014-06-27 20:26                   ` Michael J Gruber
2014-06-27 19:07               ` [PATCHv4 0/4] verify-commit: verify commit signatures Junio C Hamano
2014-06-28  0:48                 ` Jeff King
2014-06-28  0:49               ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=539AF748.3000901@drmicha.warpmail.net \
    --to=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).