| Message ID | 20231016134421.21659-1-phillip.wood123@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
Phillip Wood <phillip.wood123@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Here are a couple of cleanups for the unit test framework that I > noticed. Thanks. I trust that this will be squashed into the next update, but in the meantime, I'll include it in the copy of the series I have (without squashing). Here is another one I noticed. ----- >8 --------- >8 --------- >8 ----- Subject: [PATCH] fixup! ci: run unit tests in CI A CI job failed due to contrib/coccinelle/equals-null.cocci and suggested this change, which seems sensible. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/unit-tests/t-strbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/unit-tests/t-strbuf.c b/t/unit-tests/t-strbuf.c index c2fcb0cbd6..8388442426 100644 --- a/t/unit-tests/t-strbuf.c +++ b/t/unit-tests/t-strbuf.c @@ -28,7 +28,7 @@ static void setup_populated(void (*f)(struct strbuf*, void*), char *init_str, vo static int assert_sane_strbuf(struct strbuf *buf) { /* Initialized strbufs should always have a non-NULL buffer */ - if (buf->buf == NULL) + if (!buf->buf) return 0; /* Buffers should always be NUL-terminated */ if (buf->buf[buf->len] != '\0')
On 2023.10.16 14:43, Phillip Wood wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Here are a couple of cleanups for the unit test framework that I > noticed. > > Update the documentation of the example custom check to reflect the > change in return value of test_assert() and mention that > checks should be careful when dereferencing pointer arguments. > > Also avoid evaluating macro augments twice in check_int() and > friends. The global variable test__tmp was introduced to avoid > evaluating the arguments to these macros more than once but the macros > failed to use it when passing the values being compared to > check_int_loc(). > > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > t/unit-tests/test-lib.h | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) Applied in v9, thanks!
On 2023.10.16 09:41, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > > > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > > > Here are a couple of cleanups for the unit test framework that I > > noticed. > > Thanks. I trust that this will be squashed into the next update, > but in the meantime, I'll include it in the copy of the series I > have (without squashing). Here is another one I noticed. > > ----- >8 --------- >8 --------- >8 ----- > Subject: [PATCH] fixup! ci: run unit tests in CI > > A CI job failed due to contrib/coccinelle/equals-null.cocci > and suggested this change, which seems sensible. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > t/unit-tests/t-strbuf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied in v9, thanks!
Josh Steadmon <steadmon@google.com> writes: > On 2023.10.16 09:41, Junio C Hamano wrote: >> Phillip Wood <phillip.wood123@gmail.com> writes: >> >> > From: Phillip Wood <phillip.wood@dunelm.org.uk> >> > >> > Here are a couple of cleanups for the unit test framework that I >> > noticed. >> >> Thanks. I trust that this will be squashed into the next update, >> but in the meantime, I'll include it in the copy of the series I >> have (without squashing). Here is another one I noticed. >> >> ----- >8 --------- >8 --------- >8 ----- >> Subject: [PATCH] fixup! ci: run unit tests in CI >> >> A CI job failed due to contrib/coccinelle/equals-null.cocci >> and suggested this change, which seems sensible. >> >> Signed-off-by: Junio C Hamano <gitster@pobox.com> >> --- >> t/unit-tests/t-strbuf.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Applied in v9, thanks! Thanks for working well together.
Josh Steadmon <steadmon@google.com> writes: > On 2023.10.16 14:43, Phillip Wood wrote: >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> Here are a couple of cleanups for the unit test framework that I >> noticed. >> >> Update the documentation of the example custom check to reflect the >> change in return value of test_assert() and mention that >> checks should be careful when dereferencing pointer arguments. >> >> Also avoid evaluating macro augments twice in check_int() and >> friends. The global variable test__tmp was introduced to avoid >> evaluating the arguments to these macros more than once but the macros >> failed to use it when passing the values being compared to >> check_int_loc(). >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> t/unit-tests/test-lib.h | 26 ++++++++++++++++---------- >> 1 file changed, 16 insertions(+), 10 deletions(-) > > Applied in v9, thanks! Thanks for working well together.
diff --git a/t/unit-tests/test-lib.h b/t/unit-tests/test-lib.h index 8df3804914..a8f07ae0b7 100644 --- a/t/unit-tests/test-lib.h +++ b/t/unit-tests/test-lib.h @@ -42,18 +42,21 @@ void test_msg(const char *format, ...); /* * Test checks are built around test_assert(). checks return 1 on - * success, 0 on failure. If any check fails then the test will - * fail. To create a custom check define a function that wraps - * test_assert() and a macro to wrap that function. For example: + * success, 0 on failure. If any check fails then the test will fail. To + * create a custom check define a function that wraps test_assert() and + * a macro to wrap that function to provide a source location and + * stringified arguments. Custom checks that take pointer arguments + * should be careful to check that they are non-NULL before + * dereferencing them. For example: * * static int check_oid_loc(const char *loc, const char *check, * struct object_id *a, struct object_id *b) * { - * int res = test_assert(loc, check, oideq(a, b)); + * int res = test_assert(loc, check, a && b && oideq(a, b)); * - * if (res) { - * test_msg(" left: %s", oid_to_hex(a); - * test_msg(" right: %s", oid_to_hex(a); + * if (!res) { + * test_msg(" left: %s", a ? oid_to_hex(a) : "NULL"; + * test_msg(" right: %s", b ? oid_to_hex(a) : "NULL"; * * } * return res; @@ -79,7 +82,8 @@ int check_bool_loc(const char *loc, const char *check, int ok); #define check_int(a, op, b) \ (test__tmp[0].i = (a), test__tmp[1].i = (b), \ check_int_loc(TEST_LOCATION(), #a" "#op" "#b, \ - test__tmp[0].i op test__tmp[1].i, a, b)) + test__tmp[0].i op test__tmp[1].i, \ + test__tmp[0].i, test__tmp[1].i)) int check_int_loc(const char *loc, const char *check, int ok, intmax_t a, intmax_t b); @@ -90,7 +94,8 @@ int check_int_loc(const char *loc, const char *check, int ok, #define check_uint(a, op, b) \ (test__tmp[0].u = (a), test__tmp[1].u = (b), \ check_uint_loc(TEST_LOCATION(), #a" "#op" "#b, \ - test__tmp[0].u op test__tmp[1].u, a, b)) + test__tmp[0].u op test__tmp[1].u, \ + test__tmp[0].u, test__tmp[1].u)) int check_uint_loc(const char *loc, const char *check, int ok, uintmax_t a, uintmax_t b); @@ -101,7 +106,8 @@ int check_uint_loc(const char *loc, const char *check, int ok, #define check_char(a, op, b) \ (test__tmp[0].c = (a), test__tmp[1].c = (b), \ check_char_loc(TEST_LOCATION(), #a" "#op" "#b, \ - test__tmp[0].c op test__tmp[1].c, a, b)) + test__tmp[0].c op test__tmp[1].c, \ + test__tmp[0].c, test__tmp[1].c)) int check_char_loc(const char *loc, const char *check, int ok, char a, char b);