git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing
       [not found] <--in-reply-to=20240528113856.8348-1-chandrapratap3519@gmail.com>
@ 2024-05-29  6:55 ` Chandra Pratap
  2024-05-29  6:55   ` [GSoC][PATCH v2 1/4] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
                     ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29  6:55 UTC (permalink / raw)
  To: git; +Cc: Patrick Steinhardt, Christian Couder, Chandra Pratap

In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
testing framework written entirely in C was introduced to the Git project
aimed at simplifying testing and reducing test run times.
Currently, tests for the reftable refs-backend are performed by a custom
testing framework defined by reftable/test_framework.{c, h}. Port
reftable/basics_test.c to the unit testing framework and improve upon
the ported test.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

---
Changes in v2:
- Split up the second patch of the previous series into sub-patches

CI for v2: https://github.com/gitgitgadget/git/pull/1736

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v2 1/4] t: move reftable/basics_test.c to the unit testing framework
  2024-05-29  6:55 ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Chandra Pratap
@ 2024-05-29  6:55   ` Chandra Pratap
  2024-05-29  6:55   ` [GSoC][PATCH v2 2/4] t: move tests from reftable/stack_test.c to the new unit test Chandra Pratap
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29  6:55 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

reftable/basics_test.c exercise the functions defined in
reftable/basics.{c, h}. Migrate reftable/basics_test.c to the
unit testing framework. Migration involves refactoring the tests
to use the unit testing framework instead of reftable's test
framework.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 Makefile                                      |  2 +-
 t/helper/test-reftable.c                      |  1 -
 .../unit-tests/t-reftable-basics.c            | 41 +++++++++----------
 3 files changed, 20 insertions(+), 24 deletions(-)
 rename reftable/basics_test.c => t/unit-tests/t-reftable-basics.c (65%)

diff --git a/Makefile b/Makefile
index 8f4432ae57..36188ca256 100644
--- a/Makefile
+++ b/Makefile
@@ -1337,6 +1337,7 @@ THIRD_PARTY_SOURCES += sha1dc/%
 UNIT_TEST_PROGRAMS += t-ctype
 UNIT_TEST_PROGRAMS += t-mem-pool
 UNIT_TEST_PROGRAMS += t-prio-queue
+UNIT_TEST_PROGRAMS += t-reftable-basics
 UNIT_TEST_PROGRAMS += t-strbuf
 UNIT_TEST_PROGRAMS += t-trailer
 UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
@@ -2671,7 +2672,6 @@ REFTABLE_OBJS += reftable/stack.o
 REFTABLE_OBJS += reftable/tree.o
 REFTABLE_OBJS += reftable/writer.o
 
-REFTABLE_TEST_OBJS += reftable/basics_test.o
 REFTABLE_TEST_OBJS += reftable/block_test.o
 REFTABLE_TEST_OBJS += reftable/dump.o
 REFTABLE_TEST_OBJS += reftable/merged_test.o
diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
index bae731669c..9160bc5da6 100644
--- a/t/helper/test-reftable.c
+++ b/t/helper/test-reftable.c
@@ -5,7 +5,6 @@
 int cmd__reftable(int argc, const char **argv)
 {
 	/* test from simple to complex. */
-	basics_test_main(argc, argv);
 	record_test_main(argc, argv);
 	block_test_main(argc, argv);
 	tree_test_main(argc, argv);
diff --git a/reftable/basics_test.c b/t/unit-tests/t-reftable-basics.c
similarity index 65%
rename from reftable/basics_test.c
rename to t/unit-tests/t-reftable-basics.c
index 997c4d9e01..99e6c89120 100644
--- a/reftable/basics_test.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -6,11 +6,8 @@ license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */
 
-#include "system.h"
-
-#include "basics.h"
-#include "test_framework.h"
-#include "reftable-tests.h"
+#include "test-lib.h"
+#include "reftable/basics.h"
 
 struct integer_needle_lesseq_args {
 	int needle;
@@ -42,9 +39,8 @@ static void test_binsearch(void)
 		{11, 5},
 		{9000, 5},
 	};
-	size_t i = 0;
 
-	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
+	for (size_t i = 0; i < ARRAY_SIZE(testcases); i++) {
 		struct integer_needle_lesseq_args args = {
 			.haystack = haystack,
 			.needle = testcases[i].needle,
@@ -52,14 +48,14 @@ static void test_binsearch(void)
 		size_t idx;
 
 		idx = binsearch(ARRAY_SIZE(haystack), &integer_needle_lesseq, &args);
-		EXPECT(idx == testcases[i].expected_idx);
+		check_int(idx, ==, testcases[i].expected_idx);
 	}
 }
 
 static void test_names_length(void)
 {
 	char *a[] = { "a", "b", NULL };
-	EXPECT(names_length(a) == 2);
+	check_int(names_length(a), ==, 2);
 }
 
 static void test_parse_names_normal(void)
@@ -67,9 +63,9 @@ static void test_parse_names_normal(void)
 	char in[] = "a\nb\n";
 	char **out = NULL;
 	parse_names(in, strlen(in), &out);
-	EXPECT(!strcmp(out[0], "a"));
-	EXPECT(!strcmp(out[1], "b"));
-	EXPECT(!out[2]);
+	check_str(out[0], "a");
+	check_str(out[1], "b");
+	check(!out[2]);
 	free_names(out);
 }
 
@@ -78,8 +74,8 @@ static void test_parse_names_drop_empty(void)
 	char in[] = "a\n\n";
 	char **out = NULL;
 	parse_names(in, strlen(in), &out);
-	EXPECT(!strcmp(out[0], "a"));
-	EXPECT(!out[1]);
+	check_str(out[0], "a");
+	check(!out[1]);
 	free_names(out);
 }
 
@@ -89,17 +85,18 @@ static void test_common_prefix(void)
 	struct strbuf s2 = STRBUF_INIT;
 	strbuf_addstr(&s1, "abcdef");
 	strbuf_addstr(&s2, "abc");
-	EXPECT(common_prefix_size(&s1, &s2) == 3);
+	check_int(common_prefix_size(&s1, &s2), ==, 3);
 	strbuf_release(&s1);
 	strbuf_release(&s2);
 }
 
