| Message ID | 20250224152704.70289-2-kuforiji98@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] t/unit-tests: implement clar specific oid helper functions | expand |
Seyi Kuforiji <kuforiji98@gmail.com> writes: > -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid, > +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; > > - if (!check(sz <= algop->hexsz)) { > - test_msg("BUG: hex string (%s) bigger than maximum allowed (%lu)", > - hex, (unsigned long)algop->hexsz); > - return -1; > - } > + 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); > - if (!check_int(ret, ==, 0)) > - test_msg("BUG: invalid hex input (%s) provided", hex); > + cl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0); > > strbuf_release(&buf); > - return ret; > } As you are not returning "ret" and making the function void, you made "int ret" an unused variable that needs removing. Thanks.
On Mon, 24 Feb 2025 at 18:56, Junio C Hamano <gitster@pobox.com> wrote: > > Seyi Kuforiji <kuforiji98@gmail.com> writes: > > > -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid, > > +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; > > > > - if (!check(sz <= algop->hexsz)) { > > - test_msg("BUG: hex string (%s) bigger than maximum allowed (%lu)", > > - hex, (unsigned long)algop->hexsz); > > - return -1; > > - } > > + 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); > > - if (!check_int(ret, ==, 0)) > > - test_msg("BUG: invalid hex input (%s) provided", hex); > > + cl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0); > > > > strbuf_release(&buf); > > - return ret; > > } > > As you are not returning "ret" and making the function void, you > made "int ret" an unused variable that needs removing. > > Thanks. Hi Junio, Do I send in a new patch series addressing this? Thanks, Seyi
On Tue, Feb 25, 2025 at 08:14:35AM +0100, Seyi Chamber wrote: > On Mon, 24 Feb 2025 at 18:56, Junio C Hamano <gitster@pobox.com> wrote: > > > > Seyi Kuforiji <kuforiji98@gmail.com> writes: > > > > > -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid, > > > +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; > > > > > > - if (!check(sz <= algop->hexsz)) { > > > - test_msg("BUG: hex string (%s) bigger than maximum allowed (%lu)", > > > - hex, (unsigned long)algop->hexsz); > > > - return -1; > > > - } > > > + 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); > > > - if (!check_int(ret, ==, 0)) > > > - test_msg("BUG: invalid hex input (%s) provided", hex); > > > + cl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0); > > > > > > strbuf_release(&buf); > > > - return ret; > > > } > > > > As you are not returning "ret" and making the function void, you > > made "int ret" an unused variable that needs removing. > > > > Thanks. > > Hi Junio, > > Do I send in a new patch series addressing this? The expectation would be that you reroll the series and send out a new version thereof that gets rid of the return value in the same commit where you stop returning it. This would typically be the case as long as your series has not yet been merged to `next`. Once it has been merged to `next`, you would have to send an entirely new patch series as a follow-up. Patrick
diff --git a/Makefile b/Makefile index bcf5ed3f85..81799488f0 100644 --- a/Makefile +++ b/Makefile @@ -1365,6 +1365,7 @@ 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 UNIT_TEST_PROGRAMS += t-oid-array UNIT_TEST_PROGRAMS += t-oidmap @@ -1381,7 +1382,6 @@ UNIT_TEST_PROGRAMS += t-trailer UNIT_TEST_PROGRAMS += t-urlmatch-normalization 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-oid.o UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o # xdiff and reftable libs may in turn depend on what is in libgit.a diff --git a/t/meson.build b/t/meson.build index 780939d49f..862cf1cfd4 100644 --- a/t/meson.build +++ b/t/meson.build @@ -14,6 +14,7 @@ clar_test_suites = [ clar_sources = [ 'unit-tests/clar/clar.c', 'unit-tests/unit-test.c', + 'unit-tests/lib-oid.c' ] clar_decls_h = custom_target( @@ -68,7 +69,6 @@ foreach unit_test_program : unit_test_programs unit_test = executable(unit_test_name, sources: [ 'unit-tests/test-lib.c', - 'unit-tests/lib-oid.c', 'unit-tests/lib-reftable.c', unit_test_program, ], diff --git a/t/unit-tests/lib-oid.c b/t/unit-tests/lib-oid.c index 8f0ccac532..8bf09ea1cb 100644 --- a/t/unit-tests/lib-oid.c +++ b/t/unit-tests/lib-oid.c @@ -1,9 +1,9 @@ -#include "test-lib.h" +#include "unit-test.h" #include "lib-oid.h" #include "strbuf.h" #include "hex.h" -int init_hash_algo(void) +int cl_setup_hash_algo(void) { static int algo = -1; @@ -11,42 +11,33 @@ int init_hash_algo(void) const char *algo_name = getenv("GIT_TEST_DEFAULT_HASH"); algo = algo_name ? hash_algo_by_name(algo_name) : GIT_HASH_SHA1; - if (!check(algo != GIT_HASH_UNKNOWN)) - test_msg("BUG: invalid GIT_TEST_DEFAULT_HASH value ('%s')", - algo_name); + cl_assert(algo != GIT_HASH_UNKNOWN); } return algo; } -static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid, +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; - if (!check(sz <= algop->hexsz)) { - test_msg("BUG: hex string (%s) bigger than maximum allowed (%lu)", - hex, (unsigned long)algop->hexsz); - return -1; - } + 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); - if (!check_int(ret, ==, 0)) - test_msg("BUG: invalid hex input (%s) provided", hex); + cl_assert_equal_i(get_oid_hex_algop(buf.buf, oid, algop), 0); strbuf_release(&buf); - return ret; } -int get_oid_arbitrary_hex(const char *hex, struct object_id *oid) + +void cl_parse_any_oid(const char *hex, struct object_id *oid) { - int hash_algo = init_hash_algo(); + int hash_algo = cl_setup_hash_algo(); - if (!check_int(hash_algo, !=, GIT_HASH_UNKNOWN)) - return -1; - return get_oid_arbitrary_hex_algop(hex, oid, &hash_algos[hash_algo]); + cl_assert(hash_algo != GIT_HASH_UNKNOWN); + cl_parse_oid(hex, oid, &hash_algos[hash_algo]); } diff --git a/t/unit-tests/lib-oid.h b/t/unit-tests/lib-oid.h index 4e77c04bd2..4031775104 100644 --- a/t/unit-tests/lib-oid.h +++ b/t/unit-tests/lib-oid.h @@ -5,6 +5,7 @@ /* * 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. @@ -12,14 +13,16 @@ * algo is not allowed. The hash algo is decided based on GIT_TEST_DEFAULT_HASH * environment variable. */ -int get_oid_arbitrary_hex(const char *s, struct object_id *oid); + +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 - * check(algo != GIT_HASH_UNKNOWN) before returning to verify if the + * 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); + +int cl_setup_hash_algo(void); #endif /* LIB_OID_H */ diff --git a/t/unit-tests/unit-test.c b/t/unit-tests/unit-test.c index fa8818842a..5af645048a 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"
`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. Adapt functions in lib-oid.{c,h} to use clar. 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> --- Makefile | 2 +- t/meson.build | 2 +- t/unit-tests/lib-oid.c | 31 +++++++++++-------------------- t/unit-tests/lib-oid.h | 9 ++++++--- t/unit-tests/unit-test.c | 2 ++ 5 files changed, 21 insertions(+), 25 deletions(-)