On Tue, Apr 29, 2025 at 06:53:02PM +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++)`
> Adapt functions in lib-reftable.{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 change migrates the helper
> functions back into `lib-reftable.{c,h}`.
>
> Signed-off-by: Seyi Kuforiji <kuforiji98@xxxxxxxxx>
> ---
> Makefile | 4 +-
> t/meson.build | 4 +-
> t/unit-tests/lib-reftable.c | 26 +++++------
> t/unit-tests/lib-reftable.h | 6 +--
> t/unit-tests/unit-test.c | 93 -------------------------------------
> t/unit-tests/unit-test.h | 16 -------
> 6 files changed, 20 insertions(+), 129 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 0b42893611..7e646e16ee 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1379,12 +1379,12 @@ CLAR_TEST_SUITES += u-urlmatch-normalization
> CLAR_TEST_PROG = $(UNIT_TEST_BIN)/unit-tests$(X)
> CLAR_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(CLAR_TEST_SUITES))
> CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/clar/clar.o
> -CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
> CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-oid.o
> +CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o
> +CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
>
> UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
> UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
> -UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o
>
`UNIT_TEST_PROGS` only contains the "unit-test" binary now, right? Does
this mean that we can simplify build rules in our Makefile now?
> diff --git a/t/meson.build b/t/meson.build
> index 8fa00fc9ef..7c305a90b5 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -26,8 +26,9 @@ clar_test_suites = [
>
> clar_sources = [
> 'unit-tests/clar/clar.c',
> + 'unit-tests/lib-oid.c',
> + 'unit-tests/lib-reftable.c',
> 'unit-tests/unit-test.c',
> - 'unit-tests/lib-oid.c'
> ]
>
> clar_decls_h = custom_target(
> @@ -69,7 +70,6 @@ foreach unit_test_program : unit_test_programs
> unit_test = executable(unit_test_name,
> sources: [
> 'unit-tests/test-lib.c',
> - 'unit-tests/lib-reftable.c',
> unit_test_program,
> ],
> dependencies: [libgit_commonmain],
Can't we remove this completely? `unit_test_programs` is empty now, so
this loop is a no-op now.
> diff --git a/t/unit-tests/lib-reftable.h b/t/unit-tests/lib-reftable.h
> index e4c360fa7e..2958db5dc0 100644
> --- a/t/unit-tests/lib-reftable.h
> +++ b/t/unit-tests/lib-reftable.h
> @@ -6,12 +6,12 @@
>
> struct reftable_buf;
>
> -void t_reftable_set_hash(uint8_t *p, int i, enum reftable_hash id);
> +void cl_reftable_set_hash(uint8_t *p, int i, enum reftable_hash id);
>
> -struct reftable_writer *t_reftable_strbuf_writer(struct reftable_buf *buf,
> +struct reftable_writer *cl_reftable_strbuf_writer(struct reftable_buf *buf,
> struct reftable_write_options *opts);
>
> -void t_reftable_write_to_buf(struct reftable_buf *buf,
> +void cl_reftable_write_to_buf(struct reftable_buf *buf,
> struct reftable_ref_record *refs,
> size_t nrecords,
> struct reftable_log_record *logs,
It is quite weird that we declare the replacement functions in
"unit-test.h" in the first commit only to remove them at a later point.
It would make way more sense if we introduced the functions in
"t/unit/lib-reftable.{c,h}" right from the start and then only remove
the unused functions in the last step.
Patrick