-int basics_test_main(int argc, const char *argv[])
+int cmd_main(int argc, const char *argv[])
 {
-	RUN_TEST(test_common_prefix);
-	RUN_TEST(test_parse_names_normal);
-	RUN_TEST(test_parse_names_drop_empty);
-	RUN_TEST(test_binsearch);
-	RUN_TEST(test_names_length);
-	return 0;
+	TEST(test_common_prefix(), "common_prefix_size works");
+	TEST(test_parse_names_normal(), "parse_names works for basic input");
+	TEST(test_parse_names_drop_empty(), "parse_names drops empty string");
+	TEST(test_binsearch(), "binary search with binsearch works");
+	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
+
+	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v2 2/4] t: move tests from reftable/stack_test.c to the new unit test
  2024-05-29  6:55 ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Chandra Pratap
  2024-05-29  6:55   ` [GSoC][PATCH v2 1/4] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
@ 2024-05-29  6:55   ` Chandra Pratap
  2024-05-29  6:55   ` [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c " Chandra Pratap
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29  6:55 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

parse_names() and names_equal() are functions defined in
reftable/basics.{c, h}. Move the tests for these functions from
reftable/stack_test.c to the newly ported test.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 reftable/stack_test.c            | 25 -------------------------
 t/unit-tests/t-reftable-basics.c | 25 ++++++++++++++++++++++---
 2 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 7889f818d1..6f6af11e53 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -102,29 +102,6 @@ static void test_read_file(void)
 	(void) remove(fn);
 }
 
-static void test_parse_names(void)
-{
-	char buf[] = "line\n";
-	char **names = NULL;
-	parse_names(buf, strlen(buf), &names);
-
-	EXPECT(NULL != names[0]);
-	EXPECT(0 == strcmp(names[0], "line"));
-	EXPECT(NULL == names[1]);
-	free_names(names);
-}
-
-static void test_names_equal(void)
-{
-	char *a[] = { "a", "b", "c", NULL };
-	char *b[] = { "a", "b", "d", NULL };
-	char *c[] = { "a", "b", NULL };
-
-	EXPECT(names_equal(a, a));
-	EXPECT(!names_equal(a, b));
-	EXPECT(!names_equal(a, c));
-}
-
 static int write_test_ref(struct reftable_writer *wr, void *arg)
 {
 	struct reftable_ref_record *ref = arg;
@@ -1048,8 +1025,6 @@ static void test_reftable_stack_compaction_concurrent_clean(void)
 int stack_test_main(int argc, const char *argv[])
 {
 	RUN_TEST(test_empty_add);
-	RUN_TEST(test_names_equal);
-	RUN_TEST(test_parse_names);
 	RUN_TEST(test_read_file);
 	RUN_TEST(test_reflog_expire);
 	RUN_TEST(test_reftable_stack_add);
diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index 99e6c89120..55fcff12d9 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -58,14 +58,32 @@ static void test_names_length(void)
 	check_int(names_length(a), ==, 2);
 }
 
+static void test_names_equal(void)
+{
+	char *a[] = { "a", "b", "c", NULL };
+	char *b[] = { "a", "b", "d", NULL };
+	char *c[] = { "a", "b", NULL };
+
+	check(names_equal(a, a));
+	check(!names_equal(a, b));
+	check(!names_equal(a, c));
+}
+
 static void test_parse_names_normal(void)
 {
-	char in[] = "a\nb\n";
+	char in1[] = "line\n";
+	char in2[] = "a\nb\nc";
 	char **out = NULL;
-	parse_names(in, strlen(in), &out);
+	parse_names(in1, strlen(in1), &out);
+	check_str(out[0], "line");
+	check(!out[1]);
+	free_names(out);
+
+	parse_names(in2, strlen(in2), &out);
 	check_str(out[0], "a");
 	check_str(out[1], "b");
-	check(!out[2]);
+	check_str(out[2], "c");
+	check(!out[3]);
 	free_names(out);
 }
 
@@ -97,6 +115,7 @@ int cmd_main(int argc, const char *argv[])
 	TEST(test_parse_names_drop_empty(), "parse_names drops empty string");
 	TEST(test_binsearch(), "binary search with binsearch works");
 	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
+	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
 
 	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c to the new unit test
  2024-05-29  6:55 ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Chandra Pratap
  2024-05-29  6:55   ` [GSoC][PATCH v2 1/4] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
  2024-05-29  6:55   ` [GSoC][PATCH v2 2/4] t: move tests from reftable/stack_test.c to the new unit test Chandra Pratap
@ 2024-05-29  6:55   ` Chandra Pratap
  2024-05-29  9:30     ` Patrick Steinhardt
  2024-05-29  6:55   ` [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names() Chandra Pratap
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29  6:55 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

common_prefix_size(), get_be24() and put_be24() are functions defined
in reftable/basics.{c, h}. Move the tests for these functions from
reftable/record_test.c to the newly ported test.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 reftable/record_test.c           | 37 -----------------------------
 t/unit-tests/t-reftable-basics.c | 40 ++++++++++++++++++++++++++------
 2 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/reftable/record_test.c b/reftable/record_test.c
index c158ee79ff..58290bdba3 100644
--- a/reftable/record_test.c
+++ b/reftable/record_test.c
@@ -64,31 +64,6 @@ static void test_varint_roundtrip(void)
 	}
 }
 
-static void test_common_prefix(void)
-{
-	struct {
-		const char *a, *b;
-		int want;
-	} cases[] = {
-		{ "abc", "ab", 2 },
-		{ "", "abc", 0 },
-		{ "abc", "abd", 2 },
-		{ "abc", "pqr", 0 },
-	};
-
-	int i = 0;
-	for (i = 0; i < ARRAY_SIZE(cases); i++) {
-		struct strbuf a = STRBUF_INIT;
-		struct strbuf b = STRBUF_INIT;
-		strbuf_addstr(&a, cases[i].a);
-		strbuf_addstr(&b, cases[i].b);
-		EXPECT(common_prefix_size(&a, &b) == cases[i].want);
-
-		strbuf_release(&a);
-		strbuf_release(&b);
-	}
-}
-
 static void set_hash(uint8_t *h, int j)
 {
 	int i = 0;
@@ -258,16 +233,6 @@ static void test_reftable_log_record_roundtrip(void)
 	strbuf_release(&scratch);
 }
 
