Re: [PATCH v2 01/10] t/unit-tests: implement reftable test helper functions in unit-test.{c,h}

[Patrick Steinhardt <ps@pks.im>]

On Tue, Apr 29, 2025 at 06:52:53PM +0100, Seyi Kuforiji wrote:
> Helper functions defined in `t/unit-tests/lib-reftable.{c,h}` are
> required for the reftable-related test files to run efficeintly. In the
> current implementation these functions are designed to conform with our
> homegrown unit-testing structure. So in other to convert the reftable
> test files, there is need for a clar specific implementation of these
> helper functions.
> 
> type cast `for (size_t i = 0; i < (size_t)stats->ref_stats.blocks;
> i++)`, implement equivalent helper functions in unit-test.{c,h} to use
> clar. These functions conform with the clar testing framework and become
> available for all reftable-related test files implemented using the clar
> testing framework, which requires them. This will be used by subsequent
> commits.
> 
> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
> ---
>  t/unit-tests/unit-test.c | 93 ++++++++++++++++++++++++++++++++++++++++
>  t/unit-tests/unit-test.h | 16 +++++++
>  2 files changed, 109 insertions(+)

I think this functionality should be added to
"t/unit-tests/lib-reftable.{c,h}" instead of to the generic unit testing
library as it is highly specific to reftables.

> diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c
> index 5af645048a..6c2a4e6aa8 100644
> --- a/t/unit-tests/unit-test.c
> +++ b/t/unit-tests/unit-test.c
> @@ -1,10 +1,103 @@
>  #include "unit-test.h"
>  #include "hex.h"
>  #include "parse-options.h"
> +#include "reftable/constants.h"
> +#include "reftable/writer.h"
>  #include "strbuf.h"
>  #include "string-list.h"
>  #include "strvec.h"
>  
> +void cl_reftable_set_hash(uint8_t *p, int i, enum reftable_hash id)
> +{
> +	memset(p, (uint8_t)i, hash_size(id));
> +}
> +
> +static ssize_t strbuf_writer_write(void *b, const void *data, size_t sz)
> +{
> +	strbuf_add(b, data, sz);
> +	return sz;
> +}
> +
> +static int strbuf_writer_flush(void *arg UNUSED)
> +{
> +	return 0;
> +}
> +
> +struct reftable_writer *cl_reftable_strbuf_writer(struct reftable_buf *buf,
> +						 struct reftable_write_options *opts)
> +{
> +	struct reftable_writer *writer;
> +	int ret = reftable_writer_new(&writer, &strbuf_writer_write, &strbuf_writer_flush,
> +				      buf, opts);
> +	cl_assert(ret == 0);

We typically don't explicitly compare with zero, so this should rather
be `cl_assert(!ret)`.

> +	return writer;
> +}
> +
> +void cl_reftable_write_to_buf(struct reftable_buf *buf,
> +			     struct reftable_ref_record *refs,
> +			     size_t nrefs,
> +			     struct reftable_log_record *logs,
> +			     size_t nlogs,
> +			     struct reftable_write_options *_opts)
> +{
> +	struct reftable_write_options opts = { 0 };
> +	const struct reftable_stats *stats;
> +	struct reftable_writer *writer;
> +	uint64_t min = 0xffffffff;
> +	uint64_t max = 0;
> +	int ret;
> +
> +	if (_opts)
> +		opts = *_opts;
> +
> +	for (size_t i = 0; i < nrefs; i++) {
> +		uint64_t ui = refs[i].update_index;
> +		if (ui > max)
> +			max = ui;
> +		if (ui < min)
> +			min = ui;
> +	}
> +	for (size_t i = 0; i < nlogs; i++) {
> +		uint64_t ui = logs[i].update_index;
> +		if (ui > max)
> +			max = ui;
> +		if (ui < min)
> +			min = ui;
> +	}
> +
> +	writer = cl_reftable_strbuf_writer(buf, &opts);
> +	reftable_writer_set_limits(writer, min, max);

This function may return an error, as well, so let's verify it while at
it.

Patrick