[PATCH v2] apply: add unit tests for parse_range

["Philip Peterson via GitGitGadget" <gitgitgadget@gmail.com>]

From: Philip Peterson <philip.c.peterson@gmail.com>

Also rename parse_range to parse_fragment_range for external linkage.

Signed-off-by: Philip Peterson <philip.c.peterson@gmail.com>
---
    apply: add unit tests for parse_range
    
    Add unit tests for apply's parse_range function. Also rename parse_range
    to parse_fragment_range and expose it externally for use by the unit
    tests. Internal callers may continue to refer to it by the name
    parse_range.
    
    It is necessary to make the function be non-internal linkage in order to
    expose it to the unit testing framework. Because there is another
    function in the codebase also called parse_range, change this one to a
    more specific name as well: parse_fragment_range. Also add several test
    cases (both positive and negative) for the function.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1677%2Fphilip-peterson%2Fpeterson%2Funit-tests-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1677/philip-peterson/peterson/unit-tests-v2
Pull-Request: https://github.com/git/git/pull/1677

Range-diff vs v1:

 1:  2c60c4406d4 < -:  ----------- apply: add unit tests for parse_range and rename to parse_fragment_range
 2:  7dab12ab7b8 ! 1:  c0d7b93e383 apply: rewrite unit tests with structured cases
     @@ Metadata
      Author: Philip Peterson <philip.c.peterson@gmail.com>
      
       ## Commit message ##
     -    apply: rewrite unit tests with structured cases
     +    apply: add unit tests for parse_range
      
     -    The imperative format was a little hard to read, so I rewrote the test cases
     -    in a declarative style by defining a common structure for each test case and
     -    its assertions.
     +    Also rename parse_range to parse_fragment_range for external linkage.
      
          Signed-off-by: Philip Peterson <philip.c.peterson@gmail.com>
      
     - ## t/unit-tests/t-apply.c ##
     + ## Makefile ##
     +@@ Makefile: THIRD_PARTY_SOURCES += compat/regex/%
     + THIRD_PARTY_SOURCES += sha1collisiondetection/%
     + THIRD_PARTY_SOURCES += sha1dc/%
     + 
     ++UNIT_TEST_PROGRAMS += t-apply
     + UNIT_TEST_PROGRAMS += t-ctype
     + UNIT_TEST_PROGRAMS += t-mem-pool
     + UNIT_TEST_PROGRAMS += t-prio-queue
     +
     + ## apply.c ##
      @@
     + #include "wildmatch.h"
     + #include "ws.h"
       
     - #define FAILURE -1
     ++#define parse_range apply_parse_fragment_range
     ++
     + struct gitdiff_data {
     + 	struct strbuf *root;
     + 	int linenr;
     +@@ apply.c: static int parse_num(const char *line, unsigned long *p)
     + 	return ptr - line;
     + }
       
     --static void setup_static(const char *line, int len, int offset,
     --						 const char *expect, int assert_result,
     --						 unsigned long assert_p1,
     --						 unsigned long assert_p2)
     +-static int parse_range(const char *line, int len, int offset, const char *expect,
     +-		       unsigned long *p1, unsigned long *p2)
     ++int apply_parse_fragment_range(const char *line, int len, int offset, const char *expect,
     ++			 unsigned long *p1, unsigned long *p2)
     + {
     + 	int digits, ex;
     + 
     +
     + ## apply.h ##
     +@@ apply.h: int apply_all_patches(struct apply_state *state,
     + 		      int argc, const char **argv,
     + 		      int options);
     + 
     ++/*
     ++ * exposed only for tests; do not call this as it not
     ++ * a part of the API
     ++ */
     ++extern int apply_parse_fragment_range(const char *line, int len, int offset,
     ++				      const char *expect, unsigned long *p1,
     ++				      unsigned long *p2);
     ++
     + #endif
     +
     + ## t/unit-tests/t-apply.c (new) ##
     +@@
     ++#include "test-lib.h"
     ++#include "apply.h"
     ++
     ++#define FAILURE -1
     ++
      +typedef struct test_case {
      +	const char *line;
      +	const char *expect_suffix;
     @@ t/unit-tests/t-apply.c
      +} test_case;
      +
      +static void setup_static(struct test_case t)
     - {
     - 	unsigned long p1 = 9999;
     - 	unsigned long p2 = 9999;
     --	int result = parse_fragment_range(line, len, offset, expect, &p1, &p2);
     --	check_int(result, ==, assert_result);
     --	check_int(p1, ==, assert_p1);
     --	check_int(p2, ==, assert_p2);
     -+	int result = parse_fragment_range(t.line, strlen(t.line), t.offset, t.expect_suffix, &p1, &p2);
     ++{
     ++	unsigned long p1 = 9999;
     ++	unsigned long p2 = 9999;
     ++	int result = apply_parse_fragment_range(t.line, strlen(t.line), t.offset,
     ++						t.expect_suffix, &p1, &p2);
      +	check_int(result, ==, t.expect_result);
      +	check_int(p1, ==, t.expect_p1);
      +	check_int(p2, ==, t.expect_p2);
     - }
     - 
     - int cmd_main(int argc, const char **argv)
     - {
     --	char* text;
     --	int expected_result;
     --
     --	/* Success */
     --	text = "@@ -4,4 +";
     --	expected_result = 9;
     --	TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
     --		 "well-formed range");
     ++}
     ++
     ++int cmd_main(int argc, const char **argv)
     ++{
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -4,4 +",
      +		.offset = 4,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 4,
      +		.expect_p2 = 4
      +	}), "well-formed range");
     - 
     --	text = "@@ -4 +8 @@";
     --	expected_result = 7;
     --	TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 1),
     --		 "non-comma range");
     ++
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -4 +8 @@",
      +		.offset = 4,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 4,
      +		.expect_p2 = 1
      +	}), "non-comma range");
     - 
     --	/* Failure */
     --	text = "@@ -X,4 +";
     --	expected_result = FAILURE;
     --	TEST(setup_static(text, strlen(text), 4, " +", expected_result, 9999, 9999),
     --		 "non-digit range (first coordinate)");
     ++
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -X,4 +",
      +		.offset = 4,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 9999,
      +		.expect_p2 = 9999
      +	}), "non-digit range (first coordinate)");
     - 
     --	text = "@@ -4,X +";
     --	expected_result = FAILURE;
     --	TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 1), // p2 is 1, a little strange but not catastrophic
     --		 "non-digit range (second coordinate)");
     ++
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -4,X +",
      +		.offset = 4,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 4,
      +		.expect_p2 = 1 // A little strange this is 1, but not end of the world
      +	}), "non-digit range (second coordinate)");
     - 
     --	text = "@@ -4,4 -";
     --	expected_result = FAILURE;
     --	TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
     --		 "non-expected trailing text");
     ++
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -4,4 -",
      +		.offset = 4,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 4,
      +		.expect_p2 = 4
      +	}), "non-expected trailing text");
     - 
     --	text = "@@ -4,4";
     --	expected_result = FAILURE;
     --	TEST(setup_static(text, strlen(text), 4, " +", expected_result, 4, 4),
     --		 "not long enough for expected trailing text");
     ++
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -4,4",
      +		.offset = 4,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 4,
      +		.expect_p2 = 4
      +	}), "not long enough for expected trailing text");
     - 
     --	text = "@@ -4,4";
     --	expected_result = FAILURE;
     --	TEST(setup_static(text, strlen(text), 7, " +", expected_result, 9999, 9999),
     --		 "not long enough for offset");
     ++
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -4,4",
      +		.offset = 7,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 9999,
      +		.expect_p2 = 9999
      +	}), "not long enough for offset");
     - 
     --	text = "@@ -4,4";
     --	expected_result = FAILURE;
     --	TEST(setup_static(text, strlen(text), -1, " +", expected_result, 9999, 9999),
     --		 "negative offset");
     ++
      +	TEST(setup_static((struct test_case) {
      +		.line = "@@ -4,4",
      +		.offset = -1,
     @@ t/unit-tests/t-apply.c
      +		.expect_p1 = 9999,
      +		.expect_p2 = 9999
      +	}), "negative offset");
     - 
     - 	return test_done();
     - }
     ++
     ++	return test_done();
     ++}


 Makefile               |   1 +
 apply.c                |   6 ++-
 apply.h                |   8 ++++
 t/unit-tests/t-apply.c | 101 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+), 2 deletions(-)
 create mode 100644 t/unit-tests/t-apply.c

