* t5607 fail with GIT_TEST_FAIL_PREREQS enabled
@ 2021-08-11 13:02 Son Luong Ngoc
2021-08-11 14:03 ` Jeff King
0 siblings, 1 reply; 5+ messages in thread
From: Son Luong Ngoc @ 2021-08-11 13:02 UTC (permalink / raw)
To: git; +Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano
Hi folks,
Our internal CI spotted a failing test when build from 'next' and
'master' branch
git/t% GIT_TEST_FAIL_PREREQS=1 ./t5607-clone-bundle.sh
ok 1 - setup
ok 2 - "verify" needs a worktree
ok 3 - annotated tags can be excluded by rev-list options
ok 4 - die if bundle file cannot be created
ok 5 - bundle --stdin
ok 6 - bundle --stdin <rev-list options>
ok 7 - empty bundle file is rejected
not ok 8 - ridiculously long subject in boundary
#
# >file4 &&
# test_tick &&
# git add file4 &&
# printf "%01200d\n" 0 | git commit -F - &&
# test_commit fifth &&
# git bundle create long-subject-bundle.bdl HEAD^..HEAD &&
# cat >expect <<-EOF &&
# $(git rev-parse main) HEAD
# EOF
# git bundle list-heads long-subject-bundle.bdl >actual &&
# test_cmp expect actual &&
#
# git fetch long-subject-bundle.bdl &&
#
# if ! test_have_prereq SHA1
# then
# echo "@object-format=sha256"
# fi >expect &&
# cat >>expect <<-EOF &&
# -$(git log --pretty=format:"%H %s" -1 HEAD^)
# $(git rev-parse HEAD) HEAD
# EOF
#
# if test_have_prereq SHA1
# then
# head -n 3 long-subject-bundle.bdl
# else
# head -n 4 long-subject-bundle.bdl
# fi | grep -v "^#" >actual &&
#
# test_cmp expect actual
#
ok 9 - prerequisites with an empty commit message
ok 10 - failed bundle creation does not leave cruft
ok 11 - fetch SHA-1 from bundle
ok 12 - git bundle uses expected default format
ok 13 - git bundle v3 has expected contents
ok 14 - git bundle v3 rejects unknown capabilities
# failed 1 among 14 test(s)
1..14
Cheers,
Son Luong.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: t5607 fail with GIT_TEST_FAIL_PREREQS enabled
2021-08-11 13:02 t5607 fail with GIT_TEST_FAIL_PREREQS enabled Son Luong Ngoc
@ 2021-08-11 14:03 ` Jeff King
2021-08-11 23:14 ` brian m. carlson
0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2021-08-11 14:03 UTC (permalink / raw)
To: Son Luong Ngoc
Cc: git, Ævar Arnfjörð Bjarmason, Junio C Hamano
On Wed, Aug 11, 2021 at 03:02:52PM +0200, Son Luong Ngoc wrote:
> git/t% GIT_TEST_FAIL_PREREQS=1 ./t5607-clone-bundle.sh
> [...]
> # if ! test_have_prereq SHA1
> # then
> # echo "@object-format=sha256"
> # fi >expect &&
The problem is presumably here. If test_have_prereq lies and say "no, we
are using sha256" then we cannot expect what the built binary does to
match that lie.
Perhaps that is a sign that test_have_prereq is not the right tool to
check "which hash format are we using", but I don't think we have
another convenient mechanism to do so currently.
I also think that the FAIL_PREREQS system may be mis-designed a bit. We
had a similar problem a few months ago, and I think Junio's response
here points in a good direction:
https://lore.kernel.org/git/xmqqblbgrwkg.fsf@gitster.g/
-Peff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: t5607 fail with GIT_TEST_FAIL_PREREQS enabled
2021-08-11 14:03 ` Jeff King
@ 2021-08-11 23:14 ` brian m. carlson
2021-08-11 23:16 ` [PATCH v2] t5607: avoid using prerequisites to select algorithm brian m. carlson
0 siblings, 1 reply; 5+ messages in thread
From: brian m. carlson @ 2021-08-11 23:14 UTC (permalink / raw)
To: Jeff King
Cc: Son Luong Ngoc, git, Ævar Arnfjörð Bjarmason,
Junio C Hamano
[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]
On 2021-08-11 at 14:03:25, Jeff King wrote:
> On Wed, Aug 11, 2021 at 03:02:52PM +0200, Son Luong Ngoc wrote:
>
> > git/t% GIT_TEST_FAIL_PREREQS=1 ./t5607-clone-bundle.sh
> > [...]
> > # if ! test_have_prereq SHA1
> > # then
> > # echo "@object-format=sha256"
> > # fi >expect &&
>
> The problem is presumably here. If test_have_prereq lies and say "no, we
> are using sha256" then we cannot expect what the built binary does to
> match that lie.
>
> Perhaps that is a sign that test_have_prereq is not the right tool to
> check "which hash format are we using", but I don't think we have
> another convenient mechanism to do so currently.
We can use something like this:
if "$(test_oid algo)" != sha1
> I also think that the FAIL_PREREQS system may be mis-designed a bit. We
> had a similar problem a few months ago, and I think Junio's response
> here points in a good direction:
>
> https://lore.kernel.org/git/xmqqblbgrwkg.fsf@gitster.g/
I take no position on this, but I'll send a patch to do something
similar to the above in a few minutes in case someone feels like picking
it up.
--
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] t5607: avoid using prerequisites to select algorithm
2021-08-11 23:14 ` brian m. carlson
@ 2021-08-11 23:16 ` brian m. carlson
2021-08-11 23:30 ` Jeff King
0 siblings, 1 reply; 5+ messages in thread
From: brian m. carlson @ 2021-08-11 23:16 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason,
Son Luong Ngoc
In this test, we currently use the SHA1 prerequisite to specify the
algorithm we're using to test, since SHA-256 bundles are always v3,
whereas SHA-1 bundles default to v2, and as a result the default output
differs.
However, this causes a problem if we run with GIT_TEST_FAIL_PREREQS set,
since that means that we'll unexpectedly fail the SHA1 prerequisite,
resulting in incorrect expected output. Let's fix this by checking
against the built-in data called "algo", which tells us which algorithm
is in use. This should work in any situation, making our test a little
more robust.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
t/t5607-clone-bundle.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh
index ed0d911e95..51705aa86a 100755
--- a/t/t5607-clone-bundle.sh
+++ b/t/t5607-clone-bundle.sh
@@ -91,7 +91,8 @@ test_expect_success 'ridiculously long subject in boundary' '
git fetch long-subject-bundle.bdl &&
- if ! test_have_prereq SHA1
+ algo=$(test_oid algo) &&
+ if test "$algo" != sha1
then
echo "@object-format=sha256"
fi >expect &&
@@ -100,7 +101,7 @@ test_expect_success 'ridiculously long subject in boundary' '
$(git rev-parse HEAD) HEAD
EOF
- if test_have_prereq SHA1
+ if test "$algo" = sha1
then
head -n 3 long-subject-bundle.bdl
else
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] t5607: avoid using prerequisites to select algorithm
2021-08-11 23:16 ` [PATCH v2] t5607: avoid using prerequisites to select algorithm brian m. carlson
@ 2021-08-11 23:30 ` Jeff King
0 siblings, 0 replies; 5+ messages in thread
From: Jeff King @ 2021-08-11 23:30 UTC (permalink / raw)
To: brian m. carlson
Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason,
Son Luong Ngoc
On Wed, Aug 11, 2021 at 11:16:44PM +0000, brian m. carlson wrote:
> In this test, we currently use the SHA1 prerequisite to specify the
> algorithm we're using to test, since SHA-256 bundles are always v3,
> whereas SHA-1 bundles default to v2, and as a result the default output
> differs.
>
> However, this causes a problem if we run with GIT_TEST_FAIL_PREREQS set,
> since that means that we'll unexpectedly fail the SHA1 prerequisite,
> resulting in incorrect expected output. Let's fix this by checking
> against the built-in data called "algo", which tells us which algorithm
> is in use. This should work in any situation, making our test a little
> more robust.
Thanks, this seems like a reasonable step, and fixes the test for me.
I still get tons of other failures because I set GIT_TEST_HTTPD=yes,
which implies to me we should still be fixing GIT_TEST_FAIL_PREREQS. But
I am happy to take this in the meantime for people who do care.
-Peff
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-11 23:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11 13:02 t5607 fail with GIT_TEST_FAIL_PREREQS enabled Son Luong Ngoc
2021-08-11 14:03 ` Jeff King
2021-08-11 23:14 ` brian m. carlson
2021-08-11 23:16 ` [PATCH v2] t5607: avoid using prerequisites to select algorithm brian m. carlson
2021-08-11 23:30 ` Jeff King
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).