Re: [PATCH] t5534: fix misleading grep invocation
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 5, 2017, 16:26 UTC
- Message-ID
- <xmqq1spukfyw.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <936f43ce2c993e545deaee5e196c018933375214.1499254650.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 39 quoted lines
> 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.
Show 37 quoted lines
> 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