Subject: Re: [PATCH v7 06/17] cat-file tests: test for missing/bogus object with -t, -s and -p

From: Taylor Blau <[email-masked]>
Date: Mon, 20 Sep 2021 23:30:07 -0400

On Mon, Sep 20, 2021 at 09:04:10PM +0200, Ævar Arnfjörð Bjarmason wrote:
> diff --git a/t/oid-info/oid b/t/oid-info/oid
> index a754970523c..ecffa9045f9 100644
> --- a/t/oid-info/oid
> +++ b/t/oid-info/oid
> @@ -27,3 +27,5 @@ numeric		sha1:0123456789012345678901234567890123456789
>  numeric		sha256:0123456789012345678901234567890123456789012345678901234567890123
>  deadbeef	sha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbeef
>  deadbeef	sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
> +deadbeef_short	sha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbee
> +deadbee_short	sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbee

This jumped out at me while I was reading it. In the second line,
s/deadbee_short/deadbeef_short/ ?

> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index ea6a53d425b..af59613250b 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -327,6 +327,81 @@ test_expect_success 'setup bogus data' '
>  	bogus_long_sha1=$(echo_without_newline "$bogus_long_content" | git hash-object -t $bogus_long_type --literally -w --stdin)
>  '
>
> +for arg1 in '' --allow-unknown-type
> +do
> +	for arg2 in -s -t -p
> +	do

This is quite the loop! I appreciate the extra thoroughness, although it
may come at some extra cost of intertwining all of these combinations of
tests together.

But that may be warranted, since they are all related. But it's not a
full matrix of all possible combinations; e.g., "--allow-unknown-type"
does not go with "-p".

So this may be the best that we can do. It's definitely a mouthful, but
I think it's overall an easier read than what we had in the previous
version. And it's definitely more thorough, which is good. Thanks for
spending the time improving this test.

Thanks,
Taylor