Subject: Re: [PATCH v4 09/20] mktag tests: stress test whitespace handling

From: Junio C Hamano <[email-masked]>
Date: Tue, 22 Dec 2020 18:27:58 -0800

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

> +cat >tag.sig <<EOF
> +object $head
> +type commit
> +tag mytag
> +tagger T A Gger <[email-masked]> 1206478233 -0500
> +
> +
> +this line comes after an extra newline
> +EOF
> +
> +test_expect_success 'allow extra newlines at start of body' '
> +	git mktag <tag.sig
> +'

OK.

> +cat >tag.sig <<EOF
> +object $head
> +type commit
> +tag mytag
> +tagger T A Gger <[email-masked]> 1206478233 -0500
> +
> +EOF
> +
> +test_expect_success 'allow extra newlines at end of headers' '
> +	git mktag <tag.sig
> +'

I am not sure what the contrast between "at start of body" and "at
end of headers" means.

Ahh, OK, we are making sure that we "allow a blank line after the
headers before an empty body".

Makes sense (but see below).

> +space=' '
> +cat >tag.sig <<EOF
> +object $head
> +type commit
> +tag mytag
> +tagger T A Gger <[email-masked]> 1206478233 -0500$space
> +
> +EOF
> +
> +check_verify_failure 'extra whitespace at end of headers' \
> +	'^error: char.*: malformed tag timezone$'

OK.  Presumably all the other header lines forbid trailing space
as a malformed line?

> +cat >tag.sig <<EOF
> +object $head
> +type commit
> +tag mytag
> +tagger T A Gger <[email-masked]> 1206478233 -0500
> +EOF
> +
> +check_verify_failure 'disallow no header / body newline separator' \
> +	'^error: char.*: trailing garbage in tag header$'

Now I am confused.  This fails because...?   We earlier saw that we
"allow" a blank line after the header when no body is present, but
this says the test fails if a blank line exists after the header
when no body exists.  I do not have any objection to the behaviour
of the command, but doesn't this mean the earlier test was not
labelled correctly?  It may make more sense to move this test next
to the earlier one, and name them

    "require a blank line before even an empty body (1)"
    "require a blank line before even an empty body (2)"

or something?  The earlier one ensures that a payload with such a
blank line is OK, and this one ensures that a payload without such a
blank line causes a failure.

Thanks.

> +
>  ############################################################
>  # 24. create valid tag