t6392 broken on pu (Mac OS X)

Torsten Bögershausen· May 7, 2016, 16:15 UTC · lore

expecting success:
	git for-each-ref --format="%(if)%(authorname)%(then)%(authorname):
%(refname)%(end)" >actual &&
	cat >expect <<-\EOF &&
	A U Thor: refs/heads/master
	A U Thor: refs/heads/side
	A U Thor: refs/odd/spot

	A U Thor: refs/tags/foo1.10
	A U Thor: refs/tags/foo1.3
	A U Thor: refs/tags/foo1.6
	A U Thor: refs/tags/four
	A U Thor: refs/tags/one

	A U Thor: refs/tags/three
	A U Thor: refs/tags/two
	EOF
	test_cmp expect actual

@@ -3,12 +3,10 @@
 A U Thor: refs/odd/spot


-
 A U Thor: refs/tags/foo1.10
 A U Thor: refs/tags/foo1.3
 A U Thor: refs/tags/foo1.6
 A U Thor: refs/tags/four
 A U Thor: refs/tags/one
-
 A U Thor: refs/tags/three
 A U Thor: refs/tags/two
not ok 34 - check %(if)...%(then)...%(end) atoms
#	
#		git for-each-ref --format="%(if)%(authorname)%(then)%(authorname):
%(refname)%(end)" >actual &&
#		cat >expect <<-\EOF &&
#		A U Thor: refs/heads/master
#		A U Thor: refs/heads/side
#		A U Thor: refs/odd/spot
#	
#	
#	
#		A U Thor: refs/tags/foo1.10
#		A U Thor: refs/tags/foo1.3
#		A U Thor: refs/tags/foo1.6
#		A U Thor: refs/tags/four
#		A U Thor: refs/tags/one
#	
#		A U Thor: refs/tags/three
#		A U Thor: refs/tags/two
#		EOF
#		test_cmp expect actual
#	

expecting success:
	git for-each-ref --format="%(if)%(authorname)%(then)%(authorname)%(else)No
author%(end): %(refname)" >actual &&
	cat >expect <<-\EOF &&
	A U Thor: refs/heads/master
	A U Thor: refs/heads/side
	A U Thor: refs/odd/spot
	No author: refs/tags/annotated-tag
	No author: refs/tags/doubly-annotated-tag
	No author: refs/tags/doubly-signed-tag
	A U Thor: refs/tags/foo1.10
	A U Thor: refs/tags/foo1.3
	A U Thor: refs/tags/foo1.6
	A U Thor: refs/tags/four
	A U Thor: refs/tags/one
	No author: refs/tags/signed-tag
	A U Thor: refs/tags/three
	A U Thor: refs/tags/two
	EOF
	test_cmp expect actual

--- expect	2016-05-07 16:08:32.000000000 +0000
+++ actual	2016-05-07 16:08:32.000000000 +0000
@@ -3,12 +3,10 @@
 A U Thor: refs/odd/spot
 No author: refs/tags/annotated-tag
 No author: refs/tags/doubly-annotated-tag
-No author: refs/tags/doubly-signed-tag
 A U Thor: refs/tags/foo1.10
 A U Thor: refs/tags/foo1.3
 A U Thor: refs/tags/foo1.6
 A U Thor: refs/tags/four
 A U Thor: refs/tags/one
-No author: refs/tags/signed-tag
 A U Thor: refs/tags/three
 A U Thor: refs/tags/two
not ok 35 - check %(if)...%(then)...%(else)...%(end) atoms
#	
#		git for-each-ref --format="%(if)%(authorname)%(then)%(authorname)%(else)No
author%(end): %(refname)" >actual &&
#		cat >expect <<-\EOF &&
#		A U Thor: refs/heads/master
#		A U Thor: refs/heads/side
#		A U Thor: refs/odd/spot
#		No author: refs/tags/annotated-tag
#		No author: refs/tags/doubly-annotated-tag
#		No author: refs/tags/doubly-signed-tag
#		A U Thor: refs/tags/foo1.10
#		A U Thor: refs/tags/foo1.3
#		A U Thor: refs/tags/foo1.6
#		A U Thor: refs/tags/four
#		A U Thor: refs/tags/one
#		No author: refs/tags/signed-tag
#		A U Thor: refs/tags/three
#		A U Thor: refs/tags/two
#		EOF
#		test_cmp expect actual
#	

