| Message ID | 20250220082959.10854-2-kuforiji98@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | t/unit-tests: convert unit-tests to use clar | expand |
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);
On Thu, Feb 20, 2025 at 02:38:21PM +0000, Phillip Wood wrote: > 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
On Thu, Feb 20, 2025 at 09:29:55AM +0100, Seyi Kuforiji wrote: > 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); Nit: let's add a space between the function decarations. I'd also add a blank line into the comments before the "For example" paragraph and after it to make it easier to parse. Patrick
Hi Seyi On 20/02/2025 08:29, Seyi Kuforiji wrote: > +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); These last two lines would be better written as cl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0); So that if it fails the message shows which function was being called Best Wishes Phillip
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);
`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. 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(+)