-static void test_u24_roundtrip(void)
-{
-	uint32_t in = 0x112233;
-	uint8_t dest[3];
-	uint32_t out;
-	put_be24(dest, in);
-	out = get_be24(dest);
-	EXPECT(in == out);
-}
-
 static void test_key_roundtrip(void)
 {
 	uint8_t buffer[1024] = { 0 };
@@ -411,9 +376,7 @@ int record_test_main(int argc, const char *argv[])
 	RUN_TEST(test_reftable_ref_record_roundtrip);
 	RUN_TEST(test_varint_roundtrip);
 	RUN_TEST(test_key_roundtrip);
-	RUN_TEST(test_common_prefix);
 	RUN_TEST(test_reftable_obj_record_roundtrip);
 	RUN_TEST(test_reftable_index_record_roundtrip);
-	RUN_TEST(test_u24_roundtrip);
 	return 0;
 }
diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index 55fcff12d9..b02ca02040 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -99,13 +99,38 @@ static void test_parse_names_drop_empty(void)
 
 static void test_common_prefix(void)
 {
-	struct strbuf s1 = STRBUF_INIT;
-	struct strbuf s2 = STRBUF_INIT;
-	strbuf_addstr(&s1, "abcdef");
-	strbuf_addstr(&s2, "abc");
-	check_int(common_prefix_size(&s1, &s2), ==, 3);
-	strbuf_release(&s1);
-	strbuf_release(&s2);
+	struct strbuf a = STRBUF_INIT;
+	struct strbuf b = STRBUF_INIT;
+	struct {
+		const char *a, *b;
+		int want;
+	} cases[] = {
+		{"abcdef", "abc", 3},
+		{ "abc", "ab", 2 },
+		{ "", "abc", 0 },
+		{ "abc", "abd", 2 },
+		{ "abc", "pqr", 0 },
+	};
+
+	for (size_t i = 0; i < ARRAY_SIZE(cases); i++) {
+		strbuf_addstr(&a, cases[i].a);
+		strbuf_addstr(&b, cases[i].b);
+		check_int(common_prefix_size(&a, &b), ==, cases[i].want);
+		strbuf_reset(&a);
+		strbuf_reset(&b);
+	}
+	strbuf_release(&a);
+	strbuf_release(&b);
+}
+
+static void test_u24_roundtrip(void)
+{
+	uint32_t in = 0x112233;
+	uint8_t dest[3];
+	uint32_t out;
+	put_be24(dest, in);
+	out = get_be24(dest);
+	check_int(in, ==, out);
 }
 
 int cmd_main(int argc, const char *argv[])
@@ -116,6 +141,7 @@ int cmd_main(int argc, const char *argv[])
 	TEST(test_binsearch(), "binary search with binsearch works");
 	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
 	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
+	TEST(test_u24_roundtrip(), "put_be24 and get_be24 work");
 
 	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names()
  2024-05-29  6:55 ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Chandra Pratap
                     ` (2 preceding siblings ...)
  2024-05-29  6:55   ` [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c " Chandra Pratap
@ 2024-05-29  6:55   ` Chandra Pratap
  2024-05-29  9:30     ` Patrick Steinhardt
  2024-05-29  9:29   ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Patrick Steinhardt
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
  5 siblings, 1 reply; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29  6:55 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

put_be16() is a function defined in reftable/basics.{c, h} for which
there are no tests in the current setup. Add a test for the same and
improve the existing test-case for parse_names().

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 t/unit-tests/t-reftable-basics.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index b02ca02040..8372faec8c 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -89,11 +89,13 @@ static void test_parse_names_normal(void)
 
 static void test_parse_names_drop_empty(void)
 {
-	char in[] = "a\n\n";
+	char in[] = "a\n\nb\n";
 	char **out = NULL;
 	parse_names(in, strlen(in), &out);
 	check_str(out[0], "a");
-	check(!out[1]);
+	/* simply '\n' should be dropped as empty string */
+	check_str(out[1], "b");
+	check(!out[2]);
 	free_names(out);
 }
 
@@ -123,14 +125,20 @@ static void test_common_prefix(void)
 	strbuf_release(&b);
 }
 
-static void test_u24_roundtrip(void)
+static void test_be_roundtrip(void)
 {
 	uint32_t in = 0x112233;
 	uint8_t dest[3];
 	uint32_t out;
+	/* test put_be24 and get_be24 roundtrip */
 	put_be24(dest, in);
 	out = get_be24(dest);
 	check_int(in, ==, out);
+	/* test put_be16 and get_be16 roundtrip */
+	in = 0xfef1;
+	put_be16(dest, in);
+	out = get_be16(dest);
+	check_int(in, ==, out);
 }
 
 int cmd_main(int argc, const char *argv[])
@@ -141,7 +149,7 @@ int cmd_main(int argc, const char *argv[])
 	TEST(test_binsearch(), "binary search with binsearch works");
 	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
 	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
