Re: [PATCH] t5534: fix misleading grep invocation

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

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> It seems to be a little-known feature of `grep` (and it certainly came
> as a surprise to this here developer who believed to know the Unix tools
> pretty well) that multiple patterns can be passed in the same
> command-line argument simply by separating them by newlines. Watch, and
> learn:
>
> 	$ printf '1\n2\n3\n' | grep "$(printf '1\n3\n')"
> 	1
> 	3
>
> That behavior also extends to patterns passed via `-e`, and it is not
> modified by passing the option `-E` (but trying this with -P issues the
> error "grep: the -P option only supports a single pattern").
>
> It seems that there are more old Unix hands who are surprised by this
> behavior, as grep invocations of the form
>
> 	grep "$(git rev-parse A B) C" file
>
> were introduced in a85b377d041 (push: the beginning of "git push
> --signed", 2014-09-12), and later faithfully copy-edited in b9459019bbb
> (push: heed user.signingkey for signed pushes, 2014-10-22).
>
> Please note that the output of `git rev-parse A B` separates the object
> IDs via *newlines*, not via spaces, and those newlines are preserved
> because the interpolation is enclosed in double quotes.
>
> As a consequence, these tests try to validate that the file contains
> either A's object ID, or B's object ID followed by C, or both. Clearly,
> however, what the test wanted to see is that there is a line that
> contains all of them.
>
> This is clearly unintended, and the grep invocations in question really
> match too many lines.
>
> Fix the test by avoiding the newlines in the patterns.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---

The invocation this fixes is not just misleading but simply wrong.
Nicely spotted.

Thanks, will queue.

>  t/t5534-push-signed.sh | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
> index 5bcb288f5c4..464ffdd147a 100755
> --- a/t/t5534-push-signed.sh
> +++ b/t/t5534-push-signed.sh
> @@ -119,8 +119,11 @@ test_expect_success GPG 'signed push sends push certificate' '
>  		sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
>  	) >expect &&
>  
> -	grep "$(git rev-parse noop ff) refs/heads/ff" dst/push-cert &&
> -	grep "$(git rev-parse noop noff) refs/heads/noff" dst/push-cert &&
> +	noop=$(git rev-parse noop) &&
> +	ff=$(git rev-parse ff) &&
> +	noff=$(git rev-parse noff) &&
> +	grep "$noop $ff refs/heads/ff" dst/push-cert &&
> +	grep "$noop $noff refs/heads/noff" dst/push-cert &&
>  	test_cmp expect dst/push-cert-status
>  '
>  
> @@ -200,8 +203,11 @@ test_expect_success GPG 'fail without key and heed user.signingkey' '
>  		sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
>  	) >expect &&
>  
> -	grep "$(git rev-parse noop ff) refs/heads/ff" dst/push-cert &&
> -	grep "$(git rev-parse noop noff) refs/heads/noff" dst/push-cert &&
> +	noop=$(git rev-parse noop) &&
> +	ff=$(git rev-parse ff) &&
> +	noff=$(git rev-parse noff) &&
> +	grep "$noop $ff refs/heads/ff" dst/push-cert &&
> +	grep "$noop $noff refs/heads/noff" dst/push-cert &&
>  	test_cmp expect dst/push-cert-status
>  '
>  
>
> base-commit: 5116f791c12dda6b6c22fa85b600a8e30dfa168a