On Thu, Mar 23, 2017 at 03:00:08PM -0700, Junio C Hamano wrote:
> Santiago Torres <santiago@nyu.edu> writes:
 
> OK, so has everybody agreed what the next step would be? 

I believe it is, although I imagine getting a confirmation from Peff
would be adequate.

> Is the patch below a good first step (I still need to get it signed
> off)?

I'm adding a signoff to the patch below.

Thanks,
-Santiago

-- >8 --
Subject: t7004, t7030: fix here-doc syntax errors
From: Santiago Torres <santiago@nyu.edu>

Jan Palus noticed that some here-doc are spelled incorrectly,
resulting the entire remainder of the test as if it were data
slurped into the "expect" file, e.g. in this sequence

	cat >expect <<EOF &&
	... expectation ...
	EOF
	git $cmd_being_tested >actual &&
	test_cmp expect actual

the last command of the test is "cat" that sends everything to
'expect' and succeeds.

Fixing these issues in t7004 and t7030 reveals that "git tag -v"
and "git verify-tag" with their --format option do not work as the
test was expecting originally.  Instead of showing both valid tags
and tags with incorrect signatures on their output, tags that do not
pass verification are omitted from the output.

Arguably, that is a safer behaviour, and because the format
specifiers like %(tag) do not have a way to show if the signature
verifies correctly, the command with the --format option cannot be
used to get a list of tags annotated with their signature validity
anyway.

For now, let's fix the here-doc syntax and update the expectation to
match the reality.  Maybe later when we extend the --format language
available to "git tag -v" and "git verify-tag" to include things
like "%(gpg:status)", we may want to change the behaviour so that
piping a list of tag names into

    xargs git verify-tag --format='%(gpg:status) %(tag)'

becomes a good way to produce such a list, but that is a separate
topic.

Signed-off-by: Santiago Torres <santiago@nyu.edu>
Noticed-by: Jan Palus <jan.palus@gmail.com>
Helped-by: Jeff King <peff@peff.net>
---
 t/t7004-tag.sh        | 10 ++++------
 t/t7030-verify-tag.sh | 10 ++++------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index b4698ab5f5..0581053a06 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -896,17 +896,15 @@ test_expect_success GPG 'verifying a forged tag should fail' '
 '
 
 test_expect_success 'verifying a proper tag with --format pass and format accordingly' '
-	cat >expect <<-\EOF
+	cat >expect <<-\EOF &&
 	tagname : signed-tag
-	EOF &&
+	EOF
 	git tag -v --format="tagname : %(tag)" "signed-tag" >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
-	cat >expect <<-\EOF
-	tagname : forged-tag
-	EOF &&
+test_expect_success 'verifying a forged tag with --format should fail silently' '
+	>expect &&
 	test_must_fail git tag -v --format="tagname : %(tag)" "forged-tag" >actual &&
 	test_cmp expect actual
 '
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index d62ccbb98e..79864a3411 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -126,17 +126,15 @@ test_expect_success GPG 'verify multiple tags' '
 '
 
 test_expect_success 'verifying tag with --format' '
-	cat >expect <<-\EOF
+	cat >expect <<-\EOF &&
 	tagname : fourth-signed
-	EOF &&
+	EOF
 	git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success 'verifying a forged tag with --format fail and format accordingly' '
-	cat >expect <<-\EOF
-	tagname : 7th forged-signed
-	EOF &&
+test_expect_success 'verifying a forged tag with --format should fail silently' '
+	>expect &&
 	test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged &&
 	test_cmp expect actual-forged
 '