Re: [PATCH 03/10] t4018: an infrastructure to test hunk headers

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 03/10] t4018: an infrastructure to test hunk headers

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:00:25

Johannes Sixt [off-list ref] writes:
Add an infrastructure that simplifies adding new tests of the hunk
header regular expressions.

To add new tests, a file with the syntax to test can be dropped in the
directory t4018. The README file explains how a test file must contain;
s/how/what/, or "how a test file must be written" you mean?
the README itself tests the default behavior.
Thanks.  Looks like a reasonable way to mark what must be found.
quoted hunk
Signed-off-by: Johannes Sixt <redacted>
---
 t/t4018-diff-funcname.sh | 60 +++++++++++++++++++++++++++++++++++++++++++-----
 t/t4018/README           | 18 +++++++++++++++
 2 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 t/t4018/README
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 38a092a..b467d9e 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -100,7 +100,25 @@ test_expect_funcname () {
 	grep "^@@.*@@ $1" diff
 }
 
-for p in ada bibtex cpp csharp fortran html java matlab objc pascal perl php python ruby tex
+diffpatterns="
+	ada
+	bibtex
+	cpp
+	csharp
+	fortran
+	html
+	java
+	matlab
+	objc
+	pascal
+	perl
+	php
+	python
+	ruby
+	tex
+"
+
+for p in $diffpatterns
 do
 	test_expect_success "builtin $p pattern compiles" '
 		echo "*.java diff=$p" >.gitattributes &&
I always found this "Let's apply rules for language $p to these
*.java files" strange.  I have wonder if it makes sense to further
change the framework to read the name of the rule to be applied from
the file in t/t4018/ directory, instead of using filename that is
the same as the name of the rule?  That way, you can list the files
in t/t4018/ directory to come up with the above list, without having
to maintain the list of rules separately like the above.
quoted hunk
@@ -118,11 +136,6 @@ do
 	'
 done
 
-test_expect_success 'default behaviour' '
-	rm -f .gitattributes &&
-	test_expect_funcname "public class Beer\$"
-'
-
 test_expect_success 'set up .gitattributes declaring drivers to test' '
 	cat >.gitattributes <<-\EOF
 	*.java diff=java
@@ -182,4 +195,39 @@ test_expect_success 'alternation in pattern' '
 	test_expect_funcname "public static void main("
 '
 
+test_expect_success 'setup hunk header tests' '
+	for i in $diffpatterns
+	do
+		echo "$i-* diff=$i"
+	done > .gitattributes &&
I like that you can have more than one test for each language/rule
this way, allowing you to test one kind of breakage without getting
affected by lines prepared for other tests in the same file.
quoted hunk
+	# add all test files to the index
+	(
+		cd "$TEST_DIRECTORY"/t4018 &&
+		git --git-dir="$TRASH_DIRECTORY/.git" add .
+	) &&
+
+	# place modified files in the worktree
+	for i in $(git ls-files)
+	do
+		sed -e "s/ChangeMe/IWasChanged/" <"$TEST_DIRECTORY/t4018/$i" >"$i" || return 1
+	done
+'
+
+# check each individual file
+for i in $(git ls-files)
+do
+	if grep broken "$i" >/dev/null 2>&1
+	then
+		result=failure
+	else
+		result=success
+	fi
+	test_expect_$result "hunk header: $i" "
+		test_when_finished 'cat actual' &&	# for debugging only
+		git diff -U1 $i >actual &&
+		grep '@@ .* @@.*RIGHT' actual
+	"
+done
+
 test_done
diff --git a/t/t4018/README b/t/t4018/README
new file mode 100644
index 0000000..283e01cc
--- /dev/null
+++ b/t/t4018/README
@@ -0,0 +1,18 @@
+How to write RIGHT test cases
+=============================
+
+Insert the word "ChangeMe" (exactly this form) at a distance of
+at least two lines from the line that must appear in the hunk header.
+
+The text that must appear in the hunk header must contain the word
+"right", but in all upper-case, like in the title above.
+
+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.
+
+BTW, this tests that the head line goes to the hunk header, not the line
+of equal signs.

Re: [PATCH 03/10] t4018: an infrastructure to test hunk headers

From: Johannes Sixt <hidden>
Date: 2016-06-15 23:00:25

Am 21.03.2014 23:00, schrieb Junio C Hamano:
Johannes Sixt [off-list ref] writes:
quoted
Add an infrastructure that simplifies adding new tests of the hunk
header regular expressions.

To add new tests, a file with the syntax to test can be dropped in the
directory t4018. The README file explains how a test file must contain;
s/how/what/, or "how a test file must be written" you mean?
I do mean one of those, preferably the latter ;)
quoted
-for p in ada bibtex cpp csharp fortran html java matlab objc pascal perl php python ruby tex
+diffpatterns="
+	ada
+	bibtex
+	cpp
+	csharp
+	fortran
+	html
+	java
+	matlab
+	objc
+	pascal
+	perl
+	php
+	python
+	ruby
+	tex
+"
+
+for p in $diffpatterns
 do
 	test_expect_success "builtin $p pattern compiles" '
 		echo "*.java diff=$p" >.gitattributes &&
I always found this "Let's apply rules for language $p to these
*.java files" strange.  I have wonder if it makes sense to further
change the framework to read the name of the rule to be applied from
the file in t/t4018/ directory, instead of using filename that is
the same as the name of the rule?  That way, you can list the files
in t/t4018/ directory to come up with the above list, without having
to maintain the list of rules separately like the above.
I see what you mean. Notice, however, that we also test whether the
regular expressions can be compiled successfully, and for this it is
desirable to have the complete list of userdiff drivers. Until we have
at least one test-case for each driver, we wouldn't get the complete
list. In summary, I do agree that a bit more automation is also
desirable, but we are not there, yet.

-- Hannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help