Subject: Re: [PATCH v5 2/2] t5300: add test for 'show-index --object-format'

From: Junio C Hamano <[email-masked]>
Date: Mon, 04 Nov 2024 17:19:05 -0800

Abhijeet Sonar <[email-masked]> writes:

> In 88a09a557c (builtin/show-index: provide options to determine hash
> algo), the flag --object-format was added to show-index builtin as a way
> to provide a hash algorithm explicitly. However, we do not have tests in
> place for that functionality. Add them.
>
> Signed-off-by: Abhijeet Sonar <[email-masked]>
> ---
>  t/t5300-pack-object.sh | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

Nicely described.

> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index 51fed26cc4..301d5f1b61 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -527,6 +527,28 @@ test_expect_success SHA1 'show-index works OK outside a repository' '
>  	nongit git show-index <foo.idx
>  '
>  
> +for hash in sha1 sha256
> +do
> +	test_expect_success 'setup: show-index works OK outside a repository with hash algo passed in via --object-format' '
> +		git init explicit-hash-$hash --object-format=$hash &&

"git help cli"; dashed options first and then other arguments.

> +		test_commit -C explicit-hash-$hash one &&
> +
> +		cat >in <<-EOF &&
> +		$(git -C explicit-hash-$hash rev-parse one)
> +		EOF

Hmph, is the above a roundabout way to say

		git -C explicit-hash-$hash rev-parse one >in &&

or am I missing some subtlety?

> +		git -C explicit-hash-$hash pack-objects explicit-hash-$hash <in

> +	'
> +
> +	test_expect_success 'show-index works OK outside a repository with hash algo passed in via --object-format' '
> +		idx=$(echo explicit-hash-$hash/explicit-hash-$hash*.idx) &&
> +		nongit git show-index --object-format=$hash <"$idx" >actual &&
> +		test_line_count = 1 actual &&
> +
> +		rm -rf explicit-hash-$hash

When this test fails (e.g., the number of lines in the show-index
output is not 1), explicit-hash-$hash is not removed, because &&-
chain short-circuits.

Perhaps join thw two into one and use test_when_finished, like this?

	test_expect_success 'show-index with explicit --object-format=$hash outside repo' '
		test_when_finished "rm -fr explicit-hash-$hash" &&
		git init --object-format=$hash explicit-hash-$hash &&
		...
                nongit git show-index --object-format=$hash <"$idx" >actual &&
		test_line_count 1 actual
	'

Other than that, very nicely done.

Thanks.