diff mbox series

[GSoC,v2,4/4] t: add test for put_be16() and improve test-case for parse_names()

Message ID 20240529070341.4248-5-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
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(-)

Comments

Patrick Steinhardt May 29, 2024, 9:30 a.m. UTC | #1
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
diff mbox series

Patch

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