Subject: Re: [PATCH v2 06/10] mktag tests: remove needless SHA-1 hardcoding

From: Junio C Hamano <[email-masked]>
Date: Tue, 01 Dec 2020 12:24:42 -0800

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> Change the tests amended in acb49d1cc8b (t3800: make hash-size
> independent, 2019-08-18) even more to make them independent of either
> SHA-1 or SHA-256.
>
> Some of these tests were failing for the wrong reasons. The first one
> being modified here would fail because the line starts with "xxxxxx"
> instead of "object", the rest of the line doesn't matter.
>
> Let's just put a valid hash on the rest of the line anyway to narrow
> the test down for just the s/object/xxxxxx/ case.
>
> The second one being modified here would fail under
> GIT_TEST_DEFAULT_HASH=sha256 because <some sha-1 length garbage> is an
> invalid SHA-256, but we should really be testing <some sha-256 length
> garbage> when under SHA-256.
>
> This doesn't really matter since we should be able to trust other
> parts of the code to validate things in the 0-9a-f range, but let's
> keep it for good measure.
>
> There's a later test which tests an invalid SHA which looks like a
> valid one, to stress the "We refuse to tag something we can't
> verify[...]" logic in mktag.c.
>
> But here we're testing for a SHA-length string which contains
> characters outside of the /[0-9a-f]/i set.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  t/t3800-mktag.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
> index 0e411e3c45..e9d7799537 100755
> --- a/t/t3800-mktag.sh
> +++ b/t/t3800-mktag.sh
> @@ -43,7 +43,7 @@ check_verify_failure 'Tag object length check' \
>  #  2. object line label check
>  
>  cat >tag.sig <<EOF
> -xxxxxx 139e9b33986b1c2670fff52c5067603117b3e895
> +xxxxxx $head
>  type tag
>  tag mytag
>  tagger . <> 0 +0000

These steps to prepare the input and the expected output outside the
test_expect_* block look too old-fashioned but they are consistently
so, so let's let them pass, at least inside this series.

> @@ -53,10 +53,10 @@ EOF
>  check_verify_failure '"object" line label check' '^error: char0: .*"object "$'
>  
>  ############################################################
> -#  3. object line SHA1 check
> +#  3. object line SHA check

Shouldn't we say hash instead of SHA for consistency?

>  
>  cat >tag.sig <<EOF
> -object zz9e9b33986b1c2670fff52c5067603117b3e895
> +object $(echo $head | tr 0-9a-f z)
>  type tag
>  tag mytag
>  tagger . <> 0 +0000