Re: [PATCH] t4018: introduce test cases for the internal hunk header patterns

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

Re: [PATCH] t4018: introduce test cases for the internal hunk header patterns

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:40

Brandon Casey [off-list ref] writes:

quoted

   * after the line that contains "RIGHT" token, there should be one (and
     only one) line that contains "ChangeMe". The test modifies this
     token to "IWasChanged", compares the original with the modified
     result, and expects the "RIGHT" token above appears on the hunk
     header.

Both good improvements.

Let's not call "ChangeMe" a *token*, as I think the easiest example would
look like the following and it would not use it as such.  Rephrasing it as
a "string" would be better.

    @@ ... @@ int RIGHT_function_hunk_header(void)

     int RIGHT_function_hunk_header(void)
     {
    -    const char *msg = "ChangeME";
    +    const char *msg = "IWasChanged";
         printf("Hello, world, %s\n", msg);
	 return 0;
     }
    @@ ...

[PATCH v2] t4018: introduce test cases for the internal hunk header patterns

From: Brandon Casey <hidden>
Date: 2016-06-15 22:52:40

Recently it has been pointed out that one (or more?) of the internal hunk
header patterns is sub-optimal.  Specifically, the C/C++ "cpp" pattern was
called out.

Let's introduce some infrastructure to make it easy to create test cases
for the hunk header patterns and provide a few cases for the cpp pattern.

   * new test cases can be dropped into the t4018 directory
   * filenames end with the pattern name e.g. .cpp .objc .matlab etc.
   * filenames should be descriptive since it will be used in the test
     suite output
   * broken test cases should be given a filename prefixed with "broken_"
   * a test case must have one (and only one) line that contains "RIGHT"
     (all uppercase) and that line should become the hunk header for the
     test to succeed
   * after the line that contains "RIGHT" token, there should be one (and
     only one) line that contains the string "ChangeMe". The test modifies
     this string to "IWasChanged", compares the original with the modified
     result, and expects the "RIGHT" token above to appear on the hunk
     header

Example test case:

   int WRONG_function_hunk_header (void)
   {
           return 0;
   }

   int RIGHT_function_hunk_header (void)
   {
           const char *msg = "ChangeMe";
           printf("Hello, world, %s\n", msg);
           return 0;
   }

Signed-off-by: Brandon Casey <redacted>
---


Updated based on Junio's comments and squashed the additional tests I
sent.  Plus I added -U1 to the git diff line so that the filler lines are
no longer necessary.

-Brandon


 t/t4018-diff-funcname.sh             |   18 ++++++++++++++++++
 t/t4018/broken_class_constructor.cpp |   16 ++++++++++++++++
 t/t4018/broken_class_destructor.cpp  |   16 ++++++++++++++++
 t/t4018/broken_gnu_style.cpp         |   17 +++++++++++++++++
 t/t4018/broken_reference.cpp         |   16 ++++++++++++++++
 t/t4018/broken_template.cpp          |   16 ++++++++++++++++
 t/t4018/class_method.cpp             |   16 ++++++++++++++++
 t/t4018/ignore_declaration.cpp       |   17 +++++++++++++++++
 t/t4018/ignore_global.cpp            |   19 +++++++++++++++++++
 t/t4018/ignore_label.cpp             |   17 +++++++++++++++++
 t/t4018/pointer_return.cpp           |   16 ++++++++++++++++
 t/t4018/simple.cpp                   |   32 ++++++++++++++++++++++++++++++++
 t/t4018/static.cpp                   |   16 ++++++++++++++++
 13 files changed, 232 insertions(+), 0 deletions(-)
 create mode 100644 t/t4018/broken_class_constructor.cpp
 create mode 100644 t/t4018/broken_class_destructor.cpp
 create mode 100644 t/t4018/broken_gnu_style.cpp
 create mode 100644 t/t4018/broken_reference.cpp
 create mode 100644 t/t4018/broken_template.cpp
 create mode 100644 t/t4018/class_method.cpp
 create mode 100644 t/t4018/ignore_declaration.cpp
 create mode 100644 t/t4018/ignore_global.cpp
 create mode 100644 t/t4018/ignore_label.cpp
 create mode 100644 t/t4018/pointer_return.cpp
 create mode 100644 t/t4018/simple.cpp
 create mode 100644 t/t4018/static.cpp

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index 4bd2a1c..a3c4577 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh

