On Tue, Apr 13 2021, Martin Ågren wrote:
> On Mon, 12 Apr 2021 at 19:17, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>
>> Martin Ågren <martin.agren@xxxxxxxxx> 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).
