Subject: Re: [PATCH v3 03/10] t/unit-tests: convert reftable block test to use clar

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 3 Jun 2025 16:18:32 +0200

On Mon, Jun 02, 2025 at 01:25:51PM +0100, Seyi Kuforiji wrote:
> diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/u-reftable-block.c
> similarity index 74%
> rename from t/unit-tests/t-reftable-block.c
> rename to t/unit-tests/u-reftable-block.c
> index 52f1dae1c9..5d1419b801 100644
> --- a/t/unit-tests/t-reftable-block.c
> +++ b/t/unit-tests/u-reftable-block.c
> @@ -6,14 +6,15 @@ license that can be found in the LICENSE file or at
>  https://developers.google.com/open-source/licenses/bsd
>  */
>  
> -#include "test-lib.h"
> +#include "unit-test.h"
> +#include "lib-reftable-clar.h"
>  #include "reftable/block.h"
>  #include "reftable/blocksource.h"
>  #include "reftable/constants.h"
>  #include "reftable/reftable-error.h"
>  #include "strbuf.h"
>  
> -static void t_ref_block_read_write(void)
> +void test_reftable_block__index_read_write(void)
>  {
>  	const int header_off = 21; /* random */
>  	struct reftable_record recs[30];

The name here still looks wrong to me. This is about writing ref blocks,
not index blocks.

> @@ -220,12 +222,12 @@ static void t_obj_block_read_write(void)
>  	struct reftable_buf block_data = REFTABLE_BUF_INIT;
>  
>  	REFTABLE_CALLOC_ARRAY(block_data.buf, block_size);
> -	check(block_data.buf != NULL);
> +	cl_assert(block_data.buf != NULL);
>  	block_data.len = block_size;
>  
>  	ret = block_writer_init(&bw, REFTABLE_BLOCK_TYPE_OBJ, (uint8_t *) block_data.buf, block_size,
>  				header_off, hash_size(REFTABLE_HASH_SHA1));
> -	check(!ret);
> +	cl_assert(ret == 0);

All of these could be retained as `cl_assert(!ret)`, which is more
idiomatic in our codebase. Either that, or these should use
`cl_assert_equal_i()` to give better diagnostics.

Patrick