expecting success:
	git for-each-ref --format="%(refname:short): %(if)%(HEAD)%(then)Head
ref%(else)Not Head ref%(end)" >actual &&
	cat >expect <<-\EOF &&
	master: Head ref
	side: Not Head ref
	odd/spot: Not Head ref
	annotated-tag: Not Head ref
	doubly-annotated-tag: Not Head ref
	doubly-signed-tag: Not Head ref
	foo1.10: Not Head ref
	foo1.3: Not Head ref
	foo1.6: Not Head ref
	four: Not Head ref
	one: Not Head ref
	signed-tag: Not Head ref
	three: Not Head ref
	two: Not Head ref
	EOF
	test_cmp expect actual

--- expect	2016-05-07 16:08:32.000000000 +0000
+++ actual	2016-05-07 16:08:32.000000000 +0000
@@ -3,12 +3,10 @@
 odd/spot: Not Head ref
 annotated-tag: Not Head ref
 doubly-annotated-tag: Not Head ref
-doubly-signed-tag: Not Head ref
 foo1.10: Not Head ref
 foo1.3: Not Head ref
 foo1.6: Not Head ref
 four: Not Head ref
 one: Not Head ref
-signed-tag: Not Head ref
 three: Not Head ref
 two: Not Head ref
not ok 36 - ignore spaces in %(if) atom usage
#	
#		git for-each-ref --format="%(refname:short): %(if)%(HEAD)%(then)Head
ref%(else)Not Head ref%(end)" >actual &&
#		cat >expect <<-\EOF &&
#		master: Head ref
#		side: Not Head ref
#		odd/spot: Not Head ref
#		annotated-tag: Not Head ref
#		doubly-annotated-tag: Not Head ref
#		doubly-signed-tag: Not Head ref
#		foo1.10: Not Head ref
#		foo1.3: Not Head ref
#		foo1.6: Not Head ref
#		four: Not Head ref
#		one: Not Head ref
#		signed-tag: Not Head ref
#		three: Not Head ref
#		two: Not Head ref
#		EOF
#		test_cmp expect actual
#	

Jeff King· May 9, 2016, 16:07 UTC · re: Torsten Bögershausen · lore

Re: t6392 broken on pu (Mac OS X)

On Sat, May 07, 2016 at 06:15:19PM +0200, Torsten Bögershausen wrote:

> These tests fail here under Mac OS,
> they pass under Linux:
> commit ff3d9b660a4b6e9d3eeb664ce1febe717adff737
> I haven't had a chance to dig further.

We could fix it by tweaking the test like this:

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 7420e48..04042e1 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -343,29 +343,27 @@ test_expect_success 'improper usage of %(if), %(then), %(else) and %(end) atoms'
 '
 
 test_expect_success 'check %(if)...%(then)...%(end) atoms' '
-	git for-each-ref --format="%(if)%(authorname)%(then)%(authorname): %(refname)%(end)" >actual &&
-	cat >expect <<-\EOF &&
-	A U Thor: refs/heads/master
-	A U Thor: refs/heads/side
-	A U Thor: refs/odd/spot
-
-
-
-	A U Thor: refs/tags/foo1.10
-	A U Thor: refs/tags/foo1.3
-	A U Thor: refs/tags/foo1.6
-	A U Thor: refs/tags/four
-	A U Thor: refs/tags/one
-
-	A U Thor: refs/tags/three
-	A U Thor: refs/tags/two
+	git for-each-ref --format="%(refname):%(if)%(authorname)%(then) author=%(authorname)%(end)" >actual &&
+	test_prepare_expect >expect <<-\EOF &&
+	refs/heads/master: author=A U Thor
+	refs/heads/side: author=A U Thor
+	refs/odd/spot: author=A U Thor
+	refs/tags/annotated-tag:
+	refs/tags/doubly-annotated-tag:
+	refs/tags/foo1.10: author=A U Thor
+	refs/tags/foo1.3: author=A U Thor
+	refs/tags/foo1.6: author=A U Thor
+	refs/tags/four: author=A U Thor
+	refs/tags/one: author=A U Thor
+	refs/tags/three: author=A U Thor
+	refs/tags/two: author=A U Thor
 	EOF
 	test_cmp expect actual
 '
 
 test_expect_success 'check %(if)...%(then)...%(else)...%(end) atoms' '
 	git for-each-ref --format="%(if)%(authorname)%(then)%(authorname)%(else)No author%(end): %(refname)" >actual &&
