[PATCH 0/6] mktag tests: test more exhaustively

[PATCH 0/6] mktag tests: test more exhaustively

[Ævar Arnfjörð Bjarmason]

In [1] I suggested that we could catch the segfault fixed in
c6854508808 (ref-filter: fix NULL check for parse object failure,
2021-04-01) (and possibly other future bugs) my extending the mktag
tests.

Junio wanted the more isolated bug fix first[2], which is fair
enough. But now that the dust has settled I figured I'd submit this
test-only series which stresses mktag, fsck and various commands that
might deal with the broken objects the mktag tests creates.

1. https://lore.kernel.org/git/cover-0.6-00000000000-20210401T135419Z-avarab@gmail.com/
2. https://lore.kernel.org/git/xmqqeefs4dge.fsf@gitster.g/

Ævar Arnfjörð Bjarmason (6):
  mktag tests: parse out options in helper
  mktag tests: invert --no-strict test
  mktag tests: test hash-object --literally and unreachable fsck
  mktag tests: test update-ref and reachable fsck
  mktag tests: test for-each-ref
  mktag tests: test fast-export

 t/t3800-mktag.sh | 112 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 97 insertions(+), 15 deletions(-)

-- 
2.32.0.555.g0268d380f7b

[PATCH 1/6] mktag tests: parse out options in helper

[Ævar Arnfjörð Bjarmason]

Change check_verify_failure() helper to parse out options from
$@. This makes it easier to add new options in the future. See
06ce79152be (mktag: add a --[no-]strict option, 2021-01-06) for the
initial implementation.

Let's also replace "" quotes with '' for the test body, the varables
we need are eval'd into the body, so there's no need for the quoting
confusion.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 43 +++++++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 6275c98523f..e9008744e3d 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -12,15 +12,29 @@ test_description='git mktag: tag object verify test'
 # given in the expect.pat file.
 
 check_verify_failure () {
-	test_expect_success "$1" "
-		test_must_fail git mktag <tag.sig 2>message &&
-		grep '$2' message &&
-		if test '$3' != '--no-strict'
+	subject=$1 &&
+	message=$2 &&
+	shift 2 &&
+
+	no_strict= &&
+	while test $# != 0
+	do
+		case "$1" in
+		--no-strict)
+			no_strict=yes
+			;;
+		esac &&
+		shift
+	done &&
+
+	test_expect_success "fail with [--[no-]strict]: $subject" '
+		test_must_fail git mktag <tag.sig 2>err &&
+		if test -z "$no_strict"
 		then
-			test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict &&
-			grep '$2' message.no-strict
+			test_must_fail git mktag <tag.sig 2>err2 &&
+			test_cmp err err2
 		fi
-	"
+	'
 }
 
 test_expect_mktag_success() {
@@ -243,7 +257,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify tag-name check' \
-	'^error:.* badTagName:' '--no-strict'
+	'^error:.* badTagName:' \
+	--no-strict
 
 ############################################################
 # 11. tagger line label check #1
@@ -257,7 +272,8 @@ This is filler
 EOF
 
 check_verify_failure '"tagger" line label check #1' \
-	'^error:.* missingTaggerEntry:' '--no-strict'
+	'^error:.* missingTaggerEntry:' \
+	--no-strict
 
 ############################################################
 # 12. tagger line label check #2
@@ -272,7 +288,8 @@ This is filler
 EOF
 
 check_verify_failure '"tagger" line label check #2' \
-	'^error:.* missingTaggerEntry:' '--no-strict'
+	'^error:.* missingTaggerEntry:' \
+	--no-strict
 
 ############################################################
 # 13. allow missing tag author name like fsck
@@ -301,7 +318,8 @@ tagger T A Gger <
 EOF
 
 check_verify_failure 'disallow malformed tagger' \
-	'^error:.* badEmail:' '--no-strict'
+	'^error:.* badEmail:' \
+	--no-strict
 
 ############################################################
 # 15. allow empty tag email
@@ -425,7 +443,8 @@ this line should not be here
 EOF
 
 check_verify_failure 'detect invalid header entry' \
-	'^error:.* extraHeaderEntry:' '--no-strict'
+	'^error:.* extraHeaderEntry:' \
+	--no-strict
 
 test_expect_success 'invalid header entry config & fsck' '
 	test_must_fail git mktag <tag.sig &&
-- 
2.32.0.555.g0268d380f7b

[PATCH 2/6] mktag tests: invert --no-strict test

[Ævar Arnfjörð Bjarmason]

Change the mktag --no-strict test to actually test success under
--no-strict, that test was added in 06ce79152be (mktag: add a
--[no-]strict option, 2021-01-06).

It doesn't make sense to check that we have the same failure except
when we want --no-strict, by doing that we're assuming that the
behavior will be different under --no-strict, bun nothing was testing
for that.

We should instead assert that --strict is the same as --no-strict,
except in the cases where we've declared that it's not.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index e9008744e3d..951e6d39c2a 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -33,6 +33,8 @@ check_verify_failure () {
 		then
 			test_must_fail git mktag <tag.sig 2>err2 &&
 			test_cmp err err2
+		else
+			git mktag --no-strict <tag.sig
 		fi
 	'
 }
-- 
2.32.0.555.g0268d380f7b

[PATCH 3/6] mktag tests: test hash-object --literally and unreachable fsck

[Ævar Arnfjörð Bjarmason]

Extend the mktag tests to pass the tag we've created through both
hash-object --literally and fsck.

This checks that fsck itself will not complain about certain invalid
content if a reachable tip isn't involved. Due to how fsck works and
walks the graph the failure will be different if the object is
reachable, so we might succeed before we've created the ref.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 48 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 951e6d39c2a..43b67a149f8 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -16,6 +16,8 @@ check_verify_failure () {
 	message=$2 &&
 	shift 2 &&
 
+	no_strict= &&
+	fsck_obj_ok= &&
 	no_strict= &&
 	while test $# != 0
 	do
@@ -23,7 +25,10 @@ check_verify_failure () {
 		--no-strict)
 			no_strict=yes
 			;;
-		esac &&
+		--fsck-obj-ok)
+			fsck_obj_ok=yes
+			;;
+		esac
 		shift
 	done &&
 
