From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "Martin Ågren" <martin.agren@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
"SZEDER Gábor" <szeder.dev@gmail.com>,
"Emily Shaffer" <emilyshaffer@google.com>,
"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH] t0091-bugreport.sh: actually verify some content of report
Date: Tue, 13 Apr 2021 21:27:33 +0200 [thread overview]
Message-ID: <87a6q22dei.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CAN0heSr3MQnqMiWiSYpmpx3ZRkyXK6z1VfW_T-EQi31ifs0drA@mail.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).
next prev parent reply other threads:[~2021-04-13 19:27 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-16 21:18 [PATCH v13 0/5] git-bugreport with fixed VS build Emily Shaffer
2020-04-16 21:18 ` [PATCH v13 1/5] help: move list_config_help to builtin/help Emily Shaffer
2020-04-16 22:21 ` Junio C Hamano
2020-04-16 22:28 ` Junio C Hamano
2020-04-17 19:36 ` Emily Shaffer
2020-04-17 2:04 ` Danh Doan
2020-04-17 2:11 ` Danh Doan
2021-04-08 21:29 ` [PATCH] Makefile: add missing dependencies of 'config-list.h' SZEDER Gábor
2021-04-08 22:08 ` Ævar Arnfjörð Bjarmason
2021-04-08 23:40 ` Jeff King
2021-04-09 21:20 ` SZEDER Gábor
2021-04-16 19:03 ` Junio C Hamano
2021-04-16 21:33 ` SZEDER Gábor
2021-04-16 22:25 ` Junio C Hamano
2021-04-13 19:07 ` Ævar Arnfjörð Bjarmason
2020-04-16 21:18 ` [PATCH v13 2/5] bugreport: add tool to generate debugging info Emily Shaffer
2020-08-12 15:53 ` SZEDER Gábor
2021-04-08 21:36 ` SZEDER Gábor
2020-04-16 21:18 ` [PATCH v13 3/5] bugreport: gather git version and build info Emily Shaffer
2020-04-16 21:18 ` [PATCH v13 4/5] bugreport: add uname info Emily Shaffer
2021-04-08 22:19 ` Ævar Arnfjörð Bjarmason
2021-04-08 22:25 ` Junio C Hamano
2021-04-08 22:33 ` Ævar Arnfjörð Bjarmason
2021-04-08 23:41 ` Emily Shaffer
2021-04-08 23:58 ` Junio C Hamano
2021-04-09 21:27 ` SZEDER Gábor
2021-04-11 14:33 ` [PATCH] t0091-bugreport.sh: actually verify some content of report Martin Ågren
2021-04-12 17:17 ` Junio C Hamano
2021-04-13 18:32 ` Martin Ågren
2021-04-13 19:27 ` Ævar Arnfjörð Bjarmason [this message]
2021-04-13 22:21 ` Emily Shaffer
2023-07-01 19:26 ` [PATCH v2] " Martin Ågren
2023-07-03 15:47 ` Phillip Wood
2023-07-05 18:31 ` Martin Ågren
2023-07-05 18:40 ` [PATCH v3] " Martin Ågren
2023-07-05 19:46 ` Phillip Wood
2021-04-13 19:44 ` [PATCH] " SZEDER Gábor
2020-04-16 21:18 ` [PATCH v13 5/5] bugreport: add compiler info Emily Shaffer
2021-04-08 22:23 ` Ævar Arnfjörð Bjarmason
2021-04-08 22:59 ` Đoàn Trần Công Danh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a6q22dei.fsf@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=martin.agren@gmail.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).