diff --git a/Makefile b/Makefile
index 8f4432ae57c..1615de69e6c 100644
--- a/Makefile
+++ b/Makefile
@@ -1334,6 +1334,7 @@ THIRD_PARTY_SOURCES += compat/regex/%
 THIRD_PARTY_SOURCES += sha1collisiondetection/%
 THIRD_PARTY_SOURCES += sha1dc/%
 
+UNIT_TEST_PROGRAMS += t-apply
 UNIT_TEST_PROGRAMS += t-ctype
 UNIT_TEST_PROGRAMS += t-mem-pool
 UNIT_TEST_PROGRAMS += t-prio-queue
diff --git a/apply.c b/apply.c
index 901b67e6255..8cdf7e7d77f 100644
--- a/apply.c
+++ b/apply.c
@@ -36,6 +36,8 @@
 #include "wildmatch.h"
 #include "ws.h"
 
+#define parse_range apply_parse_fragment_range
+
 struct gitdiff_data {
 	struct strbuf *root;
 	int linenr;
@@ -1438,8 +1440,8 @@ static int parse_num(const char *line, unsigned long *p)
 	return ptr - line;
 }
 
-static int parse_range(const char *line, int len, int offset, const char *expect,
-		       unsigned long *p1, unsigned long *p2)
+int apply_parse_fragment_range(const char *line, int len, int offset, const char *expect,
+			 unsigned long *p1, unsigned long *p2)
 {
 	int digits, ex;
 
diff --git a/apply.h b/apply.h
index 7cd38b1443c..283aba77495 100644
--- a/apply.h
+++ b/apply.h
@@ -186,4 +186,12 @@ int apply_all_patches(struct apply_state *state,
 		      int argc, const char **argv,
 		      int options);
 
+/*
+ * exposed only for tests; do not call this as it not
+ * a part of the API
+ */
+extern int apply_parse_fragment_range(const char *line, int len, int offset,
+				      const char *expect, unsigned long *p1,
+				      unsigned long *p2);
+
 #endif
diff --git a/t/unit-tests/t-apply.c b/t/unit-tests/t-apply.c
new file mode 100644
index 00000000000..0eb0c22fc0d
--- /dev/null
+++ b/t/unit-tests/t-apply.c
@@ -0,0 +1,101 @@
+#include "test-lib.h"
+#include "apply.h"
+
+#define FAILURE -1
+
+typedef struct test_case {
+	const char *line;
+	const char *expect_suffix;
+	int offset;
+	unsigned long expect_p1;
+	unsigned long expect_p2;
+	int expect_result;
+} test_case;
+
+static void setup_static(struct test_case t)
+{
+	unsigned long p1 = 9999;
+	unsigned long p2 = 9999;
+	int result = apply_parse_fragment_range(t.line, strlen(t.line), t.offset,
+						t.expect_suffix, &p1, &p2);
+	check_int(result, ==, t.expect_result);
+	check_int(p1, ==, t.expect_p1);
+	check_int(p2, ==, t.expect_p2);
+}
+
+int cmd_main(int argc, const char **argv)
+{
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -4,4 +",
+		.offset = 4,
+		.expect_suffix = " +",
+		.expect_result = 9,
+		.expect_p1 = 4,
+		.expect_p2 = 4
+	}), "well-formed range");
+
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -4 +8 @@",
+		.offset = 4,
+		.expect_suffix = " +",
+		.expect_result = 7,
+		.expect_p1 = 4,
+		.expect_p2 = 1
+	}), "non-comma range");
+
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -X,4 +",
+		.offset = 4,
+		.expect_suffix = " +",
+		.expect_result = FAILURE,
+		.expect_p1 = 9999,
+		.expect_p2 = 9999
+	}), "non-digit range (first coordinate)");
+
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -4,X +",
+		.offset = 4,
+		.expect_suffix = " +",
+		.expect_result = FAILURE,
+		.expect_p1 = 4,
+		.expect_p2 = 1 // A little strange this is 1, but not end of the world
+	}), "non-digit range (second coordinate)");
+
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -4,4 -",
+		.offset = 4,
+		.expect_suffix = " +",
+		.expect_result = FAILURE,
+		.expect_p1 = 4,
+		.expect_p2 = 4
+	}), "non-expected trailing text");
+
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -4,4",
+		.offset = 4,
+		.expect_suffix = " +",
+		.expect_result = FAILURE,
+		.expect_p1 = 4,
+		.expect_p2 = 4
+	}), "not long enough for expected trailing text");
+
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -4,4",
+		.offset = 7,
+		.expect_suffix = " +",
+		.expect_result = FAILURE,
+		.expect_p1 = 9999,
+		.expect_p2 = 9999
+	}), "not long enough for offset");
+
+	TEST(setup_static((struct test_case) {
+		.line = "@@ -4,4",
+		.offset = -1,
+		.expect_suffix = " +",
+		.expect_result = FAILURE,
+		.expect_p1 = 9999,
+		.expect_p2 = 9999
+	}), "negative offset");
+
+	return test_done();
+}

base-commit: b9cfe4845cb2562584837bc0101c0ab76490a239
-- 
gitgitgadget