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

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 21, 2025, 07:59 UTC
Message-ID: <Z7gySOeGuicIOWZS@pks.im>
In-Reply-To: <c9e8a5f8-fda6-4383-bbb3-0c960a4cf86e@gmail.com>

On Thu, Feb 20, 2025 at 02:38:21PM +0000, Phillip Wood wrote:

Show 21 quoted lines

> 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.

That should probably work, yeah. In that case we'd have to rename `init_hash_algo()`, e.g. to something like `cl_setup_hash_algo()`, to clearly show that it is part of the clar testing framework. I think having a common prefix for such helper functions would be good anyway so that the symbols never conflict with symbols we have in libgit.

Patrick