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

[Patrick Steinhardt <ps@pks.im>]

[-- 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 --]