Thread (17 messages) 17 messages, 3 authors, 2020-05-11

Re: [PATCH v4] bugreport: collect list of populated hooks

From: Emily Shaffer <hidden>
Date: 2020-05-11 21:22:15

On Fri, May 08, 2020 at 08:34:05AM +0700, Đoàn Trần Công Danh wrote:
quoted hunk ↗ jump to hunk
On 2020-05-07 17:53:57-0700, Emily Shaffer [off-list ref] wrote:
quoted
+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 <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help