On Fri, May 08, 2020 at 08:34:05AM +0700, Đoàn Trần Công Danh wrote:
>
> On 2020-05-07 17:53:57-0700, Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote:
> > +test_expect_success 'indicates populated hooks' '
> > + test_when_finished rm git-bugreport-hooks.txt &&
> > + test_when_finished rm -fr .git/hooks &&
> > + rm -fr .git/hooks &&
> > + mkdir .git/hooks &&
> > + for hook in applypatch-msg prepare-commit-msg.sample
> > + do
> > + write_script ".git/hooks/$hook" <<-\EOF || return 1
> > + echo "hook $hook exists"
> > + EOF
> > + done &&
> > + git bugreport -s hooks &&
> > + grep applypatch-msg git-bugreport-hooks.txt &&
> > + ! grep prepare-commit-msg git-bugreport-hooks.txt
>
> Hi Emily,
>
> I think this is a bit more correct test.
> ---------8<----------
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Use an exact match to check for populated hooks
>
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx>
> ---
> t/t0091-bugreport.sh | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> index 9450cc02e3..789e8f1ac7 100755
> --- a/t/t0091-bugreport.sh
> +++ b/t/t0091-bugreport.sh
> @@ -67,8 +67,13 @@ test_expect_success 'indicates populated hooks' '
> EOF
> done &&
> git bugreport -s hooks &&
> - grep applypatch-msg git-bugreport-hooks.txt &&
> - ! grep prepare-commit-msg git-bugreport-hooks.txt
> + cat <<-\EOF >expected &&
> + applypatch-msg
> +
> + EOF
> + awk -F "]\\n" -v RS="[" "/applypatch-msg/{print \$2}" \
> + git-bugreport-hooks.txt >actual &&
If I understand correctly, you are saying "look for a line like [...]
which is followed by a line that says 'applypatch-msg'" - that is,
making sure that you don't see some false positive should
"applypatch-msg" exist in the rest of the bugreport.
Could we compromise and grep for "^applypatch-msg$", e.g. "there is a
line calling out applypatch-msg in some way that isn't in the context of
the report template"?
- Even though regex magic is used, ^$ are beginner regex that many
people can understand easily.
- Using grep for a single line means we are not allergic to header
format changes later on
It doesn't search for false positives, true, but I found your awk
suggestion hard to understand - my impression is that awk is less
commonly understood, even among Git contributors.
In sed, it's a little bit more readable:
cat <<-\EOF >expected &&
[Enabled Hooks]
applypatch-msg
EOF
sed -n '/\[Enabled Hooks\]/, /^$/ p' git-bugreport-hooks.txt >actual
test_cmp expected actual
But, I don't like this because it relies on the name of the header, and
the newline spacing between sections - and would be nontrivial to change
if we decided to underline headers instead, or add a newline between a
header and its contents. So I think it may be overkill.
Thanks for your suggestion, though.
- Emily
