Re: [PATCH 2/4] t/unit-tests: convert mem-pool test to use clar test framework

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jan 16, 2025 at 11:49:09AM +0100, Seyi Kuforiji wrote:
> Adapt the mem-pool test script to use clar framework by using clar
> assertions where necessary. Following the consensus to convert the
> unit-tests scripts found in the t/unit-tests folder to clar driven by
> Patrick Steinhardt.

I think it's a minor detail that isn't really worth mentioning that I
was the one introducing the clar, so I'd leave my name out of it. This
also applies to subsequent commit messages.

> diff --git a/t/unit-tests/t-mem-pool.c b/t/unit-tests/t-mem-pool.c
> deleted file mode 100644
> index fe500c704b..0000000000
> --- a/t/unit-tests/t-mem-pool.c
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -#include "test-lib.h"
> -#include "mem-pool.h"
> -
> -static void setup_static(void (*f)(struct mem_pool *), size_t block_alloc)
> -{
> -	struct mem_pool pool = { .block_alloc = block_alloc };
> -	f(&pool);
> -	mem_pool_discard(&pool, 0);
> -}
> -
> -static void t_calloc_100(struct mem_pool *pool)
> -{
> -	size_t size = 100;
> -	char *buffer = mem_pool_calloc(pool, 1, size);
> -	for (size_t i = 0; i < size; i++)
> -		check_int(buffer[i], ==, 0);
> -	if (!check(pool->mp_block != NULL))
> -		return;
> -	check(pool->mp_block->next_free != NULL);
> -	check(pool->mp_block->end != NULL);
> -}
> -
> -int cmd_main(int argc UNUSED, const char **argv UNUSED)
> -{
> -	TEST(setup_static(t_calloc_100, 1024 * 1024),
> -	     "mem_pool_calloc returns 100 zeroed bytes with big block");
> -	TEST(setup_static(t_calloc_100, 1),
> -	     "mem_pool_calloc returns 100 zeroed bytes with tiny block");
> -
> -	return test_done();
> -}

Mh, too bad that Git doesn't render it as a 

> diff --git a/t/unit-tests/u-mem-pool.c b/t/unit-tests/u-mem-pool.c
> new file mode 100644
> index 0000000000..36e31a3201
> --- /dev/null
> +++ b/t/unit-tests/u-mem-pool.c
> @@ -0,0 +1,26 @@
> +#include "unit-test.h"
> +#include "mem-pool.h"
> +
> +static void t_calloc_100(size_t block_alloc)

Can we maybe give this a more descriptive name? Something like
`test_many_pool_allocations()` maybe?

> +{
> +	struct mem_pool pool = { .block_alloc = block_alloc };
> +	size_t size = 100;
> +	char *buffer = mem_pool_calloc(&pool, 1, size);
> +	for (size_t i = 0; i < size; i++)
> +		cl_assert_equal_i(0, buffer[i]);
> +	cl_assert(pool.mp_block != NULL);
> +	cl_assert(pool.mp_block->next_free != NULL);
> +	cl_assert(pool.mp_block->end != NULL);
> +	mem_pool_discard(&pool, 0);
> +}
> +
> +void test_mem_pool__big_block(void)
> +{
> +	t_calloc_100(1024 * 1024);
> +

There is a needless empty line here.

Other than that the changes look good to me.

Patrick