Re: [PATCH v2 09/27] userdiff tests: match full hunk headers
From: Junio C Hamano <hidden>
Date: 2021-02-16 18:33:22
Johannes Sixt [off-list ref] writes:
quoted
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()|