-	TEST(test_u24_roundtrip(), "put_be24 and get_be24 work");
+	TEST(test_be_roundtrip(), "put_be24, get_be24 and put_be16 work");
 
 	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing
  2024-05-29  6:55 ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Chandra Pratap
                     ` (3 preceding siblings ...)
  2024-05-29  6:55   ` [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names() Chandra Pratap
@ 2024-05-29  9:29   ` Patrick Steinhardt
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
  5 siblings, 0 replies; 18+ messages in thread
From: Patrick Steinhardt @ 2024-05-29  9:29 UTC (permalink / raw)
  To: Chandra Pratap; +Cc: git, Christian Couder

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

On Wed, May 29, 2024 at 12:25:08PM +0530, Chandra Pratap wrote:
> In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
> testing framework written entirely in C was introduced to the Git project
> aimed at simplifying testing and reducing test run times.
> Currently, tests for the reftable refs-backend are performed by a custom
> testing framework defined by reftable/test_framework.{c, h}. Port
> reftable/basics_test.c to the unit testing framework and improve upon
> the ported test.
> 
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

The evolution of a patch series can be followed a bit easier if
subsequent versions are attached to the initial thread. You can do this
by passing e.g. `--in-reply-to=<message-id>` to git-format-patch(1),
where the message ID is the one of the cover letter of the first
version.

I'd also recommend to attach a range diff to your cover letter via the
`--range-diff=` parameter. This range diff helps the reviewer to spot
what has changed between your preceding version and this one.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c to the new unit test
  2024-05-29  6:55   ` [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c " Chandra Pratap
@ 2024-05-29  9:30     ` Patrick Steinhardt
  2024-05-29 22:38       ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Patrick Steinhardt @ 2024-05-29  9:30 UTC (permalink / raw)
  To: Chandra Pratap; +Cc: git, Christian Couder

[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]

On Wed, May 29, 2024 at 12:25:11PM +0530, Chandra Pratap wrote:
[snip]
> diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
> index 55fcff12d9..b02ca02040 100644
> --- a/t/unit-tests/t-reftable-basics.c
> +++ b/t/unit-tests/t-reftable-basics.c
> @@ -99,13 +99,38 @@ static void test_parse_names_drop_empty(void)
>  
>  static void test_common_prefix(void)
>  {
> -	struct strbuf s1 = STRBUF_INIT;
> -	struct strbuf s2 = STRBUF_INIT;
> -	strbuf_addstr(&s1, "abcdef");
> -	strbuf_addstr(&s2, "abc");
> -	check_int(common_prefix_size(&s1, &s2), ==, 3);
> -	strbuf_release(&s1);
> -	strbuf_release(&s2);
> +	struct strbuf a = STRBUF_INIT;
> +	struct strbuf b = STRBUF_INIT;
> +	struct {
> +		const char *a, *b;
> +		int want;
> +	} cases[] = {
> +		{"abcdef", "abc", 3},
> +		{ "abc", "ab", 2 },
> +		{ "", "abc", 0 },
> +		{ "abc", "abd", 2 },
> +		{ "abc", "pqr", 0 },
> +	};
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(cases); i++) {
> +		strbuf_addstr(&a, cases[i].a);
> +		strbuf_addstr(&b, cases[i].b);
> +		check_int(common_prefix_size(&a, &b), ==, cases[i].want);
> +		strbuf_reset(&a);
> +		strbuf_reset(&b);
> +	}
> +	strbuf_release(&a);
> +	strbuf_release(&b);
> +}

Oh, so this test was even duplicated. It may make sense to point out
details like this in the commit message to prepare the reader. But
that's probably not worth a reroll.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names()
  2024-05-29  6:55   ` [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names() Chandra Pratap
@ 2024-05-29  9:30     ` Patrick Steinhardt
  0 siblings, 0 replies; 18+ messages in thread
From: Patrick Steinhardt @ 2024-05-29  9:30 UTC (permalink / raw)
  To: Chandra Pratap; +Cc: git, Christian Couder

[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]

On Wed, May 29, 2024 at 12:25:12PM +0530, Chandra Pratap wrote:
> put_be16() is a function defined in reftable/basics.{c, h} for which
> there are no tests in the current setup. Add a test for the same and
> improve the existing test-case for parse_names().
> 
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> ---
>  t/unit-tests/t-reftable-basics.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
> index b02ca02040..8372faec8c 100644
> --- a/t/unit-tests/t-reftable-basics.c
> +++ b/t/unit-tests/t-reftable-basics.c
> @@ -89,11 +89,13 @@ static void test_parse_names_normal(void)
>  
>  static void test_parse_names_drop_empty(void)
>  {
> -	char in[] = "a\n\n";
> +	char in[] = "a\n\nb\n";
>  	char **out = NULL;
>  	parse_names(in, strlen(in), &out);
>  	check_str(out[0], "a");
> -	check(!out[1]);
> +	/* simply '\n' should be dropped as empty string */
> +	check_str(out[1], "b");
> +	check(!out[2]);
>  	free_names(out);
>  }

I'd split out this change into yet another commit. Also, you say that
the test case is being "improved", but without mentioning what the
improvement actually is.

> @@ -123,14 +125,20 @@ static void test_common_prefix(void)
>  	strbuf_release(&b);
>  }
>  
> -static void test_u24_roundtrip(void)
> +static void test_be_roundtrip(void)
>  {
>  	uint32_t in = 0x112233;
>  	uint8_t dest[3];
>  	uint32_t out;
> +	/* test put_be24 and get_be24 roundtrip */
>  	put_be24(dest, in);
>  	out = get_be24(dest);
>  	check_int(in, ==, out);
> +	/* test put_be16 and get_be16 roundtrip */
> +	in = 0xfef1;
> +	put_be16(dest, in);
> +	out = get_be16(dest);
> +	check_int(in, ==, out);
>  }

Would it make sense to have separate tests for each of the variants
instead of one test for all of these? Might make things a bit easier to
follow.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing
  2024-05-29  6:55 ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Chandra Pratap
                     ` (4 preceding siblings ...)
  2024-05-29  9:29   ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Patrick Steinhardt
@ 2024-05-29 16:59   ` Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 1/5] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
                       ` (5 more replies)
  5 siblings, 6 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29 16:59 UTC (permalink / raw)
  To: git; +Cc: Patrick Steinhardt, Christian Couder, Chandra Pratap

In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
testing framework written entirely in C was introduced to the Git project
aimed at simplifying testing and reducing test run times.
Currently, tests for the reftable refs-backend are performed by a custom
testing framework defined by reftable/test_framework.{c, h}. Port
reftable/basics_test.c to the unit testing framework and improve upon
the ported test.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

---
Changes in v3:
- Split up the 4th patch of the previous series into 2 sub-patches

CI/PR for v3: https://github.com/gitgitgadget/git/pull/1736

range-diff against v2:

1:  3ab1b415f4 = 1:  3ab1b415f4 t: move reftable/basics_test.c to the unit testing framework
2:  51fec8a376 = 2:  0143bbd2e4 t: move tests from reftable/stack_test.c to the new unit test
3:  e0e9adfdf6 = 3:  2f1d02e945 t: move tests from reftable/record_test.c to the new unit test
-:  ---------- > 4:  14606ac8db t: add test for put_be16()
4:  81b8975b4c ! 5:  2e741bab6d t: add test for put_be16() and improve test-case for parse_names()
    @@ Metadata
     Author: Chandra Pratap <chandrapratap3519@gmail.com>

      ## Commit message ##
    -    t: add test for put_be16() and improve test-case for parse_names()
    +    t: improve the test-case for parse_names()

    -    put_be16() is a function defined in reftable/basics.{c, h} for which
    -    there are no tests in the current setup. Add a test for the same and
    -    improve the existing test-case for parse_names().
    +    In the existing test-case for parse_names(), the fact that empty
    +    lines should be ignored is not obvious because the empty line is
    +    immediately followed by end-of-string. This can be mistaken as the
    +    empty line getting replaced by NULL. Improve this by adding a
    +    non-empty line after the empty one to demonstrate the intended behavior.

         Mentored-by: Patrick Steinhardt <ps@pks.im>
         Mentored-by: Christian Couder <chriscool@tuxfamily.org>
    @@ t/unit-tests/t-reftable-basics.c: static void test_parse_names_normal(void)
      	free_names(out);
      }

    -@@ t/unit-tests/t-reftable-basics.c: static void test_common_prefix(void)
    - 	strbuf_release(&b);
    - }
    -
    --static void test_u24_roundtrip(void)
    -+static void test_be_roundtrip(void)
    - {
    - 	uint32_t in = 0x112233;
    - 	uint8_t dest[3];
    - 	uint32_t out;
    -+	/* test put_be24 and get_be24 roundtrip */
    - 	put_be24(dest, in);
    - 	out = get_be24(dest);
    - 	check_int(in, ==, out);
    -+	/* test put_be16 and get_be16 roundtrip */
    -+	in = 0xfef1;
    -+	put_be16(dest, in);
    -+	out = get_be16(dest);
    -+	check_int(in, ==, out);
    - }
    -
    - int cmd_main(int argc, const char *argv[])
    -@@ t/unit-tests/t-reftable-basics.c: int cmd_main(int argc, const char *argv[])
    - 	TEST(test_binsearch(), "binary search with binsearch works");
    - 	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
    - 	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
    --	TEST(test_u24_roundtrip(), "put_be24 and get_be24 work");
    -+	TEST(test_be_roundtrip(), "put_be24, get_be24 and put_be16 work");
    -
    - 	return test_done();
    - }


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v3 1/5] t: move reftable/basics_test.c to the unit testing framework
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
@ 2024-05-29 16:59     ` Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 2/5] t: move tests from reftable/stack_test.c to the new unit test Chandra Pratap
                       ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29 16:59 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

reftable/basics_test.c exercise the functions defined in
reftable/basics.{c, h}. Migrate reftable/basics_test.c to the
unit testing framework. Migration involves refactoring the tests
to use the unit testing framework instead of reftable's test
framework.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 Makefile                                      |  2 +-
 t/helper/test-reftable.c                      |  1 -
 .../unit-tests/t-reftable-basics.c            | 41 +++++++++----------
 3 files changed, 20 insertions(+), 24 deletions(-)
 rename reftable/basics_test.c => t/unit-tests/t-reftable-basics.c (65%)

diff --git a/Makefile b/Makefile
index 8f4432ae57..36188ca256 100644
--- a/Makefile
+++ b/Makefile
@@ -1337,6 +1337,7 @@ THIRD_PARTY_SOURCES += sha1dc/%
 UNIT_TEST_PROGRAMS += t-ctype
 UNIT_TEST_PROGRAMS += t-mem-pool
 UNIT_TEST_PROGRAMS += t-prio-queue
+UNIT_TEST_PROGRAMS += t-reftable-basics
 UNIT_TEST_PROGRAMS += t-strbuf
 UNIT_TEST_PROGRAMS += t-trailer
 UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
@@ -2671,7 +2672,6 @@ REFTABLE_OBJS += reftable/stack.o
 REFTABLE_OBJS += reftable/tree.o
 REFTABLE_OBJS += reftable/writer.o
 
-REFTABLE_TEST_OBJS += reftable/basics_test.o
 REFTABLE_TEST_OBJS += reftable/block_test.o
 REFTABLE_TEST_OBJS += reftable/dump.o
 REFTABLE_TEST_OBJS += reftable/merged_test.o
diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
index bae731669c..9160bc5da6 100644
--- a/t/helper/test-reftable.c
+++ b/t/helper/test-reftable.c
@@ -5,7 +5,6 @@
 int cmd__reftable(int argc, const char **argv)
 {
 	/* test from simple to complex. */
-	basics_test_main(argc, argv);
 	record_test_main(argc, argv);
 	block_test_main(argc, argv);
 	tree_test_main(argc, argv);
diff --git a/reftable/basics_test.c b/t/unit-tests/t-reftable-basics.c
similarity index 65%
rename from reftable/basics_test.c
rename to t/unit-tests/t-reftable-basics.c
index 997c4d9e01..99e6c89120 100644
--- a/reftable/basics_test.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -6,11 +6,8 @@ license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */
 
-#include "system.h"
-
-#include "basics.h"
-#include "test_framework.h"
-#include "reftable-tests.h"
+#include "test-lib.h"
+#include "reftable/basics.h"
 
 struct integer_needle_lesseq_args {
 	int needle;
@@ -42,9 +39,8 @@ static void test_binsearch(void)
 		{11, 5},
 		{9000, 5},
 	};
-	size_t i = 0;
 
-	for (i = 0; i < ARRAY_SIZE(testcases); i++) {
+	for (size_t i = 0; i < ARRAY_SIZE(testcases); i++) {
 		struct integer_needle_lesseq_args args = {
 			.haystack = haystack,
 			.needle = testcases[i].needle,
@@ -52,14 +48,14 @@ static void test_binsearch(void)
 		size_t idx;
 
 		idx = binsearch(ARRAY_SIZE(haystack), &integer_needle_lesseq, &args);
-		EXPECT(idx == testcases[i].expected_idx);
+		check_int(idx, ==, testcases[i].expected_idx);
 	}
 }
 
 static void test_names_length(void)
 {
 	char *a[] = { "a", "b", NULL };
-	EXPECT(names_length(a) == 2);
+	check_int(names_length(a), ==, 2);
 }
 
 static void test_parse_names_normal(void)
@@ -67,9 +63,9 @@ static void test_parse_names_normal(void)
 	char in[] = "a\nb\n";
 	char **out = NULL;
 	parse_names(in, strlen(in), &out);
-	EXPECT(!strcmp(out[0], "a"));
-	EXPECT(!strcmp(out[1], "b"));
-	EXPECT(!out[2]);
+	check_str(out[0], "a");
+	check_str(out[1], "b");
+	check(!out[2]);
 	free_names(out);
 }
 
@@ -78,8 +74,8 @@ static void test_parse_names_drop_empty(void)
 	char in[] = "a\n\n";
 	char **out = NULL;
 	parse_names(in, strlen(in), &out);
-	EXPECT(!strcmp(out[0], "a"));
-	EXPECT(!out[1]);
+	check_str(out[0], "a");
+	check(!out[1]);
 	free_names(out);
 }
 
@@ -89,17 +85,18 @@ static void test_common_prefix(void)
 	struct strbuf s2 = STRBUF_INIT;
 	strbuf_addstr(&s1, "abcdef");
 	strbuf_addstr(&s2, "abc");
-	EXPECT(common_prefix_size(&s1, &s2) == 3);
+	check_int(common_prefix_size(&s1, &s2), ==, 3);
 	strbuf_release(&s1);
 	strbuf_release(&s2);
 }
 
-int basics_test_main(int argc, const char *argv[])
+int cmd_main(int argc, const char *argv[])
 {
-	RUN_TEST(test_common_prefix);
-	RUN_TEST(test_parse_names_normal);
-	RUN_TEST(test_parse_names_drop_empty);
-	RUN_TEST(test_binsearch);
-	RUN_TEST(test_names_length);
-	return 0;
+	TEST(test_common_prefix(), "common_prefix_size works");
+	TEST(test_parse_names_normal(), "parse_names works for basic input");
+	TEST(test_parse_names_drop_empty(), "parse_names drops empty string");
+	TEST(test_binsearch(), "binary search with binsearch works");
+	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
+
+	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v3 2/5] t: move tests from reftable/stack_test.c to the new unit test
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 1/5] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
@ 2024-05-29 16:59     ` Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 3/5] t: move tests from reftable/record_test.c " Chandra Pratap
                       ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29 16:59 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

