Re: [PATCH] t0091-bugreport.sh: actually verify some content of report

[Junio C Hamano <gitster@pobox.com>]

Martin Ågren <martin.agren@gmail.com> writes:

> In the first test in this script, 'creates a report with content in the
> right places', we generate a report and pipe it into our helper
> `check_all_headers_populated()`. The idea of the helper is to find all
> lines that look like headers ("[Some Header Here]") and to check that
> the next line is non-empty. This is supposed to catch erroneous outputs
> such as the following:
>
>   [A Header]
>   something
>   more here
>
>   [Another Header]
>
>   [Too Early Header]
>   contents
>
> However, we provide the lines of the bug report as filenames to grep,
> meaning we mostly end up spewing errors:
>
>   grep: : No such file or directory
>   grep: [System Info]: No such file or directory
>   grep: git version:: No such file or directory
>   grep: git version 2.31.1.164.g984c2561cd: No such file
>
> This doesn't disturb the test, which tugs along and reports success, not
> really having verified the contents of the report at all.
>
> Note that after 788a776069 ("bugreport: collect list of populated
> hooks", 2020-05-07), the bug report, which is created in our hook-less
> test repo, contains an empty section with the enabled hooks. Thus, even
> the intention of our helper is a bit misguided: there is nothing
> inherently wrong with having an empty section in the bug report.
>
> Let's instead grep for some contents that we expect to find in a bug
> report. We won't verify that they appear in the right order, but at
> least we end up verifying the contents more than before this commit.

Nicely described.  I agree that the original intent (let alone the
implementation) is misguided and we should allow an empty section as
a perfectly normal thing.

> Reported-by: SZEDER Gábor <szeder.dev@gmail.com>
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
>  > It does scare me..
>
>  Maybe something like this?
> ...
> +test_expect_success 'creates a report with content' '
>  	test_when_finished rm git-bugreport-check-headers.txt &&
>  	git bugreport -s check-headers &&
> -	check_all_headers_populated <git-bugreport-check-headers.txt
> +	grep "^Please answer " git-bugreport-check-headers.txt &&
> +	grep "^\[System Info\]$" git-bugreport-check-headers.txt &&
> +	grep "^git version:$" git-bugreport-check-headers.txt &&
> +	grep "^\[Enabled Hooks\]$" git-bugreport-check-headers.txt
>  '

It is a different matter if it is sufficient to ensure only certain
selected lines appear in the report, though.  As all the lines lost
by this fix comes from 238b439d (bugreport: add tool to generate
debugging info, 2020-04-16), it would be nice to hear from Emily.

Thanks.