diff mbox series

[2/4] t1450: increase test coverage of in-tree d/f detection

Message ID 106c58e1-9c74-46e3-c83a-88eee114d9d6@web.de (mailing list archive)
State New, archived
Headers show
Series [1/4] fsck: fix a typo in a comment | expand

Commit Message

René Scharfe May 21, 2020, 9:52 a.m. UTC 
Exercise the case of putting a conflict candidate file name back on the
stack because a matching directory might yet come up later.

Do that by factoring out the test code into a function to allow for more
concise notation in the form of parameters indicating names of trees
(with trailing slash) and blobs (without trailing slash) in no
particular order (they are sorted by git mktree).  Then add the new test
case as a second function call.

Fix a typo in the test title while at it ("dublicate").

Reported-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/t1450-fsck.sh | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

--
2.26.2

Comments

Denton Liu May 21, 2020, 10:20 a.m. UTC | #1 
Hi René,

On Thu, May 21, 2020 at 11:52:28AM +0200, René Scharfe wrote:
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index 91a6e34f38..9640ac8ff2 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -257,21 +257,33 @@ test_expect_success 'tree object with duplicate entries' '
>  	test_i18ngrep "error in tree .*contains duplicate file entries" out
>  '
> 
> -test_expect_success 'tree object with dublicate names' '
> -	test_when_finished "remove_object \$blob" &&
> -	test_when_finished "remove_object \$tree" &&
> -	test_when_finished "remove_object \$badtree" &&
> -	blob=$(echo blob | git hash-object -w --stdin) &&
> -	printf "100644 blob %s\t%s\n" $blob x.2 >tree &&
> -	tree=$(git mktree <tree) &&
> -	printf "100644 blob %s\t%s\n" $blob x.1 >badtree &&
> -	printf "100644 blob %s\t%s\n" $blob x >>badtree &&
> -	printf "040000 tree %s\t%s\n" $tree x >>badtree &&
> -	badtree=$(git mktree <badtree) &&
> -	test_must_fail git fsck 2>out &&
> -	test_i18ngrep "$badtree" out &&
> -	test_i18ngrep "error in tree .*contains duplicate file entries" out
> -'
> +check_duplicate_names () {
> +	expect=$1 &&
> +	shift &&
> +	names=$@ &&

It doesn't really make sense to use $@ here since we're not using the
argument list behaviour of $@; we're just expanding it normally. I would
replace this with $* instead.

> +	test_expect_$expect "tree object with duplicate names: $names" '
> +		test_when_finished "remove_object \$blob" &&
> +		test_when_finished "remove_object \$tree" &&
> +		test_when_finished "remove_object \$badtree" &&
> +		blob=$(echo blob | git hash-object -w --stdin) &&
> +		printf "100644 blob %s\t%s\n" $blob x.2 >tree &&
> +		tree=$(git mktree <tree) &&
> +		for name in $names
> +		do
> +			case "$name" in
> +			*/) printf "040000 tree %s\t%s\n" $tree "${name%/}" ;;
> +			*)  printf "100644 blob %s\t%s\n" $blob "$name" ;;
> +			esac
> +		done >badtree &&
> +		badtree=$(git mktree <badtree) &&
> +		test_must_fail git fsck 2>out &&
> +		test_i18ngrep "$badtree" out &&
> +		test_i18ngrep "error in tree .*contains duplicate file entries" out
> +	'
> +}
> +
> +check_duplicate_names success x x.1 x/
> +check_duplicate_names success x x.1.2 x.1/ x/
> 
>  test_expect_success 'unparseable tree object' '
>  	test_oid_cache <<-\EOF &&
> --
> 2.26.2
René Scharfe May 21, 2020, 1:31 p.m. UTC | #2 
Am 21.05.20 um 12:20 schrieb Denton Liu:
> Hi René,
>
> On Thu, May 21, 2020 at 11:52:28AM +0200, René Scharfe wrote:
>> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
>> index 91a6e34f38..9640ac8ff2 100755
>> --- a/t/t1450-fsck.sh
>> +++ b/t/t1450-fsck.sh
>> @@ -257,21 +257,33 @@ test_expect_success 'tree object with duplicate entries' '
>>  	test_i18ngrep "error in tree .*contains duplicate file entries" out
>>  '
>>
>> -test_expect_success 'tree object with dublicate names' '
>> -	test_when_finished "remove_object \$blob" &&
>> -	test_when_finished "remove_object \$tree" &&
>> -	test_when_finished "remove_object \$badtree" &&
>> -	blob=$(echo blob | git hash-object -w --stdin) &&
>> -	printf "100644 blob %s\t%s\n" $blob x.2 >tree &&
>> -	tree=$(git mktree <tree) &&
>> -	printf "100644 blob %s\t%s\n" $blob x.1 >badtree &&
>> -	printf "100644 blob %s\t%s\n" $blob x >>badtree &&
>> -	printf "040000 tree %s\t%s\n" $tree x >>badtree &&
>> -	badtree=$(git mktree <badtree) &&
>> -	test_must_fail git fsck 2>out &&
>> -	test_i18ngrep "$badtree" out &&
>> -	test_i18ngrep "error in tree .*contains duplicate file entries" out
>> -'
>> +check_duplicate_names () {
>> +	expect=$1 &&
>> +	shift &&
>> +	names=$@ &&
>
> It doesn't really make sense to use $@ here since we're not using the
> argument list behaviour of $@; we're just expanding it normally. I would
> replace this with $* instead.

The assignment to $names flattens the list, so $@ and $* behave the same
here.  I didn't think much about it, but it would be nice to support names
that contain spaces, which we could do by writing the arguments to a file.
And if we go that route then support for names with newlines would be nice
as well.  I'm not sure it's worth it, but a patch for that is below.

At least I'd like to keep the $@ as kind of a reminder that we want to
pass on arguments (full names), not words.

>
>> +	test_expect_$expect "tree object with duplicate names: $names" '
>> +		test_when_finished "remove_object \$blob" &&
>> +		test_when_finished "remove_object \$tree" &&
>> +		test_when_finished "remove_object \$badtree" &&
>> +		blob=$(echo blob | git hash-object -w --stdin) &&
>> +		printf "100644 blob %s\t%s\n" $blob x.2 >tree &&
>> +		tree=$(git mktree <tree) &&
>> +		for name in $names
>> +		do
>> +			case "$name" in
>> +			*/) printf "040000 tree %s\t%s\n" $tree "${name%/}" ;;
>> +			*)  printf "100644 blob %s\t%s\n" $blob "$name" ;;
>> +			esac
>> +		done >badtree &&
>> +		badtree=$(git mktree <badtree) &&
>> +		test_must_fail git fsck 2>out &&
>> +		test_i18ngrep "$badtree" out &&
>> +		test_i18ngrep "error in tree .*contains duplicate file entries" out
>> +	'
>> +}
>> +
>> +check_duplicate_names success x x.1 x/
>> +check_duplicate_names success x x.1.2 x.1/ x/
>>
>>  test_expect_success 'unparseable tree object' '
>>  	test_oid_cache <<-\EOF &&
>> --
>> 2.26.2

