Re: [GSoC][PATCH v2 4/4] t: add test for put_be16() and improve test-case for parse_names()
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 29, 2024, 09:30 UTC
- Message-ID
- <Zlb1oiN6E4Isrnmg@tanuki>
- In-Reply-To
- <20240529070341.4248-5-chandrapratap3519@gmail.com>
On Wed, May 29, 2024 at 12:25:12PM +0530, Chandra Pratap wrote:
Show 30 quoted lines
> 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.
Show 20 quoted lines
> @@ -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