Re: [PATCH] t0091-bugreport.sh: actually verify some content of report
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 12, 2021, 17:17 UTC
- Message-ID
- <xmqqwnt7e82i.fsf@gitster.g>
- In-Reply-To
- <20210411143354.25134-1-martin.agren@gmail.com>
Martin Ågren <martin.agren@gmail.com> writes:
Show 36 quoted lines
> 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.
Show 16 quoted lines
> 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.