@@ -121,6 +121,24 @@ do
 		! grep fatal msg &&
 		! grep error msg
 	'
+
+	for f in "$TEST_DIRECTORY"/t4018/*.$p; do
+		test -f "$f" || continue
+		name=`basename "$f" .$p`
+		test_expect_which=test_expect_success
+		if test "$name" != "${name#broken_}"; then
+			name=${name#broken_}
+			test_expect_which=test_expect_failure
+		fi
+		$test_expect_which \
+			"builtin $p pattern works correctly for $name case" "
+			echo \"*.$p diff=$p\" >.gitattributes &&
+			sed -e 's/ChangeMe/IWasChanged/' < \"$f\" > \"$name.$p\" &&
+			test_expect_code 1 git diff -U1 --no-index \"$f\" \
+				\"$name.$p\" >actual &&
+			egrep '^@@ .* @@ .*RIGHT' actual
+		"
+	done
 done
 
 test_expect_success 'default behaviour' '

diff --git a/t/t4018/broken_class_constructor.cpp b/t/t4018/broken_class_constructor.cpp
new file mode 100644
index 0000000..774c7f9
--- /dev/null
+++ b/t/t4018/broken_class_constructor.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+RIGHT_function_hunk_header::RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/broken_class_destructor.cpp b/t/t4018/broken_class_destructor.cpp
new file mode 100644
index 0000000..3045fd1
--- /dev/null
+++ b/t/t4018/broken_class_destructor.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+RIGHT_function_hunk_header::~RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/broken_gnu_style.cpp b/t/t4018/broken_gnu_style.cpp
new file mode 100644
index 0000000..58e574a
--- /dev/null
+++ b/t/t4018/broken_gnu_style.cpp

@@ -0,0 +1,17 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int
+RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/broken_reference.cpp b/t/t4018/broken_reference.cpp
new file mode 100644
index 0000000..4d4549f
--- /dev/null
+++ b/t/t4018/broken_reference.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int& RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/broken_template.cpp b/t/t4018/broken_template.cpp
new file mode 100644
index 0000000..5c62e73
--- /dev/null
+++ b/t/t4018/broken_template.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+template <class T> int RIGHT_function_hunk_header (T unused)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/class_method.cpp b/t/t4018/class_method.cpp
new file mode 100644
index 0000000..fe53620
--- /dev/null
+++ b/t/t4018/class_method.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int test_class::RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/ignore_declaration.cpp b/t/t4018/ignore_declaration.cpp
new file mode 100644
index 0000000..ce7a0f6
--- /dev/null
+++ b/t/t4018/ignore_declaration.cpp

@@ -0,0 +1,17 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int RIGHT_function_hunk_header (void)
+{
+	void WRONG_function_declaration_within_body (void);
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/ignore_global.cpp b/t/t4018/ignore_global.cpp
new file mode 100644
index 0000000..95e23bc
--- /dev/null
+++ b/t/t4018/ignore_global.cpp

@@ -0,0 +1,19 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int RIGHT_function_hunk_header (void)
+{
+	printf("Hello, world\n");
+	return 0;
+}
+
+int WRONG_global_variable;
+
+int ChangeMe;
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/ignore_label.cpp b/t/t4018/ignore_label.cpp
new file mode 100644
index 0000000..a8f407d
--- /dev/null
+++ b/t/t4018/ignore_label.cpp

@@ -0,0 +1,17 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int RIGHT_function_hunk_header (void)
+{
+WRONG_should_not_match_label:
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/pointer_return.cpp b/t/t4018/pointer_return.cpp
new file mode 100644
index 0000000..ea30d2d
--- /dev/null
+++ b/t/t4018/pointer_return.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+static int *RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/simple.cpp b/t/t4018/simple.cpp
new file mode 100644
index 0000000..c96ad87
--- /dev/null
+++ b/t/t4018/simple.cpp

@@ -0,0 +1,32 @@
+/*
+ *  Test file for testing the internal hunk header patterns
+ *
+ *  The "RIGHT" hunk header function, the one that should appear on the
+ *  hunk header line, should be named "RIGHT_function_hunk_header" and
+ *  the body of this function should have an assignment that looks like
+ *
+ *     answer = 0
+ *
+ *  within it, deep enough so the lines of context do not include the
+ *  function name.
+ *
+ *  If the name of this file begins with "broken_", then it will be
+ *  interpreted as a pattern which does not work, but which should.
+ */
+
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/static.cpp b/t/t4018/static.cpp
new file mode 100644
index 0000000..f6ee0f3
--- /dev/null
+++ b/t/t4018/static.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+static int RIGHT_function_hunk_header (void)
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

