Re: [PATCH v3 2/4] t/t7030-verify-tag.sh: Adds validation for multiple tags
- From
Jeff King <peff@peff.net>
- Date
- Apr 3, 2016, 04:40 UTC
- Message-ID
- <20160403044054.GB1519@sigill.intra.peff.net>
- In-Reply-To
- <1459638975-17705-3-git-send-email-santiago@nyu.edu>
On Sat, Apr 02, 2016 at 07:16:13PM -0400, santiago@nyu.edu wrote:
> The verify-tag command supports mutliple tag names as an argument.
s/mutliple/multiple/
> +test_expect_success GPG 'verify multiple tags' ' > + git verify-tag -v --raw fourth-signed sixth-signed seventh-signed 2>actual 1> tagnames &&
Style: we don't put a space between ">" and the filename. Also, we usually omit "1" when redirecting stdout.
> + grep -c "GOODSIG" actual > count &&
Funny indentation here.
I wondered if we could use test_cmp instead of a counting grep here, but this is looking at gpg spew, and we probably don't want to count on that never changing.
I don't see us actually verifying that "count" is 3, though.
> + ! grep "BADSIG" actual &&
Makes sense...
> + grep -E "tag\ .*" tagnames | uniq -u | wc - l | grep "3"
Do we need "grep -E" here? I don't see any extended regex in use. Is there a reason to backslash-escape the space?
Your "wc -l" has an extra space, which means "read stdin, and then the file 'l'". Which sort-of happens to work, except as you noticed, you have to grep for "3" instead of matching it.
I think, though, that rather than counting we could just write what we expect into a file and compare that. It makes it easier for somebody reading the test to see what it is we're trying to do.
In fact, I suspect you could replace the "GOODSIG" check as well by doing something like:
# verifying 3 tags in one invocation should be exactly like
# verifying the 3 separately
tags="fourth-signed sixth-signed seventh-signed"
for i in $tags; do
git verify-tag -v --raw $i || return 1
done >expect.stdout 2>expect.stderr &&
git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr &&
test_cmp expect.stdout actual.stdout &&
test_cmp expect.stderr actual.stderrbut I didn't test it.
-Peff