On Sun, Mar 10, 2024 at 03:48:19PM +0100, Achu Luma wrote:
> In the recent codebase update (8bf6fbd (Merge branch
> 'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was
> merged, providing a standardized approach for testing C code. Prior to
> this update, some unit tests relied on the test helper mechanism,
> lacking a dedicated unit testing framework. It's more natural to perform
> these unit tests using the new unit test framework.
>
> Let's migrate the unit tests for strcmp-offset functionality from the
> legacy approach using the test-tool command `test-tool strcmp-offset` in
> helper/test-strcmp-offset.c to the new unit testing framework
> (t/unit-tests/test-lib.h).
>
> The migration involves refactoring the tests to utilize the testing
> macros provided by the framework (TEST() and check_*()).
>
> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> Signed-off-by: Achu Luma <ach.lumap@xxxxxxxxx>
> ---
> Makefile | 2 +-
> t/helper/test-strcmp-offset.c | 23 -----------------------
> t/helper/test-tool.c | 1 -
> t/helper/test-tool.h | 1 -
> t/t0065-strcmp-offset.sh | 22 ----------------------
> t/unit-tests/t-strcmp-offset.c | 31 +++++++++++++++++++++++++++++++
> 6 files changed, 32 insertions(+), 48 deletions(-)
> delete mode 100644 t/helper/test-strcmp-offset.c
> delete mode 100755 t/t0065-strcmp-offset.sh
> create mode 100644 t/unit-tests/t-strcmp-offset.c
>
> diff --git a/Makefile b/Makefile
> index 4e255c81f2..b8d7019ad7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -850,7 +850,6 @@ TEST_BUILTINS_OBJS += test-sha1.o
> TEST_BUILTINS_OBJS += test-sha256.o
> TEST_BUILTINS_OBJS += test-sigchain.o
> TEST_BUILTINS_OBJS += test-simple-ipc.o
> -TEST_BUILTINS_OBJS += test-strcmp-offset.o
> TEST_BUILTINS_OBJS += test-string-list.o
> TEST_BUILTINS_OBJS += test-submodule-config.o
> TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
> @@ -1347,6 +1346,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool
> UNIT_TEST_PROGRAMS += t-strbuf
> UNIT_TEST_PROGRAMS += t-ctype
> UNIT_TEST_PROGRAMS += t-prio-queue
> +UNIT_TEST_PROGRAMS += t-strcmp-offset
> UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
> UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))
> UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
> diff --git a/t/helper/test-strcmp-offset.c b/t/helper/test-strcmp-offset.c
> deleted file mode 100644
> index d8473cf2fc..0000000000
> --- a/t/helper/test-strcmp-offset.c
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -#include "test-tool.h"
> -#include "read-cache-ll.h"
> -
> -int cmd__strcmp_offset(int argc UNUSED, const char **argv)
> -{
> - int result;
> - size_t offset;
> -
> - if (!argv[1] || !argv[2])
> - die("usage: %s <string1> <string2>", argv[0]);
> -
> - result = strcmp_offset(argv[1], argv[2], &offset);
> -
> - /*
> - * Because different CRTs behave differently, only rely on signs
> - * of the result values.
> - */
> - result = (result < 0 ? -1 :
> - result > 0 ? 1 :
> - 0);
> - printf("%d %"PRIuMAX"\n", result, (uintmax_t)offset);
> - return 0;
> -}
> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
> index 482a1e58a4..3d56de82fd 100644
> --- a/t/helper/test-tool.c
> +++ b/t/helper/test-tool.c
> @@ -76,7 +76,6 @@ static struct test_cmd cmds[] = {
> { "sha256", cmd__sha256 },
> { "sigchain", cmd__sigchain },
> { "simple-ipc", cmd__simple_ipc },
> - { "strcmp-offset", cmd__strcmp_offset },
> { "string-list", cmd__string_list },
> { "submodule", cmd__submodule },
> { "submodule-config", cmd__submodule_config },
> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
> index b1be7cfcf5..8d76a8c1e1 100644
> --- a/t/helper/test-tool.h
> +++ b/t/helper/test-tool.h
> @@ -69,7 +69,6 @@ int cmd__oid_array(int argc, const char **argv);
> int cmd__sha256(int argc, const char **argv);
> int cmd__sigchain(int argc, const char **argv);
> int cmd__simple_ipc(int argc, const char **argv);
> -int cmd__strcmp_offset(int argc, const char **argv);
> int cmd__string_list(int argc, const char **argv);
> int cmd__submodule(int argc, const char **argv);
> int cmd__submodule_config(int argc, const char **argv);
> diff --git a/t/t0065-strcmp-offset.sh b/t/t0065-strcmp-offset.sh
> deleted file mode 100755
> index 94e34c83ed..0000000000
> --- a/t/t0065-strcmp-offset.sh
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -#!/bin/sh
> -
> -test_description='Test strcmp_offset functionality'
> -
> -TEST_PASSES_SANITIZE_LEAK=true
> -. ./test-lib.sh
> -
> -while read s1 s2 expect
> -do
> - test_expect_success "strcmp_offset($s1, $s2)" '
> - echo "$expect" >expect &&
> - test-tool strcmp-offset "$s1" "$s2" >actual &&
> - test_cmp expect actual
> - '
> -done <<-EOF
> -abc abc 0 3
> -abc def -1 0
> -abc abz -1 2
> -abc abcdef -1 3
> -EOF
> -
> -test_done
> diff --git a/t/unit-tests/t-strcmp-offset.c b/t/unit-tests/t-strcmp-offset.c
> new file mode 100644
> index 0000000000..176d2ed04a
> --- /dev/null
> +++ b/t/unit-tests/t-strcmp-offset.c
> @@ -0,0 +1,31 @@
> +#include "test-lib.h"
> +#include "read-cache-ll.h"
> +
> +static void check_strcmp_offset(const char *string1, const char *string2, int expect_result, uintmax_t expect_offset)
Tiny nit: there's two spaces in front of `uintmax_t expect_offset`.
> +{
> + int result;
> + size_t offset;
> +
> + result = strcmp_offset(string1, string2, &offset);
> +
> + /* Because different CRTs behave differently, only rely on signs of the result values. */
> + result = (result < 0 ? -1 :
> + result > 0 ? 1 :
> + 0);
I was wondering a bit why we munge the result like this and don't
compare that it's an exact match. But this isn't much of an isuse in my
opinion given that this is just a straight port of the old code.
Other than that this patch looks good to me, thanks!
Patrick
> + check_int(result, ==, expect_result);
> + check_uint((uintmax_t)offset, ==, expect_offset);
> +}
> +
> +#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \
> + TEST(check_strcmp_offset(string1, string2, expect_result, expect_offset), \
> + "strcmp_offset(%s, %s) works", #string1, #string2)
> +
> +int cmd_main(int argc, const char **argv) {
> + TEST_STRCMP_OFFSET("abc", "abc", 0, 3);
> + TEST_STRCMP_OFFSET("abc", "def", -1, 0);
> + TEST_STRCMP_OFFSET("abc", "abz", -1, 2);
> + TEST_STRCMP_OFFSET("abc", "abcdef", -1, 3);
> +
> + return test_done();
> +}
> --
> 2.43.0.windows.1
>
>
Attachment:
signature.asc
Description: PGP signature
