diff mbox series

[GSoC,v2,3/4] t: move tests from reftable/record_test.c to the new unit test

Message ID 20240529070341.4248-4-chandrapratap3519@gmail.com (mailing list archive)
State Superseded
Headers show
Series t: port reftable/basics_test.c to the unit testing | expand

Commit Message

Chandra Pratap May 29, 2024, 6:55 a.m. UTC 
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(-)

Comments

Patrick Steinhardt May 29, 2024, 9:30 a.m. UTC | #1 
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
Junio C Hamano May 29, 2024, 10:38 p.m. UTC | #2 
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.

;-)
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/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();
 }