diff mbox series

[2/2] t: improve upon reftable/basics_test.c in the unit testing framework

Message ID 20240528113856.8348-3-chandrapratap3519@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] t: move reftable/basics_test.c to the unit testing framework | expand

Commit Message

Chandra Pratap May 28, 2024, 11:30 a.m. UTC
Enhance the new test for reftable/basics.{c, h} in the unit testing
framework. The enhancements include:
- Move tests for functions in reftable/basics.{c, h} from
reftable/record_test.c and reftable/stack_test.c to the new unit test.
- Add tests for functions that are not currently tested, like put_be16.
- Improve the test-cases for the already existing tests.

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 ---------------
 reftable/stack_test.c            | 25 -----------
 t/unit-tests/t-reftable-basics.c | 77 +++++++++++++++++++++++++++-----
 3 files changed, 65 insertions(+), 74 deletions(-)

Comments

Christian Couder May 28, 2024, 12:46 p.m. UTC | #1
On Tue, May 28, 2024 at 1:40 PM Chandra Pratap
<chandrapratap3519@gmail.com> wrote:
>
> Enhance the new test for reftable/basics.{c, h} in the unit testing
> framework. The enhancements include:
> - Move tests for functions in reftable/basics.{c, h} from
> reftable/record_test.c and reftable/stack_test.c to the new unit test.

It might be easier for readers if one patch moved tests from
reftable/record_test.c and a separate patch moved tests from
reftable/stack_test.c.

> - Add tests for functions that are not currently tested, like put_be16.

This could perhaps be done in a separate patch.

> - Improve the test-cases for the already existing tests.

Are these improvements to tests that were moved in patch 1/2 of this
series? If that's the case, these improvements could be in a separate
patch too.
Patrick Steinhardt May 29, 2024, 6 a.m. UTC | #2
On Tue, May 28, 2024 at 02:46:17PM +0200, Christian Couder wrote:
> On Tue, May 28, 2024 at 1:40 PM Chandra Pratap
> <chandrapratap3519@gmail.com> wrote:
> >
> > Enhance the new test for reftable/basics.{c, h} in the unit testing
> > framework. The enhancements include:
> > - Move tests for functions in reftable/basics.{c, h} from
> > reftable/record_test.c and reftable/stack_test.c to the new unit test.
> 
> It might be easier for readers if one patch moved tests from
> reftable/record_test.c and a separate patch moved tests from
> reftable/stack_test.c.
> 
> > - Add tests for functions that are not currently tested, like put_be16.
> 
> This could perhaps be done in a separate patch.
> 
> > - Improve the test-cases for the already existing tests.
> 
> Are these improvements to tests that were moved in patch 1/2 of this
> series? If that's the case, these improvements could be in a separate
> patch too.

Some of the test cases are extended to also cover additional edge cases.
But I agree that this is easy to miss, and also think that this patch
should be split up further.

Patrick
diff mbox series

Patch

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/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 b6088e1ddd..53fce33d53 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -51,36 +51,87 @@  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);
 }
 
 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);
 }
 
 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_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[])
@@ -90,6 +141,8 @@  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");
+	TEST(test_be_roundtrip(), "put_be24, get_be24 and put_be16 work");
 
 	return test_done();
 }