-- 
1.7.8

Re: [PATCH v2] t4018: introduce test cases for the internal hunk header patterns

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:52:40

Brandon Casey [off-list ref] writes:

quoted hunk

diff --git a/t/t4018/ignore_global.cpp b/t/t4018/ignore_global.cpp
new file mode 100644
index 0000000..95e23bc
--- /dev/null
+++ b/t/t4018/ignore_global.cpp

@@ -0,0 +1,19 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+int RIGHT_function_hunk_header (void)
+{
+	printf("Hello, world\n");
+	return 0;
+}
+
+int WRONG_global_variable;
+
+int ChangeMe;
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

Shouldn't ChangeMe be inside function?

quoted hunk

diff --git a/t/t4018/simple.cpp b/t/t4018/simple.cpp
new file mode 100644
index 0000000..c96ad87
--- /dev/null
+++ b/t/t4018/simple.cpp

@@ -0,0 +1,32 @@
+/*
+ *  Test file for testing the internal hunk header patterns
+ *
+ *  The "RIGHT" hunk header function, the one that should appear on the
+ *  hunk header line, should be named "RIGHT_function_hunk_header" and
+ *  the body of this function should have an assignment that looks like
+ *
+ *     answer = 0

Shouldn't it be ChangeMe?

+ *
+ *  within it, deep enough so the lines of context do not include the
+ *  function name.
+ *
+ *  If the name of this file begins with "broken_", then it will be
+ *  interpreted as a pattern which does not work, but which should.
+ */

-- 
Jakub Narêbski

Re: [PATCH v2] t4018: introduce test cases for the internal hunk header patterns

From: Brandon Casey <hidden>
Date: 2016-06-15 22:52:40

On Tue, Dec 20, 2011 at 2:25 AM, Jakub Narebski [off-list ref] wrote:

Brandon Casey [off-list ref] writes:

quoted

diff --git a/t/t4018/ignore_global.cpp b/t/t4018/ignore_global.cpp
new file mode 100644
index 0000000..95e23bc
--- /dev/null
+++ b/t/t4018/ignore_global.cpp

@@ -0,0 +1,19 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+     return 0;
+}
+
+int RIGHT_function_hunk_header (void)
+{
+     printf("Hello, world\n");
+     return 0;
+}
+
+int WRONG_global_variable;
+
+int ChangeMe;
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+     return 0;
+}

Shouldn't ChangeMe be inside function?

No, this one is placed here after the WRONG_global_variable to make
sure that a global variable is not chosen for the hunk header.  It
should chose the most recently encountered function name for the hunk
header, which is RIGHT_function_hunk_header.

quoted

diff --git a/t/t4018/simple.cpp b/t/t4018/simple.cpp
new file mode 100644
index 0000000..c96ad87
--- /dev/null
+++ b/t/t4018/simple.cpp

