From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 03 Jul 2023 15:47:53 GMT
Subject: Re: [PATCH v2] t0091-bugreport.sh: actually verify some content of report
Message-ID: <2f516828-deee-5c78-a08d-7a64e457f91a@gmail.com>
In-Reply-To: <20230701192642.647167-1-martin.agren@gmail.com>

Hi Martin

On 01/07/2023 20:26, Martin Ågren wrote:
> 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.41.0.2.gfb7d80edca: No such file or directory
> 
> This doesn't disturb the test, which tugs along and reports success, not
> really having verified the contents of the report at all.

Thanks for the clear description of the problem and for fixing it

> 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 split this test into three: first verify that we generate
> a report at all, then check that the introductory blurb looks the way it
> should, then verify that the "[System Info]" seems to contain the right
> things. (The "[Enabled Hooks]" section is tested later in the script.)

That sounds like a good plan

> +test_expect_success 'report contains wanted template (before first section)' '
> +	awk "/^\[/ { exit } { print }" git-bugreport-format.txt >actual &&

Personally I'd find

	sed -n -e '/^\[/q;p' git-bugreport-format.txt >actual

easier to understand but that's probably because I don't use awk very 
much. I'm not sure it is worth a re-roll though as I see we do use awk 
in a few of the other test scripts.

Best Wishes

Phillip