Subject: Re: [PATCH v2 09/27] userdiff tests: match full hunk headers

From: Junio C Hamano <[email-masked]>
Date: Tue, 16 Feb 2021 10:32:33 -0800

Johannes Sixt <[email-masked]> writes:

>>   t/t4018-diff-funcname.sh                      |  7 +++---
>>   t/t4018/README                                | 22 +++++++++----------
>>   t/t4018/README.ctx                            |  1 +
>>   t/t4018/bash-arithmetic-function.ctx          |  1 +
>>   t/t4018/bash-bashism-style-compact.ctx        |  1 +
>>   [...and so on...]
>
> This is what I meant by "without burdening test writers with lots of
> subtleties".
>
> I'm not a friend of this change :-(
>
> I think you are going overboard with required test precision. To have
> useful tests for userdiff patterns that demonstrate its features, 
> authors should write *many* tests. The right balance should be on the
> coverage of userdiff pattern features, not on the subtle details of
> each and everyone of it. Requiring that many additional context files
> makes it *really hard* to comply.

Yeah, the first time I saw the t4018 test framework appeared in my
tree, I truly appreciated its simplicity, how the test input file is
self-documenting and self-contained, with the clever use of "RIGHT",
"broken" and "ChangeMe" magic tokens, admired the cleverness of the
approach, and wished I was clever enough to invent that pattern to
apply to other tests myself.

A little new for each and every test for the miniscule gain of
checking which part of the function header line is extracted feels a
bit too much noise and rubs my sense of aesthetics, spoiled by the
existing t4018 tests, the wrong way.

This is a rough sketch of a different approach aiming for the same.
I converted only a few files, but I hope that this is enough to
illustrate the idea.

 t/t4018-diff-funcname.sh         | 17 ++++++++++++++---
 t/t4018/README                   |  9 ++++++---
 t/t4018/bash-arithmetic-function |  3 +++
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git c/t/t4018-diff-funcname.sh w/t/t4018-diff-funcname.sh
index 9675bc17db..dd79c99fc5 100755
--- c/t/t4018-diff-funcname.sh
+++ w/t/t4018-diff-funcname.sh
@@ -107,10 +107,21 @@ do
 	else
 		result=success
 	fi
-	test_expect_$result "hunk header: $i" "
+
+	test_expect_$result "hunk header: $i" '
+		HEAD=$(sed -n \
+			-e "s/^.*HEADER.*|\(.*\)right\(.*\)|.*/ \1RIGHT\2/p" "$i") &&
+
 		git diff -U1 $i >actual &&
-		grep '@@ .* @@.*RIGHT' actual
-	"
+
+		sed -ne "s/^@@[^@]*@@//p" actual |
+		if test -n "$HEAD"
+		then
+			grep -F "$HEAD"
+		else
+			grep "^.*RIGHT"
+		fi
+	'
 done
 
 test_done
diff --git c/t/t4018/README w/t/t4018/README
index 283e01cca1..bc3c11e083 100644
--- c/t/t4018/README
+++ w/t/t4018/README
@@ -10,9 +10,12 @@ The text that must appear in the hunk header must contain the word
 To mark a test case that highlights a malfunction, insert the word
 BROKEN in all lower-case somewhere in the file.
 
-This text is a bit twisted and out of order, but it is itself a
-test case for the default hunk header pattern. Know what you are doing
-if you change it.
+This text is a bit twisted and out of order, but it is itself a test
+case for the default hunk header pattern. Know what you are doing if
+you change it.  You can optionally force exactly what should be on
+the hunk header line by enclosing the expect text by vertical bars
+(but downcasing the word right in it) and place it on a line with
+HEADER on it, like |How to write right test cases|.
 
 BTW, this tests that the head line goes to the hunk header, not the line
 of equal signs.
diff --git c/t/t4018/bash-arithmetic-function w/t/t4018/bash-arithmetic-function
index c0b276cb50..935f18d96d 100644
--- c/t/t4018/bash-arithmetic-function
+++ w/t/t4018/bash-arithmetic-function
@@ -2,3 +2,6 @@ RIGHT() ((
 
     ChangeMe = "$x" + "$y"
 ))
+
+
+# HEADER |right()|