Re: [PATCH v5 8/8] t/fmt-merge-msg: make gpg/ssh tests more specific

[Junio C Hamano <gitster@pobox.com>]

Fabian Stelzer <fs@gigacodes.de> writes:

> All the GPG, GPGSSH and the new GPGSSH_VERIFYTIME tests are redirecing
> stdout as well as stderr to `actual` and grep for success/failure over
> the resulting flie. However, no output is printed on stderr and we do
> not need to include it in the grep. The newer SSH signing based tests
> are also missing a grep for the merged tag to make sure the merge
> message is correct.
>
> - remove unneccessary 2>&1 redirects
> - add grep for merged tag to gpgssh* tests
>
> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
> ---
>  t/t6200-fmt-merge-msg.sh | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)

The 4 hunks at the beginning seems to be a clean-up of the existing
issue, but aren't hunks -162,7, -170,7, -178,7, 187,7 "oops, we
screwed up in [6/8], and we patch up after the fact"?  

For a new topic not yet in 'next', we'd prefer to pretend to be more
perfect humans by not deliberatly keeping mistakes made in an
earlier step, only to be corrected in a later step.

While there may not be any difference in the end-result, it would be
cleaner and less sloppy to have the 4 hunks to fix the ones before
the series as a preliminary clean-up step, and the other hunks
folded into the step that introduced the problem, no?

Thanks.

> diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
> index 2dd2423643..12a1e62bf0 100755
> --- a/t/t6200-fmt-merge-msg.sh
> +++ b/t/t6200-fmt-merge-msg.sh
> @@ -124,7 +124,7 @@ test_expect_success 'message for merging local branch' '
>  test_expect_success GPG 'message for merging local tag signed by good key' '
>  	git checkout main &&
>  	git fetch . signed-good-tag &&
> -	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual &&
>  	grep "^Merge tag ${apos}signed-good-tag${apos}" actual &&
>  	grep "^# gpg: Signature made" actual &&
>  	grep "^# gpg: Good signature from" actual
> @@ -133,7 +133,7 @@ test_expect_success GPG 'message for merging local tag signed by good key' '
>  test_expect_success GPG 'message for merging local tag signed by unknown key' '
>  	git checkout main &&
>  	git fetch . signed-good-tag &&
> -	GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual &&
>  	grep "^Merge tag ${apos}signed-good-tag${apos}" actual &&
>  	grep "^# gpg: Signature made" actual &&
>  	grep -E "^# gpg: Can${apos}t check signature: (public key not found|No public key)" actual
> @@ -143,7 +143,7 @@ test_expect_success GPGSSH 'message for merging local tag signed by good ssh key
>  	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	git checkout main &&
>  	git fetch . signed-good-ssh-tag &&
> -	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual &&
>  	grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
>  	! grep "${GPGSSH_BAD_SIGNATURE}" actual
>  '
> @@ -152,7 +152,8 @@ test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh
>  	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	git checkout main &&
>  	git fetch . signed-untrusted-ssh-tag &&
> -	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual &&
> +	grep "^Merge tag ${apos}signed-untrusted-ssh-tag${apos}" actual &&
>  	grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual &&
>  	! grep "${GPGSSH_BAD_SIGNATURE}" actual &&
>  	grep "${GPGSSH_KEY_NOT_TRUSTED}" actual
> @@ -162,7 +163,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign
>  	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	git checkout main &&
>  	git fetch . expired-signed &&
> -	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual &&
> +	grep "^Merge tag ${apos}expired-signed${apos}" actual &&
>  	! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
>  '
>  
> @@ -170,7 +172,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign
>  	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	git checkout main &&
>  	git fetch . notyetvalid-signed &&
> -	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual &&
> +	grep "^Merge tag ${apos}notyetvalid-signed${apos}" actual &&
>  	! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
>  '
>  
> @@ -178,7 +181,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign
>  	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	git checkout main &&
>  	git fetch . timeboxedvalid-signed &&
> -	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual &&
> +	grep "^Merge tag ${apos}timeboxedvalid-signed${apos}" actual &&
>  	grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
>  	! grep "${GPGSSH_BAD_SIGNATURE}" actual
>  '
> @@ -187,7 +191,8 @@ test_expect_success GPGSSH,GPGSSH_VERIFYTIME 'message for merging local tag sign
>  	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	git checkout main &&
>  	git fetch . timeboxedinvalid-signed &&
> -	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
> +	git fmt-merge-msg <.git/FETCH_HEAD >actual &&
> +	grep "^Merge tag ${apos}timeboxedinvalid-signed${apos}" actual &&
>  	! grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual
>  '