-	cat >expect <<-\EOF &&
+	test_prepare_expect >expect <<-\EOF &&
 	A U Thor: refs/heads/master
 	A U Thor: refs/heads/side
 	A U Thor: refs/odd/spot
@@ -385,7 +383,7 @@ test_expect_success 'check %(if)...%(then)...%(else)...%(end) atoms' '
 '
 test_expect_success 'ignore spaces in %(if) atom usage' '
 	git for-each-ref --format="%(refname:short): %(if)%(HEAD)%(then)Head ref%(else)Not Head ref%(end)" >actual &&
-	cat >expect <<-\EOF &&
+	test_prepare_expect >expect <<-\EOF &&
 	master: Head ref
 	side: Not Head ref
 	odd/spot: Not Head ref


Though we'd perhaps want to tweak the subsequent tests to use the same
format, just to make things easier to read later.

However, I wonder if we could improve on the strategy in 618310a, and
simply create non-signed versions of the "signed" tags when GPG is not
available. That would make tests looking at the whole ref namespace
more consistent. And any tests which wanted to look specifically at the
signed attributes should be protected with the GPG prereq anyway (it
doesn't look like there are any currently, though).

I.e., something like:

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 7420e48..a3df472 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -6,12 +6,8 @@ test_description='test for-each-refs usage of ref-filter APIs'
 . "$TEST_DIRECTORY"/lib-gpg.sh
 
 test_prepare_expect () {
-	if test_have_prereq GPG
-	then
-		cat
-	else
-		sed '/signed/d'
-	fi
+	# XXX this could now go away entirely, and just use cat in each test
+	cat
 }
 
 test_expect_success 'setup some history and refs' '
@@ -24,9 +20,12 @@ test_expect_success 'setup some history and refs' '
 	git tag -m "Annonated doubly" doubly-annotated-tag annotated-tag &&
 	if test_have_prereq GPG
 	then
-		git tag -s -m "A signed tag" signed-tag &&
-		git tag -s -m "Signed doubly" doubly-signed-tag signed-tag
+		sign=-s
+	else
+		sign=
 	fi &&
+	git tag $sign -m "A signed tag" signed-tag &&
+	git tag $sign -m "Signed doubly" doubly-signed-tag signed-tag &&
 	git checkout master &&
 	git update-ref refs/odd/spot master
 '

-Peff

Eric Sunshine· May 9, 2016, 16:30 UTC · re: Jeff King · lore

Re: t6392 broken on pu (Mac OS X)

On Mon, May 9, 2016 at 12:07 PM, Jeff King <peff@peff.net> wrote:

> On Sat, May 07, 2016 at 06:15:19PM +0200, Torsten Bögershausen wrote:
>> These tests fail here under Mac OS,
>> they pass under Linux:
>> commit ff3d9b660a4b6e9d3eeb664ce1febe717adff737
>> I haven't had a chance to dig further.
>
> I assume you mean t6302. It looks like the difference is not Mac OS, but
> rather that the GPG prerequisite is not fulfilled, so we are missing a
> few of the tags.
>
> Commit 618310a introduced a helper to munge the "expect" output. Using
> that fixes some of the cases, but not test 34. That one is expecting
> blank lines for tags, so test_prepare_expect doesn't know which lines
> are related to GPG.
>
> We could fix it by tweaking the test like this:
> [...snip...]
> However, I wonder if we could improve on the strategy in 618310a, and
> simply create non-signed versions of the "signed" tags when GPG is not
> available. That would make tests looking at the whole ref namespace
> more consistent. And any tests which wanted to look specifically at the
> signed attributes should be protected with the GPG prereq anyway (it
> doesn't look like there are any currently, though).
>
> I.e., something like:
> [...snip...]
>  test_expect_success 'setup some history and refs' '
> @@ -24,9 +20,12 @@ test_expect_success 'setup some history and refs' '
>         git tag -m "Annonated doubly" doubly-annotated-tag annotated-tag &&
>         if test_have_prereq GPG
>         then
> -               git tag -s -m "A signed tag" signed-tag &&
> -               git tag -s -m "Signed doubly" doubly-signed-tag signed-tag
> +               sign=-s
> +       else
> +               sign=
>         fi &&
> +       git tag $sign -m "A signed tag" signed-tag &&
> +       git tag $sign -m "Signed doubly" doubly-signed-tag signed-tag &&
>         git checkout master &&
>         git update-ref refs/odd/spot master
>  '

    sign=
    test_have_prereq GPG && sign=-s

(But that's a minor issue.)

Jeff King· May 9, 2016, 16:49 UTC · re: Eric Sunshine · lore

[PATCH] t6302: simplify non-gpg cases

On Mon, May 09, 2016 at 12:30:43PM -0400, Eric Sunshine wrote:

> The latter seems very preferable, though perhaps it could be made more
> concise like this?
> 
>     sign=
>     test_have_prereq GPG && sign=-s
> 
> (But that's a minor issue.)

-- >8 --
Subject: [PATCH] t6302: simplify non-gpg cases

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t6302-for-each-ref-filter.sh | 45 +++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 70afb44..3225a0b 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -5,15 +5,6 @@ test_description='test for-each-refs usage of ref-filter APIs'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 
-test_prepare_expect () {
-	if test_have_prereq GPG
-	then
-		cat
-	else
-		sed '/signed/d'
-	fi
-}
-
 test_expect_success 'setup some history and refs' '
 	test_commit one &&
 	test_commit two &&
@@ -22,11 +13,19 @@ test_expect_success 'setup some history and refs' '
 	test_commit four &&
 	git tag -m "An annotated tag" annotated-tag &&
 	git tag -m "Annonated doubly" doubly-annotated-tag annotated-tag &&
+
+	# Note that these "signed" tags might not actually be signed.
+	# Tests which care about the distinction should be marked
+	# with the GPG prereq.
 	if test_have_prereq GPG
 	then
-		git tag -s -m "A signed tag" signed-tag &&
-		git tag -s -m "Signed doubly" doubly-signed-tag signed-tag
+		sign=-s
+	else
+		sign=
 	fi &&
+	git tag $sign -m "A signed tag" signed-tag &&
+	git tag $sign -m "Signed doubly" doubly-signed-tag signed-tag &&
+
 	git checkout master &&
 	git update-ref refs/odd/spot master
 '
@@ -42,7 +41,7 @@ test_expect_success 'filtering with --points-at' '
 '
 
 test_expect_success 'check signed tags with --points-at' '
-	test_prepare_expect <<-\EOF | sed -e "s/Z$//" >expect &&
+	cat <<-\EOF | sed -e "s/Z$//" >expect &&
 	refs/heads/side Z
 	refs/tags/annotated-tag four
 	refs/tags/four Z
@@ -65,7 +64,7 @@ test_expect_success 'filtering with --merged' '
 '
 
 test_expect_success 'filtering with --no-merged' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	refs/heads/side
 	refs/tags/annotated-tag
 	refs/tags/doubly-annotated-tag
@@ -78,7 +77,7 @@ test_expect_success 'filtering with --no-merged' '
 '
 
 test_expect_success 'filtering with --contains' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	refs/heads/master
 	refs/heads/side
 	refs/odd/spot
@@ -99,7 +98,7 @@ test_expect_success '%(color) must fail' '
 '
 
 test_expect_success 'left alignment is default' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	refname is refs/heads/master  |refs/heads/master
 	refname is refs/heads/side    |refs/heads/side
 	refname is refs/odd/spot      |refs/odd/spot
@@ -117,7 +116,7 @@ test_expect_success 'left alignment is default' '
 '
 
 test_expect_success 'middle alignment' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	| refname is refs/heads/master |refs/heads/master
 	|  refname is refs/heads/side  |refs/heads/side
 	|   refname is refs/odd/spot   |refs/odd/spot
@@ -135,7 +134,7 @@ test_expect_success 'middle alignment' '
 '
 
 test_expect_success 'right alignment' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	|  refname is refs/heads/master|refs/heads/master
 	|    refname is refs/heads/side|refs/heads/side
 	|      refname is refs/odd/spot|refs/odd/spot
@@ -152,7 +151,7 @@ test_expect_success 'right alignment' '
 	test_cmp expect actual
 '
 
-test_prepare_expect >expect <<-\EOF
+cat >expect <<-\EOF
 |       refname is refs/heads/master       |refs/heads/master
 |        refname is refs/heads/side        |refs/heads/side
 |         refname is refs/odd/spot         |refs/odd/spot
@@ -199,7 +198,7 @@ EOF
 # Individual atoms inside %(align:...) and %(end) must not be quoted.
 
 test_expect_success 'alignment with format quote' "
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	|'      '\''master| A U Thor'\''      '|
 	|'       '\''side| A U Thor'\''       '|
 	|'     '\''odd/spot| A U Thor'\''     '|
@@ -217,7 +216,7 @@ test_expect_success 'alignment with format quote' "
 "
 
 test_expect_success 'nested alignment with quote formatting' "
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	|'         master               '|
 	|'           side               '|
 	|'       odd/spot               '|
@@ -235,7 +234,7 @@ test_expect_success 'nested alignment with quote formatting' "
 "
 
 test_expect_success 'check `%(contents:lines=1)`' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	master |three
 	side |four
 	odd/spot |three
@@ -253,7 +252,7 @@ test_expect_success 'check `%(contents:lines=1)`' '
 '
 
 test_expect_success 'check `%(contents:lines=0)`' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	master |
 	side |
 	odd/spot |
@@ -271,7 +270,7 @@ test_expect_success 'check `%(contents:lines=0)`' '
 '
 
 test_expect_success 'check `%(contents:lines=99999)`' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	master |three
 	side |four
 	odd/spot |three

-- 
2.8.2.643.g361a07a

Eric Sunshine· May 9, 2016, 17:47 UTC · re: Jeff King · lore

Re: [PATCH] t6302: simplify non-gpg cases

On Mon, May 9, 2016 at 12:49 PM, Jeff King <peff@peff.net> wrote:

> On Mon, May 09, 2016 at 12:30:43PM -0400, Eric Sunshine wrote:
> Since you as the author of 618310a seem to agree with this direction,
> here it is as a real patch.

Thanks for working on this.

> Subject: [PATCH] t6302: simplify non-gpg cases
>
> When commit 618310a taught t6302 to run without the GPG

> prerequisite, it did so by conditionally creating the signed
> tags only when gpg is available. As a result, further tests
> need to take this into account, which they can do with the
> test_prepare_expect helper. This is a minor hassle, though,
> as the helper cannot easily cover all cases (it just matches
> "signed" in the output, so all output must include the
> actual refname).

> Instead, let's take a different approach. We'll always
> create the tags, and only conditionally sign them. This does
> mean our tag-names are a minor lie, but it lets the tests
> which do not care about signing easily behave the same in
> all settings. We'll include a comment to document our lie
> and avoid confusing further test-writers.
>
> Signed-off-by: Jeff King <peff@peff.net>

Looks good. With or without the minor change below, this patch is:

    Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

> ---
> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
>  test_expect_success 'check signed tags with --points-at' '
> -       test_prepare_expect <<-\EOF | sed -e "s/Z$//" >expect &&
> +       cat <<-\EOF | sed -e "s/Z$//" >expect &&

    sed -e "s/Z$//" >expect <<-\EOF &&

>         refs/heads/side Z
>         refs/tags/annotated-tag four
>         refs/tags/four Z

Jeff King· May 10, 2016, 02:40 UTC · re: Eric Sunshine · lore

[PATCH v2] t6302: simplify non-gpg cases

[+cc Junio as this should be the final version]

On Mon, May 09, 2016 at 01:47:35PM -0400, Eric Sunshine wrote:

> > Subject: [PATCH] t6302: simplify non-gpg cases
> >
> > When commit 618310a taught t6302 to run without the GPG
> 
> 618310a (t6302: skip only signed tags rather than all tests when GPG
> is missing, 2016-03-06)

> Should we cite bc9acea (ref-filter: implement %(if), %(then), and
> %(else) atoms, 2016-04-25) here as an example of a commit for which
> this was problematic (and which indeed broke the tests when GPG is
> unavailable)?

> Looks good. With or without the minor change below, this patch is:
> 
>     Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>

Thanks.

> > -       test_prepare_expect <<-\EOF | sed -e "s/Z$//" >expect &&
> > +       cat <<-\EOF | sed -e "s/Z$//" >expect &&
> 
> To make this as close to a reversion as possible, this could be
> restored to the original (sans 'cat'):
> 
>     sed -e "s/Z$//" >expect <<-\EOF &&

-- >8 --
Subject: t6302: simplify non-gpg cases

Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 t/t6302-for-each-ref-filter.sh | 45 +++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index 70afb44..d0ab09f 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -5,15 +5,6 @@ test_description='test for-each-refs usage of ref-filter APIs'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh
 
-test_prepare_expect () {
-	if test_have_prereq GPG
-	then
-		cat
-	else
-		sed '/signed/d'
-	fi
-}
-
 test_expect_success 'setup some history and refs' '
 	test_commit one &&
 	test_commit two &&
@@ -22,11 +13,19 @@ test_expect_success 'setup some history and refs' '
 	test_commit four &&
 	git tag -m "An annotated tag" annotated-tag &&
 	git tag -m "Annonated doubly" doubly-annotated-tag annotated-tag &&
+
+	# Note that these "signed" tags might not actually be signed.
+	# Tests which care about the distinction should be marked
+	# with the GPG prereq.
 	if test_have_prereq GPG
 	then
-		git tag -s -m "A signed tag" signed-tag &&
-		git tag -s -m "Signed doubly" doubly-signed-tag signed-tag
+		sign=-s
+	else
+		sign=
 	fi &&
+	git tag $sign -m "A signed tag" signed-tag &&
+	git tag $sign -m "Signed doubly" doubly-signed-tag signed-tag &&
+
 	git checkout master &&
 	git update-ref refs/odd/spot master
 '
@@ -42,7 +41,7 @@ test_expect_success 'filtering with --points-at' '
 '
 
 test_expect_success 'check signed tags with --points-at' '
-	test_prepare_expect <<-\EOF | sed -e "s/Z$//" >expect &&
+	sed -e "s/Z$//" >expect <<-\EOF &&
 	refs/heads/side Z
 	refs/tags/annotated-tag four
 	refs/tags/four Z
@@ -65,7 +64,7 @@ test_expect_success 'filtering with --merged' '
 '
 
 test_expect_success 'filtering with --no-merged' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	refs/heads/side
 	refs/tags/annotated-tag
 	refs/tags/doubly-annotated-tag
@@ -78,7 +77,7 @@ test_expect_success 'filtering with --no-merged' '
 '
 
 test_expect_success 'filtering with --contains' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	refs/heads/master
 	refs/heads/side
 	refs/odd/spot
@@ -99,7 +98,7 @@ test_expect_success '%(color) must fail' '
 '
 
 test_expect_success 'left alignment is default' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	refname is refs/heads/master  |refs/heads/master
 	refname is refs/heads/side    |refs/heads/side
 	refname is refs/odd/spot      |refs/odd/spot
@@ -117,7 +116,7 @@ test_expect_success 'left alignment is default' '
 '
 
 test_expect_success 'middle alignment' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	| refname is refs/heads/master |refs/heads/master
 	|  refname is refs/heads/side  |refs/heads/side
 	|   refname is refs/odd/spot   |refs/odd/spot
@@ -135,7 +134,7 @@ test_expect_success 'middle alignment' '
 '
 
 test_expect_success 'right alignment' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	|  refname is refs/heads/master|refs/heads/master
 	|    refname is refs/heads/side|refs/heads/side
 	|      refname is refs/odd/spot|refs/odd/spot
@@ -152,7 +151,7 @@ test_expect_success 'right alignment' '
 	test_cmp expect actual
 '
 
-test_prepare_expect >expect <<-\EOF
+cat >expect <<-\EOF
 |       refname is refs/heads/master       |refs/heads/master
 |        refname is refs/heads/side        |refs/heads/side
 |         refname is refs/odd/spot         |refs/odd/spot
@@ -199,7 +198,7 @@ EOF
 # Individual atoms inside %(align:...) and %(end) must not be quoted.
 
 test_expect_success 'alignment with format quote' "
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	|'      '\''master| A U Thor'\''      '|
 	|'       '\''side| A U Thor'\''       '|
 	|'     '\''odd/spot| A U Thor'\''     '|
@@ -217,7 +216,7 @@ test_expect_success 'alignment with format quote' "
 "
 
 test_expect_success 'nested alignment with quote formatting' "
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	|'         master               '|
 	|'           side               '|
 	|'       odd/spot               '|
@@ -235,7 +234,7 @@ test_expect_success 'nested alignment with quote formatting' "
 "
 
 test_expect_success 'check `%(contents:lines=1)`' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	master |three
 	side |four
 	odd/spot |three
@@ -253,7 +252,7 @@ test_expect_success 'check `%(contents:lines=1)`' '
 '
 
 test_expect_success 'check `%(contents:lines=0)`' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	master |
 	side |
 	odd/spot |
@@ -271,7 +270,7 @@ test_expect_success 'check `%(contents:lines=0)`' '
 '
 
 test_expect_success 'check `%(contents:lines=99999)`' '
-	test_prepare_expect >expect <<-\EOF &&
+	cat >expect <<-\EOF &&
 	master |three
 	side |four
 	odd/spot |three

-- 
2.8.2.643.g361a07a

Eric Sunshine· May 10, 2016, 02:49 UTC · re: Jeff King · lore

Re: [PATCH v2] t6302: simplify non-gpg cases

On Mon, May 9, 2016 at 10:40 PM, Jeff King <peff@peff.net> wrote:

> On Mon, May 09, 2016 at 01:47:35PM -0400, Eric Sunshine wrote:
>> > -       test_prepare_expect <<-\EOF | sed -e "s/Z$//" >expect &&
>> > +       cat <<-\EOF | sed -e "s/Z$//" >expect &&
>>
>> To make this as close to a reversion as possible, this could be
>> restored to the original (sans 'cat'):
>>
>>     sed -e "s/Z$//" >expect <<-\EOF &&
>
> Thanks, I did the reversion with s/test_prepare_expect/cat/ rather than
> reverting 618310a, but I agree dropping this useless-use-of-cat is worth
> doing. Here's v2 with that change and your reviewed-by.

Looks good, thanks.

Junio C Hamano· May 10, 2016, 06:03 UTC · re: Jeff King · lore

Re: [PATCH v2] t6302: simplify non-gpg cases

Jeff King <peff@peff.net> writes:

> [+cc Junio as this should be the final version]

>> > When commit 618310a taught t6302 to run without the GPG
>> 
>> 618310a (t6302: skip only signed tags rather than all tests when GPG
>> is missing, 2016-03-06)
>
> I sometimes intentionally avoid using that longer form when the title of
> the commit does not convey what I want to communicate, and I have to
> summarize the change in my own words anyway (in this case the
> interesting thing is not _what_ it did, but _how_ it chose to do it). So
> I find including the original subject line just bloats the sentence and
> makes the point harder to find.
>
> But I'm curious whether other people run into that problem, or if
> readers would prefer an unconditional full-citation.

> Thanks, I did the reversion with s/test_prepare_expect/cat/ rather than
> reverting 618310a, but I agree dropping this useless-use-of-cat is worth
> doing. Here's v2 with that change and your reviewed-by.

Eric Sunshine· May 9, 2016, 21:37 UTC · lore

Re: [PATCH] t6302: simplify non-gpg cases

On Mon, May 9, 2016 at 4:24 PM, Karthik Nayak <karthik.188@gmail.com> wrote:

> On Mon, May 9, 2016 at 11:17 PM, Eric Sunshine <sunshine@sunshineco.com>
> wrote:
>> Should we cite bc9acea (ref-filter: implement %(if), %(then), and
>> %(else) atoms, 2016-04-25) here as an example of a commit for which
>> this was problematic (and which indeed broke the tests when GPG is
>> unavailable)?
>
> But it's still in pu and I'll be re-rolling it, would that be acceptable?