@@ -37,6 +42,23 @@ check_verify_failure () {
 			git mktag --no-strict <tag.sig
 		fi
 	'
+
+	test_expect_success "setup: $subject" '
+		# Reset any leftover state from the last $subject
+		rm -rf bad-tag &&
+
+		git init --bare bad-tag &&
+		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
+	'
+
+	test_expect_success "hash-object & fsck unreachable: $subject" '
+		if test -n "$fsck_obj_ok"
+		then
+			git -C bad-tag fsck
+		else
+			test_must_fail git -C bad-tag fsck >out 2>err
+		fi
+	'
 }
 
 test_expect_mktag_success() {
@@ -183,7 +205,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify object (hash/type) check -- correct type, nonexisting object' \
-	'^fatal: could not read tagged object'
+	'^fatal: could not read tagged object' \
+	--fsck-obj-ok
 
 cat >tag.sig <<EOF
 object $head
@@ -216,7 +239,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
-	'^fatal: object.*tagged as.*tree.*but is.*commit'
+	'^fatal: object.*tagged as.*tree.*but is.*commit' \
+	--fsck-obj-ok
 
 ############################################################
 #  9.5. verify object (hash/type) check -- replacement
@@ -245,7 +269,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
-	'^fatal: object.*tagged as.*tree.*but is.*blob'
+	'^fatal: object.*tagged as.*tree.*but is.*blob' \
+	--fsck-obj-ok
 
 ############################################################
 # 10. verify tag-name check
@@ -260,7 +285,8 @@ EOF
 
 check_verify_failure 'verify tag-name check' \
 	'^error:.* badTagName:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 11. tagger line label check #1
@@ -275,7 +301,8 @@ EOF
 
 check_verify_failure '"tagger" line label check #1' \
 	'^error:.* missingTaggerEntry:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 12. tagger line label check #2
@@ -291,7 +318,8 @@ EOF
 
 check_verify_failure '"tagger" line label check #2' \
 	'^error:.* missingTaggerEntry:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 13. allow missing tag author name like fsck
@@ -321,7 +349,8 @@ EOF
 
 check_verify_failure 'disallow malformed tagger' \
 	'^error:.* badEmail:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 15. allow empty tag email
@@ -446,7 +475,8 @@ EOF
 
 check_verify_failure 'detect invalid header entry' \
 	'^error:.* extraHeaderEntry:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 test_expect_success 'invalid header entry config & fsck' '
 	test_must_fail git mktag <tag.sig &&
-- 
2.32.0.555.g0268d380f7b

[PATCH 4/6] mktag tests: test update-ref and reachable fsck

[Ævar Arnfjörð Bjarmason]

Extend the mktag tests to pass the created bad tag through update-ref
and fsck.

The reason for passing it through update-ref is to guard against it
having a segfault as for-each-ref did before c6854508808 (ref-filter:
fix NULL check for parse object failure, 2021-04-01).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 43b67a149f8..f5a51d11f81 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -44,6 +44,8 @@ check_verify_failure () {
 	'
 
 	test_expect_success "setup: $subject" '
+		tag_ref=refs/tags/bad_tag &&
+
 		# Reset any leftover state from the last $subject
 		rm -rf bad-tag &&
 
@@ -59,6 +61,20 @@ check_verify_failure () {
 			test_must_fail git -C bad-tag fsck >out 2>err
 		fi
 	'
+
+	test_expect_success "update-ref & fsck reachable: $subject" '
+		# The update-ref of the bad content will fail, do it
+		# anyway to see if it segfaults
+		test_might_fail git -C bad-tag update-ref "$tag_ref" "$bad_tag" &&
+
+		# Manually create the broken, we cannot do it with
+		# update-ref
+		echo "$bad_tag" >"bad-tag/$tag_ref" &&
+
+		# Unlike fsck-ing unreachable content above, this
+		# will always fail.
+		test_must_fail git -C bad-tag fsck
+	'
 }
 
 test_expect_mktag_success() {
-- 
2.32.0.555.g0268d380f7b

[PATCH 5/6] mktag tests: test for-each-ref

[Ævar Arnfjörð Bjarmason]

Add a "for-each-ref" for all the mktag tests. This test would have
caught the segfault which was fixed in c6854508808 (ref-filter: fix
NULL check for parse object failure, 2021-04-01). Let's make sure we
test that code more exhaustively.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index f5a51d11f81..7d0ad3c8e62 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -50,7 +50,7 @@ check_verify_failure () {
 		rm -rf bad-tag &&
 
 		git init --bare bad-tag &&
-		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
+		bad_tag=$(git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig)
 	'
 
 	test_expect_success "hash-object & fsck unreachable: $subject" '
@@ -75,6 +75,16 @@ check_verify_failure () {
 		# will always fail.
 		test_must_fail git -C bad-tag fsck
 	'
+
+	test_expect_success "for-each-ref: $subject" '
+		echo "$bad_tag" >"bad-tag/$tag_ref" &&
+
+		printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected &&
+		git -C bad-tag for-each-ref "$tag_ref" >actual &&
+		test_cmp expected actual &&
+
+		test_must_fail git -C bad-tag for-each-ref --format="%(*objectname)"
+	'
 }
 
 test_expect_mktag_success() {
-- 
2.32.0.555.g0268d380f7b

[PATCH 6/6] mktag tests: test fast-export

[Ævar Arnfjörð Bjarmason]

Pass the bad tags we've created in the mktag tests through
fast-export, it will die on the bad object or ref, let's make sure
that happens.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 7d0ad3c8e62..ad0da2cd2b2 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -85,6 +85,11 @@ check_verify_failure () {
 
 		test_must_fail git -C bad-tag for-each-ref --format="%(*objectname)"
 	'
+
+	test_expect_success "fast-export & fast-import: $subject" '
+		test_must_fail git -C bad-tag fast-export --all &&
+		test_must_fail git -C bad-tag fast-export $bad_tag
+	'
 }
 
 test_expect_mktag_success() {
-- 
2.32.0.555.g0268d380f7b

Re: [PATCH 0/6] mktag tests: test more exhaustively

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> ... But now that the dust has settled I figured I'd submit this
> test-only series which stresses mktag, fsck and various commands that
> might deal with the broken objects the mktag tests creates.

Thanks for not forgetting ;-)

Re: [PATCH 3/6] mktag tests: test hash-object --literally and unreachable fsck

[Andrei Rybak]

On 14/06/2021 19:28, Ævar Arnfjörð Bjarmason wrote:
> Extend the mktag tests to pass the tag we've created through both
> hash-object --literally and fsck.
> 
> This checks that fsck itself will not complain about certain invalid
> content if a reachable tip isn't involved. Due to how fsck works and
> walks the graph the failure will be different if the object is
> reachable, so we might succeed before we've created the ref.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   t/t3800-mktag.sh | 48 +++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
> index 951e6d39c2a..43b67a149f8 100755
> --- a/t/t3800-mktag.sh
> +++ b/t/t3800-mktag.sh
> @@ -16,6 +16,8 @@ check_verify_failure () {
>   	message=$2 &&
>   	shift 2 &&
>   
> +	no_strict= &&
> +	fsck_obj_ok= &&
>   	no_strict= &&
>   	while test $# != 0
>   	do
> @@ -23,7 +25,10 @@ check_verify_failure () {
>   		--no-strict)
>   			no_strict=yes
>   			;;
> -		esac &&
> +		--fsck-obj-ok)
> +			fsck_obj_ok=yes
> +			;;
> +		esac

"&&" after "esac" got removed.

>   		shift
>   	done &&
>   
> @@ -37,6 +42,23 @@ check_verify_failure () {
>   			git mktag --no-strict <tag.sig
>   		fi
>   	'
> +
> +	test_expect_success "setup: $subject" '
> +		# Reset any leftover state from the last $subject
> +		rm -rf bad-tag &&
> +
> +		git init --bare bad-tag &&
> +		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
> +	'
> +
> +	test_expect_success "hash-object & fsck unreachable: $subject" '
> +		if test -n "$fsck_obj_ok"
> +		then
> +			git -C bad-tag fsck
> +		else
> +			test_must_fail git -C bad-tag fsck >out 2>err

This is the very end of the check_verify_failure function. It seems
that after "out" and "err" are created here, nothing else checks their
contents.

> +		fi
> +	'
>   }
>   
>   test_expect_mktag_success() {
> @@ -183,7 +205,8 @@ tagger . <> 0 +0000
>   EOF
>   
>   check_verify_failure 'verify object (hash/type) check -- correct type, nonexisting object' \
> -	'^fatal: could not read tagged object'
> +	'^fatal: could not read tagged object' \
> +	--fsck-obj-ok
>   
>   cat >tag.sig <<EOF
>   object $head
> @@ -216,7 +239,8 @@ tagger . <> 0 +0000
>   EOF
>   
>   check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
> -	'^fatal: object.*tagged as.*tree.*but is.*commit'
> +	'^fatal: object.*tagged as.*tree.*but is.*commit' \
> +	--fsck-obj-ok
>   
>   ############################################################
>   #  9.5. verify object (hash/type) check -- replacement
> @@ -245,7 +269,8 @@ tagger . <> 0 +0000
>   EOF
>   
>   check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
> -	'^fatal: object.*tagged as.*tree.*but is.*blob'
> +	'^fatal: object.*tagged as.*tree.*but is.*blob' \
> +	--fsck-obj-ok
>   
>   ############################################################
>   # 10. verify tag-name check
> @@ -260,7 +285,8 @@ EOF
>   
>   check_verify_failure 'verify tag-name check' \
>   	'^error:.* badTagName:' \
> -	--no-strict
> +	--no-strict \
> +	--fsck-obj-ok
>   
>   ############################################################
>   # 11. tagger line label check #1
> @@ -275,7 +301,8 @@ EOF
>   
>   check_verify_failure '"tagger" line label check #1' \
>   	'^error:.* missingTaggerEntry:' \
> -	--no-strict
> +	--no-strict \
> +	--fsck-obj-ok
>   
>   ############################################################
>   # 12. tagger line label check #2
> @@ -291,7 +318,8 @@ EOF
>   
>   check_verify_failure '"tagger" line label check #2' \
>   	'^error:.* missingTaggerEntry:' \
> -	--no-strict
> +	--no-strict \
> +	--fsck-obj-ok
>   
>   ############################################################
>   # 13. allow missing tag author name like fsck
> @@ -321,7 +349,8 @@ EOF
>   
>   check_verify_failure 'disallow malformed tagger' \
>   	'^error:.* badEmail:' \
> -	--no-strict
> +	--no-strict \
> +	--fsck-obj-ok
>   
>   ############################################################
>   # 15. allow empty tag email
> @@ -446,7 +475,8 @@ EOF
>   
>   check_verify_failure 'detect invalid header entry' \
>   	'^error:.* extraHeaderEntry:' \
> -	--no-strict
> +	--no-strict \
> +	--fsck-obj-ok
>   
>   test_expect_success 'invalid header entry config & fsck' '
>   	test_must_fail git mktag <tag.sig &&
> 

Re: [PATCH 3/6] mktag tests: test hash-object --literally and unreachable fsck

[Ævar Arnfjörð Bjarmason]

On Tue, Jun 15 2021, Andrei Rybak wrote:

> On 14/06/2021 19:28, Ævar Arnfjörð Bjarmason wrote:
>> Extend the mktag tests to pass the tag we've created through both
>> hash-object --literally and fsck.
>> This checks that fsck itself will not complain about certain invalid
>> content if a reachable tip isn't involved. Due to how fsck works and
>> walks the graph the failure will be different if the object is
>> reachable, so we might succeed before we've created the ref.
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   t/t3800-mktag.sh | 48 +++++++++++++++++++++++++++++++++++++++---------
>>   1 file changed, 39 insertions(+), 9 deletions(-)
>> diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
>> index 951e6d39c2a..43b67a149f8 100755
>> --- a/t/t3800-mktag.sh
>> +++ b/t/t3800-mktag.sh
>> @@ -16,6 +16,8 @@ check_verify_failure () {
>>   	message=$2 &&
>>   	shift 2 &&
>>   +	no_strict= &&
>> +	fsck_obj_ok= &&
>>   	no_strict= &&
>>   	while test $# != 0
>>   	do
>> @@ -23,7 +25,10 @@ check_verify_failure () {
>>   		--no-strict)
>>   			no_strict=yes
>>   			;;
>> -		esac &&
>> +		--fsck-obj-ok)
>> +			fsck_obj_ok=yes
>> +			;;
>> +		esac
>
> "&&" after "esac" got removed.

Thanks, will fix.

>>   		shift
>>   	done &&
>>   @@ -37,6 +42,23 @@ check_verify_failure () {
>>   			git mktag --no-strict <tag.sig
>>   		fi
>>   	'
>> +
>> +	test_expect_success "setup: $subject" '
>> +		# Reset any leftover state from the last $subject
>> +		rm -rf bad-tag &&
>> +
>> +		git init --bare bad-tag &&
>> +		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
>> +	'
>> +
>> +	test_expect_success "hash-object & fsck unreachable: $subject" '
>> +		if test -n "$fsck_obj_ok"
>> +		then
>> +			git -C bad-tag fsck
>> +		else
>> +			test_must_fail git -C bad-tag fsck >out 2>err
>
> This is the very end of the check_verify_failure function. It seems
> that after "out" and "err" are created here, nothing else checks their
> contents.

Well spotted, will fix this and any other such needless rederiction in a
v2 I'll send after any other reviewing quiets down.

[PATCH v2 0/6] mktag tests: test more exhaustively

[Ævar Arnfjörð Bjarmason]

A re-roll of [1] which adds more mktag tests. Fixes the issues noted
in [2] and one more that I spotted myself: I used a "$bad_tag" in an
earlier commit than it was defined. I've fixed that, and changed the
structure of the tests (use rev-parse to assert) to make sure it is
defined before we do the "test_must_fail" tests.

1. https://lore.kernel.org/git/cover-0.6-00000000000-20210614T172422Z-avarab@gmail.com
2. https://lore.kernel.org/git/6148debe-fc44-67b7-ac9c-f27d376bc9f2@gmail.com

Ævar Arnfjörð Bjarmason (6):
  mktag tests: parse out options in helper
  mktag tests: invert --no-strict test
  mktag tests: test hash-object --literally and unreachable fsck
  mktag tests: test update-ref and reachable fsck
  mktag tests: test for-each-ref
  mktag tests: test fast-export

 t/t3800-mktag.sh | 121 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 106 insertions(+), 15 deletions(-)

Range-diff against v1:
1:  d49549973d = 1:  d49549973d mktag tests: parse out options in helper
2:  550dcedfcb = 2:  550dcedfcb mktag tests: invert --no-strict test
3:  96520b3e89 ! 3:  bab235d752 mktag tests: test hash-object --literally and unreachable fsck
    @@ t/t3800-mktag.sh: check_verify_failure () {
      		--no-strict)
      			no_strict=yes
      			;;
    --		esac &&
     +		--fsck-obj-ok)
     +			fsck_obj_ok=yes
     +			;;
    -+		esac
    + 		esac &&
      		shift
      	done &&
    - 
     @@ t/t3800-mktag.sh: check_verify_failure () {
      			git mktag --no-strict <tag.sig
      		fi
    @@ t/t3800-mktag.sh: check_verify_failure () {
     +		then
     +			git -C bad-tag fsck
     +		else
    -+			test_must_fail git -C bad-tag fsck >out 2>err
    ++			test_must_fail git -C bad-tag fsck
     +		fi
     +	'
      }
4:  0359becdc9 ! 4:  a1d95c81ea mktag tests: test update-ref and reachable fsck
    @@ t/t3800-mktag.sh: check_verify_failure () {
      		# Reset any leftover state from the last $subject
      		rm -rf bad-tag &&
      
    + 		git init --bare bad-tag &&
    +-		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
    ++		bad_tag=$(git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig)
    + 	'
    + 
    + 	test_expect_success "hash-object & fsck unreachable: $subject" '
     @@ t/t3800-mktag.sh: check_verify_failure () {
    - 			test_must_fail git -C bad-tag fsck >out 2>err
    + 			test_must_fail git -C bad-tag fsck
      		fi
      	'
     +
     +	test_expect_success "update-ref & fsck reachable: $subject" '
    ++		# Make sure the earlier test created it for us
    ++		git rev-parse "$bad_tag" &&
    ++
     +		# The update-ref of the bad content will fail, do it
     +		# anyway to see if it segfaults
     +		test_might_fail git -C bad-tag update-ref "$tag_ref" "$bad_tag" &&
5:  8a626fc63a ! 5:  f4c41be92d mktag tests: test for-each-ref
    @@ Commit message
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/t3800-mktag.sh ##
    -@@ t/t3800-mktag.sh: check_verify_failure () {
    - 		rm -rf bad-tag &&
    - 
    - 		git init --bare bad-tag &&
    --		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
    -+		bad_tag=$(git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig)
    - 	'
    - 
    - 	test_expect_success "hash-object & fsck unreachable: $subject" '
     @@ t/t3800-mktag.sh: check_verify_failure () {
      		# will always fail.
      		test_must_fail git -C bad-tag fsck
      	'
     +
     +	test_expect_success "for-each-ref: $subject" '
    ++		# Make sure the earlier test created it for us
    ++		git rev-parse "$bad_tag" &&
    ++
     +		echo "$bad_tag" >"bad-tag/$tag_ref" &&
     +
     +		printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected &&
6:  6d613d88ac ! 6:  797c8d3a87 mktag tests: test fast-export
    @@ t/t3800-mktag.sh: check_verify_failure () {
      	'
     +
     +	test_expect_success "fast-export & fast-import: $subject" '
    ++		# Make sure the earlier test created it for us
    ++		git rev-parse "$bad_tag" &&
    ++
     +		test_must_fail git -C bad-tag fast-export --all &&
    -+		test_must_fail git -C bad-tag fast-export $bad_tag
    ++		test_must_fail git -C bad-tag fast-export "$bad_tag"
     +	'
      }
      
-- 
2.32.0.571.gdba276db2c

[PATCH v2 1/6] mktag tests: parse out options in helper

[Ævar Arnfjörð Bjarmason]

Change check_verify_failure() helper to parse out options from
$@. This makes it easier to add new options in the future. See
06ce79152be (mktag: add a --[no-]strict option, 2021-01-06) for the
initial implementation.

Let's also replace "" quotes with '' for the test body, the varables
we need are eval'd into the body, so there's no need for the quoting
confusion.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 43 +++++++++++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 6275c98523..e9008744e3 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -12,15 +12,29 @@ test_description='git mktag: tag object verify test'
 # given in the expect.pat file.
 
 check_verify_failure () {
-	test_expect_success "$1" "
-		test_must_fail git mktag <tag.sig 2>message &&
-		grep '$2' message &&
-		if test '$3' != '--no-strict'
+	subject=$1 &&
+	message=$2 &&
+	shift 2 &&
+
+	no_strict= &&
+	while test $# != 0
+	do
+		case "$1" in
+		--no-strict)
+			no_strict=yes
+			;;
+		esac &&
+		shift
+	done &&
+
+	test_expect_success "fail with [--[no-]strict]: $subject" '
+		test_must_fail git mktag <tag.sig 2>err &&
+		if test -z "$no_strict"
 		then
-			test_must_fail git mktag --no-strict <tag.sig 2>message.no-strict &&
-			grep '$2' message.no-strict
+			test_must_fail git mktag <tag.sig 2>err2 &&
+			test_cmp err err2
 		fi
-	"
+	'
 }
 
 test_expect_mktag_success() {
@@ -243,7 +257,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify tag-name check' \
-	'^error:.* badTagName:' '--no-strict'
+	'^error:.* badTagName:' \
+	--no-strict
 
 ############################################################
 # 11. tagger line label check #1
@@ -257,7 +272,8 @@ This is filler
 EOF
 
 check_verify_failure '"tagger" line label check #1' \
-	'^error:.* missingTaggerEntry:' '--no-strict'
+	'^error:.* missingTaggerEntry:' \
+	--no-strict
 
 ############################################################
 # 12. tagger line label check #2
@@ -272,7 +288,8 @@ This is filler
 EOF
 
 check_verify_failure '"tagger" line label check #2' \
-	'^error:.* missingTaggerEntry:' '--no-strict'
+	'^error:.* missingTaggerEntry:' \
+	--no-strict
 
 ############################################################
 # 13. allow missing tag author name like fsck
@@ -301,7 +318,8 @@ tagger T A Gger <
 EOF
 
 check_verify_failure 'disallow malformed tagger' \
-	'^error:.* badEmail:' '--no-strict'
+	'^error:.* badEmail:' \
+	--no-strict
 
 ############################################################
 # 15. allow empty tag email
@@ -425,7 +443,8 @@ this line should not be here
 EOF
 
 check_verify_failure 'detect invalid header entry' \
-	'^error:.* extraHeaderEntry:' '--no-strict'
+	'^error:.* extraHeaderEntry:' \
+	--no-strict
 
 test_expect_success 'invalid header entry config & fsck' '
 	test_must_fail git mktag <tag.sig &&
-- 
2.32.0.571.gdba276db2c

[PATCH v2 2/6] mktag tests: invert --no-strict test

[Ævar Arnfjörð Bjarmason]

Change the mktag --no-strict test to actually test success under
--no-strict, that test was added in 06ce79152be (mktag: add a
--[no-]strict option, 2021-01-06).

It doesn't make sense to check that we have the same failure except
when we want --no-strict, by doing that we're assuming that the
behavior will be different under --no-strict, bun nothing was testing
for that.

We should instead assert that --strict is the same as --no-strict,
except in the cases where we've declared that it's not.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index e9008744e3..951e6d39c2 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -33,6 +33,8 @@ check_verify_failure () {
 		then
 			test_must_fail git mktag <tag.sig 2>err2 &&
 			test_cmp err err2
+		else
+			git mktag --no-strict <tag.sig
 		fi
 	'
 }
-- 
2.32.0.571.gdba276db2c

[PATCH v2 3/6] mktag tests: test hash-object --literally and unreachable fsck

[Ævar Arnfjörð Bjarmason]

Extend the mktag tests to pass the tag we've created through both
hash-object --literally and fsck.

This checks that fsck itself will not complain about certain invalid
content if a reachable tip isn't involved. Due to how fsck works and
walks the graph the failure will be different if the object is
reachable, so we might succeed before we've created the ref.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 46 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 951e6d39c2..78c6f64e36 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -16,6 +16,8 @@ check_verify_failure () {
 	message=$2 &&
 	shift 2 &&
 
+	no_strict= &&
+	fsck_obj_ok= &&
 	no_strict= &&
 	while test $# != 0
 	do
@@ -23,6 +25,9 @@ check_verify_failure () {
 		--no-strict)
 			no_strict=yes
 			;;
+		--fsck-obj-ok)
+			fsck_obj_ok=yes
+			;;
 		esac &&
 		shift
 	done &&
@@ -37,6 +42,23 @@ check_verify_failure () {
 			git mktag --no-strict <tag.sig
 		fi
 	'
+
+	test_expect_success "setup: $subject" '
+		# Reset any leftover state from the last $subject
+		rm -rf bad-tag &&
+
+		git init --bare bad-tag &&
+		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
+	'
+
+	test_expect_success "hash-object & fsck unreachable: $subject" '
+		if test -n "$fsck_obj_ok"
+		then
+			git -C bad-tag fsck
+		else
+			test_must_fail git -C bad-tag fsck
+		fi
+	'
 }
 
 test_expect_mktag_success() {
@@ -183,7 +205,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify object (hash/type) check -- correct type, nonexisting object' \
-	'^fatal: could not read tagged object'
+	'^fatal: could not read tagged object' \
+	--fsck-obj-ok
 
 cat >tag.sig <<EOF
 object $head
@@ -216,7 +239,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
-	'^fatal: object.*tagged as.*tree.*but is.*commit'
+	'^fatal: object.*tagged as.*tree.*but is.*commit' \
+	--fsck-obj-ok
 
 ############################################################
 #  9.5. verify object (hash/type) check -- replacement
@@ -245,7 +269,8 @@ tagger . <> 0 +0000
 EOF
 
 check_verify_failure 'verify object (hash/type) check -- mismatched type, valid object' \
-	'^fatal: object.*tagged as.*tree.*but is.*blob'
+	'^fatal: object.*tagged as.*tree.*but is.*blob' \
+	--fsck-obj-ok
 
 ############################################################
 # 10. verify tag-name check
@@ -260,7 +285,8 @@ EOF
 
 check_verify_failure 'verify tag-name check' \
 	'^error:.* badTagName:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 11. tagger line label check #1
@@ -275,7 +301,8 @@ EOF
 
 check_verify_failure '"tagger" line label check #1' \
 	'^error:.* missingTaggerEntry:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 12. tagger line label check #2
@@ -291,7 +318,8 @@ EOF
 
 check_verify_failure '"tagger" line label check #2' \
 	'^error:.* missingTaggerEntry:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 13. allow missing tag author name like fsck
@@ -321,7 +349,8 @@ EOF
 
 check_verify_failure 'disallow malformed tagger' \
 	'^error:.* badEmail:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 ############################################################
 # 15. allow empty tag email
@@ -446,7 +475,8 @@ EOF
 
 check_verify_failure 'detect invalid header entry' \
 	'^error:.* extraHeaderEntry:' \
-	--no-strict
+	--no-strict \
+	--fsck-obj-ok
 
 test_expect_success 'invalid header entry config & fsck' '
 	test_must_fail git mktag <tag.sig &&
-- 
2.32.0.571.gdba276db2c

[PATCH v2 4/6] mktag tests: test update-ref and reachable fsck

[Ævar Arnfjörð Bjarmason]

Extend the mktag tests to pass the created bad tag through update-ref
and fsck.

The reason for passing it through update-ref is to guard against it
having a segfault as for-each-ref did before c6854508808 (ref-filter:
fix NULL check for parse object failure, 2021-04-01).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 78c6f64e36..67f6ecbe88 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -44,11 +44,13 @@ check_verify_failure () {
 	'
 
 	test_expect_success "setup: $subject" '
+		tag_ref=refs/tags/bad_tag &&
+
 		# Reset any leftover state from the last $subject
 		rm -rf bad-tag &&
 
 		git init --bare bad-tag &&
-		git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig
+		bad_tag=$(git -C bad-tag hash-object -t tag -w --stdin --literally <tag.sig)
 	'
 
 	test_expect_success "hash-object & fsck unreachable: $subject" '
@@ -59,6 +61,23 @@ check_verify_failure () {
 			test_must_fail git -C bad-tag fsck
 		fi
 	'
+
+	test_expect_success "update-ref & fsck reachable: $subject" '
+		# Make sure the earlier test created it for us
+		git rev-parse "$bad_tag" &&
+
+		# The update-ref of the bad content will fail, do it
+		# anyway to see if it segfaults
+		test_might_fail git -C bad-tag update-ref "$tag_ref" "$bad_tag" &&
+
+		# Manually create the broken, we cannot do it with
+		# update-ref
+		echo "$bad_tag" >"bad-tag/$tag_ref" &&
+
+		# Unlike fsck-ing unreachable content above, this
+		# will always fail.
+		test_must_fail git -C bad-tag fsck
+	'
 }
 
 test_expect_mktag_success() {
-- 
2.32.0.571.gdba276db2c

[PATCH v2 5/6] mktag tests: test for-each-ref

[Ævar Arnfjörð Bjarmason]

Add a "for-each-ref" for all the mktag tests. This test would have
caught the segfault which was fixed in c6854508808 (ref-filter: fix
NULL check for parse object failure, 2021-04-01). Let's make sure we
test that code more exhaustively.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index 67f6ecbe88..bb71303399 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -78,6 +78,19 @@ check_verify_failure () {
 		# will always fail.
 		test_must_fail git -C bad-tag fsck
 	'
+
+	test_expect_success "for-each-ref: $subject" '
+		# Make sure the earlier test created it for us
+		git rev-parse "$bad_tag" &&
+
+		echo "$bad_tag" >"bad-tag/$tag_ref" &&
+
+		printf "%s tag\t%s\n" "$bad_tag" "$tag_ref" >expected &&
+		git -C bad-tag for-each-ref "$tag_ref" >actual &&
+		test_cmp expected actual &&
+
+		test_must_fail git -C bad-tag for-each-ref --format="%(*objectname)"
+	'
 }
 
 test_expect_mktag_success() {
-- 
2.32.0.571.gdba276db2c

[PATCH v2 6/6] mktag tests: test fast-export

[Ævar Arnfjörð Bjarmason]

Pass the bad tags we've created in the mktag tests through
fast-export, it will die on the bad object or ref, let's make sure
that happens.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3800-mktag.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index bb71303399..0544d58a6e 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -91,6 +91,14 @@ check_verify_failure () {
 
 		test_must_fail git -C bad-tag for-each-ref --format="%(*objectname)"
 	'
+
+	test_expect_success "fast-export & fast-import: $subject" '
+		# Make sure the earlier test created it for us
+		git rev-parse "$bad_tag" &&
+
+		test_must_fail git -C bad-tag fast-export --all &&
+		test_must_fail git -C bad-tag fast-export "$bad_tag"
+	'
 }
 
 test_expect_mktag_success() {
-- 
2.32.0.571.gdba276db2c