parse_names() and names_equal() are functions defined in
reftable/basics.{c, h}. Move the tests for these functions from
reftable/stack_test.c to the newly ported test.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 reftable/stack_test.c            | 25 -------------------------
 t/unit-tests/t-reftable-basics.c | 25 ++++++++++++++++++++++---
 2 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 7889f818d1..6f6af11e53 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -102,29 +102,6 @@ static void test_read_file(void)
 	(void) remove(fn);
 }
 
-static void test_parse_names(void)
-{
-	char buf[] = "line\n";
-	char **names = NULL;
-	parse_names(buf, strlen(buf), &names);
-
-	EXPECT(NULL != names[0]);
-	EXPECT(0 == strcmp(names[0], "line"));
-	EXPECT(NULL == names[1]);
-	free_names(names);
-}
-
-static void test_names_equal(void)
-{
-	char *a[] = { "a", "b", "c", NULL };
-	char *b[] = { "a", "b", "d", NULL };
-	char *c[] = { "a", "b", NULL };
-
-	EXPECT(names_equal(a, a));
-	EXPECT(!names_equal(a, b));
-	EXPECT(!names_equal(a, c));
-}
-
 static int write_test_ref(struct reftable_writer *wr, void *arg)
 {
 	struct reftable_ref_record *ref = arg;
@@ -1048,8 +1025,6 @@ static void test_reftable_stack_compaction_concurrent_clean(void)
 int stack_test_main(int argc, const char *argv[])
 {
 	RUN_TEST(test_empty_add);
-	RUN_TEST(test_names_equal);
-	RUN_TEST(test_parse_names);
 	RUN_TEST(test_read_file);
 	RUN_TEST(test_reflog_expire);
 	RUN_TEST(test_reftable_stack_add);
diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index 99e6c89120..55fcff12d9 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -58,14 +58,32 @@ static void test_names_length(void)
 	check_int(names_length(a), ==, 2);
 }
 
+static void test_names_equal(void)
+{
+	char *a[] = { "a", "b", "c", NULL };
+	char *b[] = { "a", "b", "d", NULL };
+	char *c[] = { "a", "b", NULL };
+
+	check(names_equal(a, a));
+	check(!names_equal(a, b));
+	check(!names_equal(a, c));
+}
+
 static void test_parse_names_normal(void)
 {
-	char in[] = "a\nb\n";
+	char in1[] = "line\n";
+	char in2[] = "a\nb\nc";
 	char **out = NULL;
-	parse_names(in, strlen(in), &out);
+	parse_names(in1, strlen(in1), &out);
+	check_str(out[0], "line");
+	check(!out[1]);
+	free_names(out);
+
+	parse_names(in2, strlen(in2), &out);
 	check_str(out[0], "a");
 	check_str(out[1], "b");
-	check(!out[2]);
+	check_str(out[2], "c");
+	check(!out[3]);
 	free_names(out);
 }
 
@@ -97,6 +115,7 @@ int cmd_main(int argc, const char *argv[])
 	TEST(test_parse_names_drop_empty(), "parse_names drops empty string");
 	TEST(test_binsearch(), "binary search with binsearch works");
 	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
+	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
 
 	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v3 3/5] t: move tests from reftable/record_test.c to the new unit test
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 1/5] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 2/5] t: move tests from reftable/stack_test.c to the new unit test Chandra Pratap
@ 2024-05-29 16:59     ` Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 4/5] t: add test for put_be16() Chandra Pratap
                       ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29 16:59 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

common_prefix_size(), get_be24() and put_be24() are functions defined
in reftable/basics.{c, h}. Move the tests for these functions from
reftable/record_test.c to the newly ported test.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 reftable/record_test.c           | 37 -----------------------------
 t/unit-tests/t-reftable-basics.c | 40 ++++++++++++++++++++++++++------
 2 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/reftable/record_test.c b/reftable/record_test.c
index c158ee79ff..58290bdba3 100644
--- a/reftable/record_test.c
+++ b/reftable/record_test.c
@@ -64,31 +64,6 @@ static void test_varint_roundtrip(void)
 	}
 }
 
-static void test_common_prefix(void)
-{
-	struct {
-		const char *a, *b;
-		int want;
-	} cases[] = {
-		{ "abc", "ab", 2 },
-		{ "", "abc", 0 },
-		{ "abc", "abd", 2 },
-		{ "abc", "pqr", 0 },
-	};
-
-	int i = 0;
-	for (i = 0; i < ARRAY_SIZE(cases); i++) {
-		struct strbuf a = STRBUF_INIT;
-		struct strbuf b = STRBUF_INIT;
-		strbuf_addstr(&a, cases[i].a);
-		strbuf_addstr(&b, cases[i].b);
-		EXPECT(common_prefix_size(&a, &b) == cases[i].want);
-
-		strbuf_release(&a);
-		strbuf_release(&b);
-	}
-}
-
 static void set_hash(uint8_t *h, int j)
 {
 	int i = 0;
@@ -258,16 +233,6 @@ static void test_reftable_log_record_roundtrip(void)
 	strbuf_release(&scratch);
 }
 
-static void test_u24_roundtrip(void)
-{
-	uint32_t in = 0x112233;
-	uint8_t dest[3];
-	uint32_t out;
-	put_be24(dest, in);
-	out = get_be24(dest);
-	EXPECT(in == out);
-}
-
 static void test_key_roundtrip(void)
 {
 	uint8_t buffer[1024] = { 0 };
@@ -411,9 +376,7 @@ int record_test_main(int argc, const char *argv[])
 	RUN_TEST(test_reftable_ref_record_roundtrip);
 	RUN_TEST(test_varint_roundtrip);
 	RUN_TEST(test_key_roundtrip);
-	RUN_TEST(test_common_prefix);
 	RUN_TEST(test_reftable_obj_record_roundtrip);
 	RUN_TEST(test_reftable_index_record_roundtrip);
-	RUN_TEST(test_u24_roundtrip);
 	return 0;
 }
diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index 55fcff12d9..b02ca02040 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -99,13 +99,38 @@ static void test_parse_names_drop_empty(void)
 
 static void test_common_prefix(void)
 {
-	struct strbuf s1 = STRBUF_INIT;
-	struct strbuf s2 = STRBUF_INIT;
-	strbuf_addstr(&s1, "abcdef");
-	strbuf_addstr(&s2, "abc");
-	check_int(common_prefix_size(&s1, &s2), ==, 3);
-	strbuf_release(&s1);
-	strbuf_release(&s2);
+	struct strbuf a = STRBUF_INIT;
+	struct strbuf b = STRBUF_INIT;
+	struct {
+		const char *a, *b;
+		int want;
+	} cases[] = {
+		{"abcdef", "abc", 3},
+		{ "abc", "ab", 2 },
+		{ "", "abc", 0 },
+		{ "abc", "abd", 2 },
+		{ "abc", "pqr", 0 },
+	};
+
+	for (size_t i = 0; i < ARRAY_SIZE(cases); i++) {
+		strbuf_addstr(&a, cases[i].a);
+		strbuf_addstr(&b, cases[i].b);
+		check_int(common_prefix_size(&a, &b), ==, cases[i].want);
+		strbuf_reset(&a);
+		strbuf_reset(&b);
+	}
+	strbuf_release(&a);
+	strbuf_release(&b);
+}
+
+static void test_u24_roundtrip(void)
+{
+	uint32_t in = 0x112233;
+	uint8_t dest[3];
+	uint32_t out;
+	put_be24(dest, in);
+	out = get_be24(dest);
+	check_int(in, ==, out);
 }
 
 int cmd_main(int argc, const char *argv[])
@@ -116,6 +141,7 @@ int cmd_main(int argc, const char *argv[])
 	TEST(test_binsearch(), "binary search with binsearch works");
 	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
 	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
+	TEST(test_u24_roundtrip(), "put_be24 and get_be24 work");
 
 	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v3 4/5] t: add test for put_be16()
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
                       ` (2 preceding siblings ...)
  2024-05-29 16:59     ` [GSoC][PATCH v3 3/5] t: move tests from reftable/record_test.c " Chandra Pratap
@ 2024-05-29 16:59     ` Chandra Pratap
  2024-05-29 16:59     ` [GSoC][PATCH v3 5/5] t: improve the test-case for parse_names() Chandra Pratap
  2024-05-30  4:35     ` [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing Patrick Steinhardt
  5 siblings, 0 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29 16:59 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

put_be16() is a function defined in reftable/basics.{c, h} for which
there are no tests in the current setup. Add a test for the same.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 t/unit-tests/t-reftable-basics.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index b02ca02040..3c08218257 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -133,6 +133,16 @@ static void test_u24_roundtrip(void)
 	check_int(in, ==, out);
 }
 
