These tests fail here under Mac OS,
they pass under Linux:
commit ff3d9b660a4b6e9d3eeb664ce1febe717adff737
I haven't had a chance to dig further.
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
--- 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- 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
#
From: Jeff King <hidden> Date: 2016-06-16 02:19:15
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:
@@ -385,7 +383,7 @@ test_expect_success 'check %(if)...%(then)...%(else)...%(end) atoms' '' test_expect_success'ignore spaces in %(if) atom usage''gitfor-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:Headrefside:NotHeadrefodd/spot:NotHeadref
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:
@@ -6,12 +6,8 @@ test_description='test for-each-refs usage of ref-filter APIs' ."$TEST_DIRECTORY"/lib-gpg.sh test_prepare_expect(){-iftest_have_prereqGPG-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' 'gittag-m"Annonated doubly"doubly-annotated-tagannotated-tag&&iftest_have_prereqGPGthen-gittag-s-m"A signed tag"signed-tag&&-gittag-s-m"Signed doubly"doubly-signed-tagsigned-tag+sign=-s+else+sign=fi&&+gittag$sign-m"A signed tag"signed-tag&&+gittag$sign-m"Signed doubly"doubly-signed-tagsigned-tag&&gitcheckoutmaster&&gitupdate-refrefs/odd/spotmaster'-Peff
From: Eric Sunshine <hidden> Date: 2016-06-16 02:19:16
On Mon, May 9, 2016 at 12:07 PM, Jeff King [off-list ref] wrote:
quoted hunk
On Sat, May 07, 2016 at 06:15:19PM +0200, Torsten Bögershausen wrote:
quoted
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 '
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.)
From: Jeff King <hidden> Date: 2016-06-16 02:19:16
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.)
I agree that is nicer, but I wanted to keep the definition inside the
test_expect_success close to its point of use. And that means we to deal
with the existing &&-chain (you can get around it with a {} block, but
at that point you might as well if/then).
Since you as the author of 618310a seem to agree with this direction,
here it is as a real patch.
-- >8 --
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 <redacted>
---
t/t6302-for-each-ref-filter.sh | 45 +++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 23 deletions(-)
@@ -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(){-iftest_have_prereqGPG-then-cat-else-sed'/signed/d'-fi-}- test_expect_success'setup some history and refs''test_commitone&&test_committwo&&
@@ -22,11 +13,19 @@ test_expect_success 'setup some history and refs' 'test_commitfour&&gittag-m"An annotated tag"annotated-tag&&gittag-m"Annonated doubly"doubly-annotated-tagannotated-tag&&++# Note that these "signed" tags might not actually be signed.+# Tests which care about the distinction should be marked+# with the GPG prereq.iftest_have_prereqGPGthen-gittag-s-m"A signed tag"signed-tag&&-gittag-s-m"Signed doubly"doubly-signed-tagsigned-tag+sign=-s+else+sign=fi&&+gittag$sign-m"A signed tag"signed-tag&&+gittag$sign-m"Signed doubly"doubly-signed-tagsigned-tag&&+gitcheckoutmaster&&gitupdate-refrefs/odd/spotmaster'
@@ -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/sideZrefs/tags/annotated-tagfourrefs/tags/fourZ
@@ -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/siderefs/tags/annotated-tagrefs/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/masterrefs/heads/siderefs/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&&refnameisrefs/heads/master|refs/heads/masterrefnameisrefs/heads/side|refs/heads/siderefnameisrefs/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 '|
From: Eric Sunshine <hidden> Date: 2016-06-16 02:19:16
On Mon, May 9, 2016 at 12:49 PM, Jeff King [off-list ref] 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
618310a (t6302: skip only signed tags rather than all tests when GPG
is missing, 2016-03-06)
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).
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)?
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 <redacted>
Looks good. With or without the minor change below, this patch is:
Reviewed-by: Eric Sunshine [off-list ref]
quoted hunk
---
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 &&
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 &&
refs/heads/side Z
refs/tags/annotated-tag four
refs/tags/four Z
From: Jeff King <hidden> Date: 2016-06-16 02:19:16
[+cc Junio as this should be the final version]
On Mon, May 09, 2016 at 01:47:35PM -0400, Eric Sunshine wrote:
quoted
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)
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. If I were writing
a book, I would probably footnote a case like this (to give extra
context if somebody wants it, but not break the flow of the text). But
"618310a" is already a footnote of sorts. So I dunno.
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)?
Nope, as Karthik mentioned, we don't know the sha1 of that commit yet.
:(
Looks good. With or without the minor change below, this patch is:
Reviewed-by: Eric Sunshine [off-list ref]
Thanks.
quoted
- 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.
-- >8 --
Subject: 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.
Reviewed-by: Eric Sunshine <redacted>
Signed-off-by: Jeff King <redacted>
---
t/t6302-for-each-ref-filter.sh | 45 +++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 23 deletions(-)
@@ -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(){-iftest_have_prereqGPG-then-cat-else-sed'/signed/d'-fi-}- test_expect_success'setup some history and refs''test_commitone&&test_committwo&&
@@ -22,11 +13,19 @@ test_expect_success 'setup some history and refs' 'test_commitfour&&gittag-m"An annotated tag"annotated-tag&&gittag-m"Annonated doubly"doubly-annotated-tagannotated-tag&&++# Note that these "signed" tags might not actually be signed.+# Tests which care about the distinction should be marked+# with the GPG prereq.iftest_have_prereqGPGthen-gittag-s-m"A signed tag"signed-tag&&-gittag-s-m"Signed doubly"doubly-signed-tagsigned-tag+sign=-s+else+sign=fi&&+gittag$sign-m"A signed tag"signed-tag&&+gittag$sign-m"Signed doubly"doubly-signed-tagsigned-tag&&+gitcheckoutmaster&&gitupdate-refrefs/odd/spotmaster'
@@ -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/sideZrefs/tags/annotated-tagfourrefs/tags/fourZ
@@ -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/siderefs/tags/annotated-tagrefs/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/masterrefs/heads/siderefs/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&&refnameisrefs/heads/master|refs/heads/masterrefnameisrefs/heads/side|refs/heads/siderefnameisrefs/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 '|
From: Eric Sunshine <hidden> Date: 2016-06-16 02:19:16
On Mon, May 9, 2016 at 10:40 PM, Jeff King [off-list ref] wrote:
On Mon, May 09, 2016 at 01:47:35PM -0400, Eric Sunshine wrote:
quoted
quoted
- 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.