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

On Tue, Apr 13, 2021 at 09:27:33PM +0200, Ævar Arnfjörð Bjarmason wrote:

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

quoted

On Mon, 12 Apr 2021 at 19:17, Junio C Hamano [off-list ref] wrote:

quoted

Martin Ågren [off-list ref] writes:

quoted

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:

...

quoted

quoted

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.

quoted

quoted

+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.

My apologies for the slow reply :)

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.

Yeah, I like the sound of this.

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).

This approach sounds more appealing to me than awk munging. I think
you're right that folks may not want to share it in some cases.

Thanks for noticing.
 - Emily