Re: [PATCH v2 01/10] t/unit-tests: implement reftable test helper functions in unit-test.{c,h}
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 2, 2025, 09:57 UTC
- Message-ID
- <aBSXEEjRyh2cQ4Jm@pks.im>
- In-Reply-To
- <20250429175302.23724-2-kuforiji98@gmail.com>
On Tue, Apr 29, 2025 at 06:52:53PM +0100, Seyi Kuforiji wrote:
Show 19 quoted lines
> 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.
Show 37 quoted lines
> 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)`.
Show 37 quoted lines
> + 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