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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Apr 13 2021, Martin Ågren wrote:

> On Mon, 12 Apr 2021 at 19:17, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> 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:
> ...
>> > 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.
>
>> > +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.
>
> Maybe something like
>
>        awk '\''BEGIN { sect="" }
>                /^\[.*]$/ { sect=$0 }
>                /./ { print sect, $0 }'\'' \
>            git-bugreport-check-headers.txt >prefixed &&
>        grep "^ Thank you for filling out a Git bug report" prefixed &&
>        grep "^ Please review the rest of the bug report below" prefixed &&
>        grep "^ You can delete any lines you don.t wish to share" prefixed &&
>        grep "\[System Info\] git version ..." prefixed
>
> Something like that could be used to verify that a line goes into the
> right section, as opposed to just seeing that it appears *somewhere*. Or
> maybe
>
>   grep -e Thank.you -e Please.review -e You.can.delete -e "^\[" \
>        -e git.version git-bugreport-check-headers.txt >actual
>
> then setting up an "expect" and comparing. That would help us verify the
> order, including which section things appear in. Slightly less friendly
> for comparing loosely, compared to the awk-then-grep above.
>
> Let's see what Emily thinks about the various alternatives. Maybe she can
> think of something else.

I think a straight-up test_cmp is preferrable, both for correctness and
also as self-documentation, you can see from the test what the full
expected output is like.

Obviously in this case we can't do a test_cmp on the raw output, as it
contains various things from uname.

But it looks like we could do that if we do some light awk/perl/sed
munging of the "[System Info]" and "[Enabled Hooks]" section(s).

Or, since we also control the generator we could pass a --no-system-info
and/or --no-hooks-info, which may be something some people want for
privacy/reporting reasons anyway (e.g. I've often used perlbug and
deleted that whole info, because info there has no relevance to the
specific issue I'm reporting).