-- >8 --
Subject: [PATCH 5/4] t1450: support names with spaces in check_duplicate_names()

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/t1450-fsck.sh | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 344a2aad82..b1766c8e11 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -257,10 +257,20 @@ test_expect_success 'tree object with duplicate entries' '
 	test_i18ngrep "error in tree .*contains duplicate file entries" out
 '

+names_to_tree () {
+	awk -v blob="$1" -v tree="$2" -v RS='\0' -v FS='/' '
+		NF == 1 {printf "100644 blob %s\t%s%c", blob, $1, 0}
+		NF == 2 {printf "040000 tree %s\t%s%c", tree, $1, 0}
+	'
+}
+
 check_duplicate_names () {
 	expect=$1 &&
 	shift &&
-	names=$@ &&
+	for name in "$@"
+	do
+		printf "%s\0" "$name"
+	done >names &&
 	test_expect_$expect "tree object with duplicate names: $names" '
 		test_when_finished "remove_object \$blob" &&
 		test_when_finished "remove_object \$tree" &&
@@ -268,14 +278,8 @@ check_duplicate_names () {
 		blob=$(echo blob | git hash-object -w --stdin) &&
 		printf "100644 blob %s\t%s\n" $blob x.2 >tree &&
 		tree=$(git mktree <tree) &&
-		for name in $names
-		do
-			case "$name" in
-			*/) printf "040000 tree %s\t%s\n" $tree "${name%/}" ;;
-			*)  printf "100644 blob %s\t%s\n" $blob "$name" ;;
-			esac
-		done >badtree &&
-		badtree=$(git mktree <badtree) &&
+		names_to_tree $blob $tree <names >badtree &&
+		badtree=$(git mktree -z <badtree) &&
 		test_must_fail git fsck 2>out &&
 		test_i18ngrep "$badtree" out &&
 		test_i18ngrep "error in tree .*contains duplicate file entries" out