@@ -0,0 +1,32 @@
+/*
+ *  Test file for testing the internal hunk header patterns
+ *
+ *  The "RIGHT" hunk header function, the one that should appear on the
+ *  hunk header line, should be named "RIGHT_function_hunk_header" and
+ *  the body of this function should have an assignment that looks like
+ *
+ *     answer = 0

Shouldn't it be ChangeMe?

Whoops, I forgot about that text.  Thanks for noticing.  Yes this is
incorrect now.  I think I'll break that out into a README file.

v3 forthcoming.

-Brandon

Re: [PATCH v2] t4018: introduce test cases for the internal hunk header patterns

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:52:40

Am 20.12.2011 03:42, schrieb Brandon Casey:

+int WRONG_global_variable;

Personally, I'm not a fan of this one. IMHO, global variable
definitions need not be ignored. (But I can live with it.)

But here are some more tests that I care about and that you can squash in.

Signed-off-by: Johannes Sixt <redacted>
---
 t/t4018/broken_class_constructor.cpp               |    3 +-
 t/t4018/broken_return_type_in_global_namespace.cpp |   16 ++++++++++++++
 t/t4018/class_definition.cpp                       |   19 +++++++++++++++++
 t/t4018/derived_class_definition.cpp               |   20 ++++++++++++++++++
 t/t4018/ignore_access_specifier.cpp                |   22 ++++++++++++++++++++
 5 files changed, 79 insertions(+), 1 deletions(-)
 create mode 100644 t/t4018/broken_return_type_in_global_namespace.cpp
 create mode 100644 t/t4018/class_definition.cpp
 create mode 100644 t/t4018/derived_class_definition.cpp
 create mode 100644 t/t4018/ignore_access_specifier.cpp

diff --git a/t/t4018/broken_class_constructor.cpp b/t/t4018/broken_class_constructor.cpp
index 774c7f9..28c1bc8 100644
--- a/t/t4018/broken_class_constructor.cpp
+++ b/t/t4018/broken_class_constructor.cpp

@@ -3,7 +3,8 @@ int WRONG_function_hunk_header_preceding_the_right_one (void)
 	return 0;
 }
 
-RIGHT_function_hunk_header::RIGHT_function_hunk_header (void)
+RIGHT_function_hunk_header::RIGHT_function_hunk_header (int x) :
+	WRONG_member_initializer(x)
 {
 	const char *msg = "ChangeMe";
 	printf("Hello, world, %s\n", msg);

diff --git a/t/t4018/broken_return_type_in_global_namespace.cpp b/t/t4018/broken_return_type_in_global_namespace.cpp
new file mode 100644
index 0000000..da9931b
--- /dev/null
+++ b/t/t4018/broken_return_type_in_global_namespace.cpp

@@ -0,0 +1,16 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+::TypeInGlobalNamespace RIGHT_function_hunk_header()
+{
+	const char *msg = "ChangeMe";
+	printf("Hello, world, %s\n", msg);
+	return 0;
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/class_definition.cpp b/t/t4018/class_definition.cpp
new file mode 100644
index 0000000..fc3df34
--- /dev/null
+++ b/t/t4018/class_definition.cpp

@@ -0,0 +1,19 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+class RIGHT_class_name
+{
+	void WRONG_function_name_following_the_right_one()
+	{
+		const char *msg = "ChangeMe";
+		printf("Hello, world, %s\n", msg);
+		return 0;
+	}
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/derived_class_definition.cpp b/t/t4018/derived_class_definition.cpp
new file mode 100644
index 0000000..b8501a5
--- /dev/null
+++ b/t/t4018/derived_class_definition.cpp

@@ -0,0 +1,20 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+class RIGHT_class_name :
+	public WRONG_class_name_following_the_right_one
+{
+	void WRONG_function_name_following_the_right_one()
+	{
+		const char *msg = "ChangeMe";
+		printf("Hello, world, %s\n", msg);
+		return 0;
+	}
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

diff --git a/t/t4018/ignore_access_specifier.cpp b/t/t4018/ignore_access_specifier.cpp
new file mode 100644
index 0000000..d865040
--- /dev/null
+++ b/t/t4018/ignore_access_specifier.cpp

@@ -0,0 +1,22 @@
+int WRONG_function_hunk_header_preceding_the_right_one (void)
+{
+	return 0;
+}
+
+class RIGHT_class_name
+{
+public:
+protected:
+private:
+	void WRONG_function_name_following_the_right_one()
+	{
+		const char *msg = "ChangeMe";
+		printf("Hello, world, %s\n", msg);
+		return 0;
+	}
+}
+
+int WRONG_function_hunk_header_following_the_right_one (void)
+{
+	return 0;
+}

-- 
1.7.8.216.g2e426

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help