+static void test_u16_roundtrip(void)
+{
+	uint32_t in = 0xfef1;
+	uint8_t dest[3];
+	uint32_t out;
+	put_be16(dest, in);
+	out = get_be16(dest);
+	check_int(in, ==, out);
+}
+
 int cmd_main(int argc, const char *argv[])
 {
 	TEST(test_common_prefix(), "common_prefix_size works");
@@ -142,6 +152,7 @@ int cmd_main(int argc, const char *argv[])
 	TEST(test_names_length(), "names_length retuns size of a NULL-terminated string array");
 	TEST(test_names_equal(), "names_equal compares NULL-terminated string arrays");
 	TEST(test_u24_roundtrip(), "put_be24 and get_be24 work");
+	TEST(test_u16_roundtrip(), "put_be16 and get_be16 work");
 
 	return test_done();
 }
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [GSoC][PATCH v3 5/5] t: improve the test-case for parse_names()
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
                       ` (3 preceding siblings ...)
  2024-05-29 16:59     ` [GSoC][PATCH v3 4/5] t: add test for put_be16() Chandra Pratap
@ 2024-05-29 16:59     ` Chandra Pratap
  2024-05-30  4:35     ` [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing Patrick Steinhardt
  5 siblings, 0 replies; 18+ messages in thread
From: Chandra Pratap @ 2024-05-29 16:59 UTC (permalink / raw)
  To: git; +Cc: Chandra Pratap, Patrick Steinhardt, Christian Couder

In the existing test-case for parse_names(), the fact that empty
lines should be ignored is not obvious because the empty line is
immediately followed by end-of-string. This can be mistaken as the
empty line getting replaced by NULL. Improve this by adding a
non-empty line after the empty one to demonstrate the intended behavior.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 t/unit-tests/t-reftable-basics.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index 3c08218257..529049af12 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -89,11 +89,13 @@ static void test_parse_names_normal(void)
 
 static void test_parse_names_drop_empty(void)
 {
-	char in[] = "a\n\n";
+	char in[] = "a\n\nb\n";
 	char **out = NULL;
 	parse_names(in, strlen(in), &out);
 	check_str(out[0], "a");
-	check(!out[1]);
+	/* simply '\n' should be dropped as empty string */
+	check_str(out[1], "b");
+	check(!out[2]);
 	free_names(out);
 }
 
-- 
2.45.GIT


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c to the new unit test
  2024-05-29  9:30     ` Patrick Steinhardt
@ 2024-05-29 22:38       ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2024-05-29 22:38 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Chandra Pratap, git, Christian Couder

Patrick Steinhardt <ps@pks.im> writes:

>> +	strbuf_release(&a);
>> +	strbuf_release(&b);
>> +}
>
> Oh, so this test was even duplicated. It may make sense to point out
> details like this in the commit message to prepare the reader. But
> that's probably not worth a reroll.

Probably.  But if you are sending out another round anyway, then it
is a good opportunity to update the proposed log message.

;-)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing
  2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
                       ` (4 preceding siblings ...)
  2024-05-29 16:59     ` [GSoC][PATCH v3 5/5] t: improve the test-case for parse_names() Chandra Pratap
@ 2024-05-30  4:35     ` Patrick Steinhardt
  2024-05-30  7:52       ` Christian Couder
  5 siblings, 1 reply; 18+ messages in thread
From: Patrick Steinhardt @ 2024-05-30  4:35 UTC (permalink / raw)
  To: Chandra Pratap; +Cc: git, Christian Couder

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Wed, May 29, 2024 at 10:29:26PM +0530, Chandra Pratap wrote:
> In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
> testing framework written entirely in C was introduced to the Git project
> aimed at simplifying testing and reducing test run times.
> Currently, tests for the reftable refs-backend are performed by a custom
> testing framework defined by reftable/test_framework.{c, h}. Port
> reftable/basics_test.c to the unit testing framework and improve upon
> the ported test.
> 
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>

This version looks good to me, thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing
  2024-05-30  4:35     ` [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing Patrick Steinhardt
@ 2024-05-30  7:52       ` Christian Couder
  2024-05-30 14:33         ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Christian Couder @ 2024-05-30  7:52 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Chandra Pratap, git, Christian Couder

On Thu, May 30, 2024 at 6:36 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Wed, May 29, 2024 at 10:29:26PM +0530, Chandra Pratap wrote:
> > In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
> > testing framework written entirely in C was introduced to the Git project
> > aimed at simplifying testing and reducing test run times.
> > Currently, tests for the reftable refs-backend are performed by a custom
> > testing framework defined by reftable/test_framework.{c, h}. Port
> > reftable/basics_test.c to the unit testing framework and improve upon
> > the ported test.
> >
> > Mentored-by: Patrick Steinhardt <ps@pks.im>
> > Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> > Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
> This version looks good to me, thanks!

It looks good to me too.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing
  2024-05-30  7:52       ` Christian Couder
@ 2024-05-30 14:33         ` Junio C Hamano
  0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2024-05-30 14:33 UTC (permalink / raw)
  To: Christian Couder
  Cc: Patrick Steinhardt, Chandra Pratap, git, Christian Couder

Christian Couder <christian.couder@gmail.com> writes:

> On Thu, May 30, 2024 at 6:36 AM Patrick Steinhardt <ps@pks.im> wrote:
>>
>> On Wed, May 29, 2024 at 10:29:26PM +0530, Chandra Pratap wrote:
>> > In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
>> > testing framework written entirely in C was introduced to the Git project
>> > aimed at simplifying testing and reducing test run times.
>> > Currently, tests for the reftable refs-backend are performed by a custom
>> > testing framework defined by reftable/test_framework.{c, h}. Port
>> > reftable/basics_test.c to the unit testing framework and improve upon
>> > the ported test.
>> >
>> > Mentored-by: Patrick Steinhardt <ps@pks.im>
>> > Mentored-by: Christian Couder <chriscool@tuxfamily.org>
>> > Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>>
>> This version looks good to me, thanks!
>
> It looks good to me too.

Thanks, all.  Queued.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-05-30 14:33 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <--in-reply-to=20240528113856.8348-1-chandrapratap3519@gmail.com>
2024-05-29  6:55 ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Chandra Pratap
2024-05-29  6:55   ` [GSoC][PATCH v2 1/4] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
2024-05-29  6:55   ` [GSoC][PATCH v2 2/4] t: move tests from reftable/stack_test.c to the new unit test Chandra Pratap
2024-05-29  6:55   ` [GSoC][PATCH v2 3/4] t: move tests from reftable/record_test.c " Chandra Pratap
2024-05-29  9:30     ` Patrick Steinhardt
2024-05-29 22:38       ` Junio C Hamano
2024-05-29  6:55   ` [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names() Chandra Pratap
2024-05-29  9:30     ` Patrick Steinhardt
2024-05-29  9:29   ` [GSoC][PATCH v2 0/4] t: port reftable/basics_test.c to the unit testing Patrick Steinhardt
2024-05-29 16:59   ` [GSoC][PATCH v3 " Chandra Pratap
2024-05-29 16:59     ` [GSoC][PATCH v3 1/5] t: move reftable/basics_test.c to the unit testing framework Chandra Pratap
2024-05-29 16:59     ` [GSoC][PATCH v3 2/5] t: move tests from reftable/stack_test.c to the new unit test Chandra Pratap
2024-05-29 16:59     ` [GSoC][PATCH v3 3/5] t: move tests from reftable/record_test.c " Chandra Pratap
2024-05-29 16:59     ` [GSoC][PATCH v3 4/5] t: add test for put_be16() Chandra Pratap
2024-05-29 16:59     ` [GSoC][PATCH v3 5/5] t: improve the test-case for parse_names() Chandra Pratap
2024-05-30  4:35     ` [GSoC][PATCH v3 0/4] t: port reftable/basics_test.c to the unit testing Patrick Steinhardt
2024-05-30  7:52       ` Christian Couder
2024-05-30 14:33         ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).