--
2.26.2
Junio C Hamano May 21, 2020, 6:01 p.m. UTC | #3 
René Scharfe <l.s.r@web.de> writes:

>>> +check_duplicate_names () {
>>> +	expect=$1 &&
>>> +	shift &&
>>> +	names=$@ &&
>>
>> It doesn't really make sense to use $@ here since we're not using the
>> argument list behaviour of $@; we're just expanding it normally. I would
>> replace this with $* instead.
>
> The assignment to $names flattens the list, so $@ and $* behave the same
> here.
> ...
> At least I'd like to keep the $@ as kind of a reminder that we want to
> pass on arguments (full names), not words.

I personally prefer to use "$*" when we are not invoking the "list"
magic of "$@" and switch it to "$@" when it starts to matter, but I
can also understand your "reminder value" reasoning, so I am on the
fence.
diff mbox series

Patch

diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 91a6e34f38..9640ac8ff2 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -257,21 +257,33 @@  test_expect_success 'tree object with duplicate entries' '
 	test_i18ngrep "error in tree .*contains duplicate file entries" out
 '

-test_expect_success 'tree object with dublicate names' '
-	test_when_finished "remove_object \$blob" &&
-	test_when_finished "remove_object \$tree" &&
-	test_when_finished "remove_object \$badtree" &&
-	blob=$(echo blob | git hash-object -w --stdin) &&
-	printf "100644 blob %s\t%s\n" $blob x.2 >tree &&
-	tree=$(git mktree <tree) &&
-	printf "100644 blob %s\t%s\n" $blob x.1 >badtree &&
-	printf "100644 blob %s\t%s\n" $blob x >>badtree &&
-	printf "040000 tree %s\t%s\n" $tree x >>badtree &&
-	badtree=$(git mktree <badtree) &&
-	test_must_fail git fsck 2>out &&
-	test_i18ngrep "$badtree" out &&
-	test_i18ngrep "error in tree .*contains duplicate file entries" out
-'
+check_duplicate_names () {
+	expect=$1 &&
+	shift &&
+	names=$@ &&
+	test_expect_$expect "tree object with duplicate names: $names" '
+		test_when_finished "remove_object \$blob" &&
+		test_when_finished "remove_object \$tree" &&
+		test_when_finished "remove_object \$badtree" &&
+		blob=$(echo blob | git hash-object -w --stdin) &&
+		printf "100644 blob %s\t%s\n" $blob x.2 >tree &&
+		tree=$(git mktree <tree) &&
+		for name in $names
+		do
+			case "$name" in
+			*/) printf "040000 tree %s\t%s\n" $tree "${name%/}" ;;
+			*)  printf "100644 blob %s\t%s\n" $blob "$name" ;;
+			esac
+		done >badtree &&
+		badtree=$(git mktree <badtree) &&
+		test_must_fail git fsck 2>out &&
+		test_i18ngrep "$badtree" out &&
+		test_i18ngrep "error in tree .*contains duplicate file entries" out
+	'
+}
+
+check_duplicate_names success x x.1 x/
+check_duplicate_names success x x.1.2 x.1/ x/

 test_expect_success 'unparseable tree object' '
 	test_oid_cache <<-\EOF &&