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:31:41 +0200	[thread overview]
Message-ID: <539AFD3D.8090407@drmicha.warpmail.net> (raw)
In-Reply-To: <539AFAF3.9040707@viscovery.net>

Johannes Sixt venit, vidit, dixit 13.06.2014 15:21:
> Am 6/13/2014 15:06, schrieb Michael J Gruber:
>> 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.
> 
> I don't think so. What is the outcome of
> 
> 	false &&  # simulate a regression
> 	grep foo && exit 1
> 	! grep bar || exit 1
> 
> assuming that the '! grep bar' happens to be true? Answer: The regression
> is not diagnosed because the &&-chain is broken.
> 
> *That* is what I think you described earlier in this thread as "I put
> something failing on the first line of the original version, and the test
> succeeded."
> 
> -- Hannes

If you say that something I have said makes sense I'm happy, because I
can't confirm that myself right now. I'll take a break and look into a
rewrite of the form

a &&
b &&
test_must_fail c &&
d || exit 1

hoping that will make things both readable (by avoiding !) and concise
(by avoiding repeated exits).

Michael

  parent reply	other threads:[~2014-06-13 13:31 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
2014-06-13 13:21                 ` Johannes Sixt
2014-06-13 13:30                   ` Jeff King
2014-06-13 13:31                   ` Michael J Gruber [this message]
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=539AFD3D.8090407@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).