Re: [PATCH 1/5] t/unit-tests: implement oid helper functions in unit-tests.{c,h}

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Seyi

On 20/02/2025 08:29, Seyi Kuforiji wrote:
> `get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for
> oid-related tests to run without errors. In the current implementation,
> both functions are defined and declared in the
> `t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in
> the homegrown unit tests structure.
> 
> Implement equivalent functions in unit-tests.{c,h}. Both these functions
> become available for oid-related test files implemented using the clar
> testing framework, which requires them. This will be used by subsequent
> commits.

It is nice to see these tests being moved over to clar but I'm not sure 
that moving these functions into this file is good idea. All the unit 
tests need to link against unit-tests.o but only a subset will want 
access to these functions. Putting them in this file means that all the 
tests will now depend on code from strbuf.o and hex.o. I think we could 
add the new functions to lib-oid.c and then remove the old ones when 
there are not needed any more.

Best Wishes

Phillip

> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
> ---
>   t/unit-tests/unit-test.c | 42 ++++++++++++++++++++++++++++++++++++++++
>   t/unit-tests/unit-test.h | 19 ++++++++++++++++++
>   2 files changed, 61 insertions(+)
> 
> diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c
> index fa8818842a..13d54f192a 100644
> --- a/t/unit-tests/unit-test.c
> +++ b/t/unit-tests/unit-test.c
> @@ -1,5 +1,7 @@
>   #include "unit-test.h"
> +#include "hex.h"
>   #include "parse-options.h"
> +#include "strbuf.h"
>   #include "string-list.h"
>   #include "strvec.h"
>   
> @@ -62,3 +64,43 @@ int cmd_main(int argc, const char **argv)
>   	strvec_clear(&args);
>   	return ret;
>   }
> +
> +int init_hash_algo(void)
> +{
> +	static int algo = -1;
> +
> +	if (algo < 0) {
> +		const char *algo_name = getenv("GIT_TEST_DEFAULT_HASH");
> +		algo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1;
> +
> +		cl_assert(algo != GIT_HASH_UNKNOWN);
> +	}
> +	return algo;
> +}
> +
> +static void cl_parse_oid(const char *hex, struct object_id *oid,
> +				       const struct git_hash_algo *algop)
> +{
> +	int ret;
> +	size_t sz = strlen(hex);
> +	struct strbuf buf = STRBUF_INIT;
> +
> +	cl_assert(sz <= algop->hexsz);
> +
> +	strbuf_add(&buf, hex, sz);
> +	strbuf_addchars(&buf, '0', algop->hexsz - sz);
> +
> +	ret = get_oid_hex_algop(buf.buf, oid, algop);
> +	cl_assert_equal_i(ret, 0);
> +
> +	strbuf_release(&buf);
> +}
> +
> +
> +void cl_parse_any_oid(const char *hex, struct object_id *oid)
> +{
> +	int hash_algo = init_hash_algo();
> +
> +	cl_assert(hash_algo != GIT_HASH_UNKNOWN);
> +	cl_parse_oid(hex, oid, &hash_algos[hash_algo]);
> +}
> diff --git a/t/unit-tests/unit-test.h b/t/unit-tests/unit-test.h
> index 85e5d6a948..ebed51212f 100644
> --- a/t/unit-tests/unit-test.h
> +++ b/t/unit-tests/unit-test.h
> @@ -8,3 +8,22 @@
>   	snprintf(desc, sizeof(desc), fmt, __VA_ARGS__); \
>   	clar__fail(__FILE__, __func__, __LINE__, "Test failed.", desc, 1); \
>   } while (0)
> +
> +/*
> + * Convert arbitrary hex string to object_id.
> + * For example, passing "abc12" will generate
> + * "abc1200000000000000000000000000000000000" hex of length 40 for SHA-1 and
> + * create object_id with that.
> + * WARNING: passing a string of length more than the hexsz of respective hash
> + * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH
> + * environment variable.
> + */
> +void cl_parse_any_oid(const char *s, struct object_id *oid);
> +/*
> + * Returns one of GIT_HASH_{SHA1, SHA256, UNKNOWN} based on the value of
> + * GIT_TEST_DEFAULT_HASH environment variable. The fallback value in the
> + * absence of GIT_TEST_DEFAULT_HASH is GIT_HASH_SHA1. It also uses
> + * cl_assert(algo != GIT_HASH_UNKNOWN) before returning to verify if the
> + * GIT_TEST_DEFAULT_HASH's value is valid or not.
> + */
> +int init_hash_algo(void);