Re: [PATCH v2 04/10] t/unit-tests: convert reftable merged test to use clar

[Patrick Steinhardt <ps@pks.im>]

On Tue, Apr 29, 2025 at 06:52:56PM +0100, Seyi Kuforiji wrote:
> diff --git a/t/unit-tests/u-reftable-merged.c b/t/unit-tests/u-reftable-merged.c
> new file mode 100644
> index 0000000000..48c8f9f6b5
> --- /dev/null
> +++ b/t/unit-tests/u-reftable-merged.c
> @@ -0,0 +1,515 @@
> +/*
> +Copyright 2020 Google LLC
> +
> +Use of this source code is governed by a BSD-style
> +license that can be found in the LICENSE file or at
> +https://developers.google.com/open-source/licenses/bsd
> +*/
> +
> +#include "unit-test.h"
> +#include "lib-reftable.h"
> +#include "reftable/blocksource.h"
> +#include "reftable/constants.h"
> +#include "reftable/merged.h"
> +#include "reftable/reader.h"
> +#include "reftable/reftable-error.h"
> +#include "reftable/reftable-merged.h"
> +#include "reftable/reftable-writer.h"
> +
> +static struct reftable_merged_table *
> +merged_table_from_records(struct reftable_ref_record **refs,
> +			  struct reftable_block_source **source,
> +			  struct reftable_reader ***readers, const size_t *sizes,
> +			  struct reftable_buf *buf, const size_t n)
> +{
> +	struct reftable_merged_table *mt = NULL;
> +	struct reftable_write_options opts = {
> +		.block_size = 256,
> +	};
> +	int err;
> +
> +	REFTABLE_CALLOC_ARRAY(*readers, n);
> +	cl_assert(*readers != NULL);
> +	REFTABLE_CALLOC_ARRAY(*source, n);
> +	cl_assert(*source != NULL);
> +
> +	for (size_t i = 0; i < n; i++) {
> +		cl_reftable_write_to_buf(&buf[i], refs[i], sizes[i], NULL, 0, &opts);
> +		block_source_from_buf(&(*source)[i], &buf[i]);
> +
> +		err = reftable_reader_new(&(*readers)[i], &(*source)[i],
> +					  "name");
> +		cl_assert(err == 0);
> +	}
> +
> +	err = reftable_merged_table_new(&mt, *readers, n, REFTABLE_HASH_SHA1);
> +	cl_assert(err == 0);
> +	return mt;
> +}
> +
> +static void readers_destroy(struct reftable_reader **readers, const size_t n)
> +{
> +	for (size_t i = 0; i < n; i++)
> +		reftable_reader_decref(readers[i]);
> +	reftable_free(readers);
> +}
> +
> +void test_reftable_merged__merged_single_record(void)

This should be `__single_record(void)`. Same for the others, the
`__merged` prefix is somewhat duplicate as you already have it in the
test suite name.

> +void test_reftable_merged__merged_seek_multiple_times(void)

So, same here, let's rename to `__seek_multiple_times()`.

> +{
> +	struct reftable_ref_record r1[] = {
> +		{
> +			.refname = (char *) "a",
> +			.update_index = 1,
> +			.value_type = REFTABLE_REF_VAL1,
> +			.value.val1 = { 1 },
> +		},
> +		{
> +			.refname = (char *) "c",
> +			.update_index = 1,
> +			.value_type = REFTABLE_REF_VAL1,
> +			.value.val1 = { 2 },
> +		}
> +	};
> +	struct reftable_ref_record r2[] = {
> +		{
> +			.refname = (char *) "b",
> +			.update_index = 2,
> +			.value_type = REFTABLE_REF_VAL1,
> +			.value.val1 = { 3 },
> +		},
> +		{
> +			.refname = (char *) "d",
> +			.update_index = 2,
> +			.value_type = REFTABLE_REF_VAL1,
> +			.value.val1 = { 4 },
> +		},
> +	};
> +	struct reftable_ref_record *refs[] = {
> +		r1, r2,
> +	};
> +	size_t sizes[] = {
> +		ARRAY_SIZE(r1), ARRAY_SIZE(r2),
> +	};
> +	struct reftable_buf bufs[] = {
> +		REFTABLE_BUF_INIT, REFTABLE_BUF_INIT,
> +	};
> +	struct reftable_block_source *sources = NULL;
> +	struct reftable_reader **readers = NULL;
> +	struct reftable_ref_record rec = { 0 };
> +	struct reftable_iterator it = { 0 };
> +	struct reftable_merged_table *mt;
> +
> +	mt = merged_table_from_records(refs, &sources, &readers, sizes, bufs, 2);
> +	merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
> +
> +	for (size_t i = 0; i < 5; i++) {
> +		int err = reftable_iterator_seek_ref(&it, "c");
> +		cl_assert(err == 0);
> +
> +		cl_assert(reftable_iterator_next_ref(&it, &rec) == 0);
> +		cl_assert_equal_i(reftable_ref_record_equal(&rec, &r1[1],
> +													REFTABLE_HASH_SIZE_SHA1), 1);

Indentation is wrong. Our tabs are 8 spaces, so this is indented too
deep now.

> +		cl_assert(reftable_iterator_next_ref(&it, &rec) == 0);
> +		cl_assert_equal_i(reftable_ref_record_equal(&rec, &r2[1],
> +													REFTABLE_HASH_SIZE_SHA1), 1);

Same here. There are also other instances in this file.

Patrick