{"thread":{"id":"61083","subject":"[Outreachy][PATCH] Port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c","startedAt":"2024-03-10T14:48:39Z","lastAt":"2024-05-20T20:55:29Z","messageCount":6,"participants":["Achu Luma","Patrick Steinhardt","Ghanshyam Thakkar","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"490322","messageId":"20240310144819.4379-1-ach.lumap@gmail.com","threadId":"61083","inReplyTo":null,"subject":"[Outreachy][PATCH] Port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c","fromName":"Achu Luma","fromEmail":"ach.lumap@gmail.com","sentAt":"2024-03-10T14:48:19Z","receivedAt":"2024-03-10T14:48:39Z","isPatch":true,"sender":{"key":"ach.lumap@gmail.com","avatar":"https://avatars.githubusercontent.com/u/142904668?v=4"},"body":"In the recent codebase update (8bf6fbd (Merge branch\n'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was\nmerged, providing a standardized approach for testing C code. Prior to\nthis update, some unit tests relied on the test helper mechanism,\nlacking a dedicated unit testing framework. It's more natural to perform\nthese unit tests using the new unit test framework.\n\nLet's migrate the unit tests for strcmp-offset functionality from the\nlegacy approach using the test-tool command `test-tool strcmp-offset` in\nhelper/test-strcmp-offset.c to the new unit testing framework\n(t/unit-tests/test-lib.h).\n\nThe migration involves refactoring the tests to utilize the testing\nmacros provided by the framework (TEST() and check_*()).\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Achu Luma <ach.lumap@gmail.com>\n---\n Makefile                       |  2 +-\n t/helper/test-strcmp-offset.c  | 23 -----------------------\n t/helper/test-tool.c           |  1 -\n t/helper/test-tool.h           |  1 -\n t/t0065-strcmp-offset.sh       | 22 ----------------------\n t/unit-tests/t-strcmp-offset.c | 31 +++++++++++++++++++++++++++++++\n 6 files changed, 32 insertions(+), 48 deletions(-)\n delete mode 100644 t/helper/test-strcmp-offset.c\n delete mode 100755 t/t0065-strcmp-offset.sh\n create mode 100644 t/unit-tests/t-strcmp-offset.c\n\ndiff --git a/Makefile b/Makefile\nindex 4e255c81f2..b8d7019ad7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -850,7 +850,6 @@ TEST_BUILTINS_OBJS += test-sha1.o\n TEST_BUILTINS_OBJS += test-sha256.o\n TEST_BUILTINS_OBJS += test-sigchain.o\n TEST_BUILTINS_OBJS += test-simple-ipc.o\n-TEST_BUILTINS_OBJS += test-strcmp-offset.o\n TEST_BUILTINS_OBJS += test-string-list.o\n TEST_BUILTINS_OBJS += test-submodule-config.o\n TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n@@ -1347,6 +1346,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-ctype\n UNIT_TEST_PROGRAMS += t-prio-queue\n+UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o\ndiff --git a/t/helper/test-strcmp-offset.c b/t/helper/test-strcmp-offset.c\ndeleted file mode 100644\nindex d8473cf2fc..0000000000\n--- a/t/helper/test-strcmp-offset.c\n+++ /dev/null\n@@ -1,23 +0,0 @@\n-#include \"test-tool.h\"\n-#include \"read-cache-ll.h\"\n-\n-int cmd__strcmp_offset(int argc UNUSED, const char **argv)\n-{\n-\tint result;\n-\tsize_t offset;\n-\n-\tif (!argv[1] || !argv[2])\n-\t\tdie(\"usage: %s <string1> <string2>\", argv[0]);\n-\n-\tresult = strcmp_offset(argv[1], argv[2], &offset);\n-\n-\t/*\n-\t * Because different CRTs behave differently, only rely on signs\n-\t * of the result values.\n-\t */\n-\tresult = (result < 0 ? -1 :\n-\t\t\t  result > 0 ? 1 :\n-\t\t\t  0);\n-\tprintf(\"%d %\"PRIuMAX\"\\n\", result, (uintmax_t)offset);\n-\treturn 0;\n-}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 482a1e58a4..3d56de82fd 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -76,7 +76,6 @@ static struct test_cmd cmds[] = {\n \t{ \"sha256\", cmd__sha256 },\n \t{ \"sigchain\", cmd__sigchain },\n \t{ \"simple-ipc\", cmd__simple_ipc },\n-\t{ \"strcmp-offset\", cmd__strcmp_offset },\n \t{ \"string-list\", cmd__string_list },\n \t{ \"submodule\", cmd__submodule },\n \t{ \"submodule-config\", cmd__submodule_config },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex b1be7cfcf5..8d76a8c1e1 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -69,7 +69,6 @@ int cmd__oid_array(int argc, const char **argv);\n int cmd__sha256(int argc, const char **argv);\n int cmd__sigchain(int argc, const char **argv);\n int cmd__simple_ipc(int argc, const char **argv);\n-int cmd__strcmp_offset(int argc, const char **argv);\n int cmd__string_list(int argc, const char **argv);\n int cmd__submodule(int argc, const char **argv);\n int cmd__submodule_config(int argc, const char **argv);\ndiff --git a/t/t0065-strcmp-offset.sh b/t/t0065-strcmp-offset.sh\ndeleted file mode 100755\nindex 94e34c83ed..0000000000\n--- a/t/t0065-strcmp-offset.sh\n+++ /dev/null\n@@ -1,22 +0,0 @@\n-#!/bin/sh\n-\n-test_description='Test strcmp_offset functionality'\n-\n-TEST_PASSES_SANITIZE_LEAK=true\n-. ./test-lib.sh\n-\n-while read s1 s2 expect\n-do\n-\ttest_expect_success \"strcmp_offset($s1, $s2)\" '\n-\t\techo \"$expect\" >expect &&\n-\t\ttest-tool strcmp-offset \"$s1\" \"$s2\" >actual &&\n-\t\ttest_cmp expect actual\n-\t'\n-done <<-EOF\n-abc abc 0 3\n-abc def -1 0\n-abc abz -1 2\n-abc abcdef -1 3\n-EOF\n-\n-test_done\ndiff --git a/t/unit-tests/t-strcmp-offset.c b/t/unit-tests/t-strcmp-offset.c\nnew file mode 100644\nindex 0000000000..176d2ed04a\n--- /dev/null\n+++ b/t/unit-tests/t-strcmp-offset.c\n@@ -0,0 +1,31 @@\n+#include \"test-lib.h\"\n+#include \"read-cache-ll.h\"\n+\n+static void check_strcmp_offset(const char *string1, const char *string2, int expect_result,  uintmax_t expect_offset)\n+{\n+\tint result;\n+\tsize_t offset;\n+\n+\tresult = strcmp_offset(string1, string2, &offset);\n+\n+\t/* Because different CRTs behave differently, only rely on signs of the result values. */\n+\tresult = (result < 0 ? -1 :\n+\t\t\t  result > 0 ? 1 :\n+\t\t\t  0);\n+\n+\tcheck_int(result, ==, expect_result);\n+\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n+}\n+\n+#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \\\n+\t\tTEST(check_strcmp_offset(string1, string2, expect_result, expect_offset), \\\n+\t\t\t\"strcmp_offset(%s, %s) works\", #string1, #string2)\n+\n+int cmd_main(int argc, const char **argv) {\n+\tTEST_STRCMP_OFFSET(\"abc\", \"abc\", 0, 3);\n+\tTEST_STRCMP_OFFSET(\"abc\", \"def\", -1, 0);\n+\tTEST_STRCMP_OFFSET(\"abc\", \"abz\", -1, 2);\n+\tTEST_STRCMP_OFFSET(\"abc\", \"abcdef\", -1, 3);\n+\n+\treturn test_done();\n+}\n--\n2.43.0.windows.1\n\n"},{"id":"491568","messageId":"ZgK1fLIlfssCRiS0@tanuki","threadId":"61083","inReplyTo":"20240310144819.4379-1-ach.lumap@gmail.com","subject":"Re: [Outreachy][PATCH] Port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-03-26T11:46:04Z","receivedAt":"2024-03-26T11:46:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Mar 10, 2024 at 03:48:19PM +0100, Achu Luma wrote:\n> In the recent codebase update (8bf6fbd (Merge branch\n> 'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was\n> merged, providing a standardized approach for testing C code. Prior to\n> this update, some unit tests relied on the test helper mechanism,\n> lacking a dedicated unit testing framework. It's more natural to perform\n> these unit tests using the new unit test framework.\n> \n> Let's migrate the unit tests for strcmp-offset functionality from the\n> legacy approach using the test-tool command `test-tool strcmp-offset` in\n> helper/test-strcmp-offset.c to the new unit testing framework\n> (t/unit-tests/test-lib.h).\n> \n> The migration involves refactoring the tests to utilize the testing\n> macros provided by the framework (TEST() and check_*()).\n> \n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Signed-off-by: Achu Luma <ach.lumap@gmail.com>\n> ---\n>  Makefile                       |  2 +-\n>  t/helper/test-strcmp-offset.c  | 23 -----------------------\n>  t/helper/test-tool.c           |  1 -\n>  t/helper/test-tool.h           |  1 -\n>  t/t0065-strcmp-offset.sh       | 22 ----------------------\n>  t/unit-tests/t-strcmp-offset.c | 31 +++++++++++++++++++++++++++++++\n>  6 files changed, 32 insertions(+), 48 deletions(-)\n>  delete mode 100644 t/helper/test-strcmp-offset.c\n>  delete mode 100755 t/t0065-strcmp-offset.sh\n>  create mode 100644 t/unit-tests/t-strcmp-offset.c\n> \n> diff --git a/Makefile b/Makefile\n> index 4e255c81f2..b8d7019ad7 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -850,7 +850,6 @@ TEST_BUILTINS_OBJS += test-sha1.o\n>  TEST_BUILTINS_OBJS += test-sha256.o\n>  TEST_BUILTINS_OBJS += test-sigchain.o\n>  TEST_BUILTINS_OBJS += test-simple-ipc.o\n> -TEST_BUILTINS_OBJS += test-strcmp-offset.o\n>  TEST_BUILTINS_OBJS += test-string-list.o\n>  TEST_BUILTINS_OBJS += test-submodule-config.o\n>  TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n> @@ -1347,6 +1346,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n>  UNIT_TEST_PROGRAMS += t-strbuf\n>  UNIT_TEST_PROGRAMS += t-ctype\n>  UNIT_TEST_PROGRAMS += t-prio-queue\n> +UNIT_TEST_PROGRAMS += t-strcmp-offset\n>  UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n>  UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))\n>  UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o\n> diff --git a/t/helper/test-strcmp-offset.c b/t/helper/test-strcmp-offset.c\n> deleted file mode 100644\n> index d8473cf2fc..0000000000\n> --- a/t/helper/test-strcmp-offset.c\n> +++ /dev/null\n> @@ -1,23 +0,0 @@\n> -#include \"test-tool.h\"\n> -#include \"read-cache-ll.h\"\n> -\n> -int cmd__strcmp_offset(int argc UNUSED, const char **argv)\n> -{\n> -\tint result;\n> -\tsize_t offset;\n> -\n> -\tif (!argv[1] || !argv[2])\n> -\t\tdie(\"usage: %s <string1> <string2>\", argv[0]);\n> -\n> -\tresult = strcmp_offset(argv[1], argv[2], &offset);\n> -\n> -\t/*\n> -\t * Because different CRTs behave differently, only rely on signs\n> -\t * of the result values.\n> -\t */\n> -\tresult = (result < 0 ? -1 :\n> -\t\t\t  result > 0 ? 1 :\n> -\t\t\t  0);\n> -\tprintf(\"%d %\"PRIuMAX\"\\n\", result, (uintmax_t)offset);\n> -\treturn 0;\n> -}\n> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\n> index 482a1e58a4..3d56de82fd 100644\n> --- a/t/helper/test-tool.c\n> +++ b/t/helper/test-tool.c\n> @@ -76,7 +76,6 @@ static struct test_cmd cmds[] = {\n>  \t{ \"sha256\", cmd__sha256 },\n>  \t{ \"sigchain\", cmd__sigchain },\n>  \t{ \"simple-ipc\", cmd__simple_ipc },\n> -\t{ \"strcmp-offset\", cmd__strcmp_offset },\n>  \t{ \"string-list\", cmd__string_list },\n>  \t{ \"submodule\", cmd__submodule },\n>  \t{ \"submodule-config\", cmd__submodule_config },\n> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\n> index b1be7cfcf5..8d76a8c1e1 100644\n> --- a/t/helper/test-tool.h\n> +++ b/t/helper/test-tool.h\n> @@ -69,7 +69,6 @@ int cmd__oid_array(int argc, const char **argv);\n>  int cmd__sha256(int argc, const char **argv);\n>  int cmd__sigchain(int argc, const char **argv);\n>  int cmd__simple_ipc(int argc, const char **argv);\n> -int cmd__strcmp_offset(int argc, const char **argv);\n>  int cmd__string_list(int argc, const char **argv);\n>  int cmd__submodule(int argc, const char **argv);\n>  int cmd__submodule_config(int argc, const char **argv);\n> diff --git a/t/t0065-strcmp-offset.sh b/t/t0065-strcmp-offset.sh\n> deleted file mode 100755\n> index 94e34c83ed..0000000000\n> --- a/t/t0065-strcmp-offset.sh\n> +++ /dev/null\n> @@ -1,22 +0,0 @@\n> -#!/bin/sh\n> -\n> -test_description='Test strcmp_offset functionality'\n> -\n> -TEST_PASSES_SANITIZE_LEAK=true\n> -. ./test-lib.sh\n> -\n> -while read s1 s2 expect\n> -do\n> -\ttest_expect_success \"strcmp_offset($s1, $s2)\" '\n> -\t\techo \"$expect\" >expect &&\n> -\t\ttest-tool strcmp-offset \"$s1\" \"$s2\" >actual &&\n> -\t\ttest_cmp expect actual\n> -\t'\n> -done <<-EOF\n> -abc abc 0 3\n> -abc def -1 0\n> -abc abz -1 2\n> -abc abcdef -1 3\n> -EOF\n> -\n> -test_done\n> diff --git a/t/unit-tests/t-strcmp-offset.c b/t/unit-tests/t-strcmp-offset.c\n> new file mode 100644\n> index 0000000000..176d2ed04a\n> --- /dev/null\n> +++ b/t/unit-tests/t-strcmp-offset.c\n> @@ -0,0 +1,31 @@\n> +#include \"test-lib.h\"\n> +#include \"read-cache-ll.h\"\n> +\n> +static void check_strcmp_offset(const char *string1, const char *string2, int expect_result,  uintmax_t expect_offset)\n\nTiny nit: there's two spaces in front of `uintmax_t expect_offset`.\n\n> +{\n> +\tint result;\n> +\tsize_t offset;\n> +\n> +\tresult = strcmp_offset(string1, string2, &offset);\n> +\n> +\t/* Because different CRTs behave differently, only rely on signs of the result values. */\n> +\tresult = (result < 0 ? -1 :\n> +\t\t\t  result > 0 ? 1 :\n> +\t\t\t  0);\n\nI was wondering a bit why we munge the result like this and don't\ncompare that it's an exact match. But this isn't much of an isuse in my\nopinion given that this is just a straight port of the old code.\n\nOther than that this patch looks good to me, thanks!\n\nPatrick\n\n> +\tcheck_int(result, ==, expect_result);\n> +\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n> +}\n> +\n> +#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \\\n> +\t\tTEST(check_strcmp_offset(string1, string2, expect_result, expect_offset), \\\n> +\t\t\t\"strcmp_offset(%s, %s) works\", #string1, #string2)\n> +\n> +int cmd_main(int argc, const char **argv) {\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"abc\", 0, 3);\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"def\", -1, 0);\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"abz\", -1, 2);\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"abcdef\", -1, 3);\n> +\n> +\treturn test_done();\n> +}\n> --\n> 2.43.0.windows.1\n> \n> \n"},{"id":"495063","messageId":"20240519204530.12258-3-shyamthakkar001@gmail.com","threadId":"61083","inReplyTo":"20240310144819.4379-1-ach.lumap@gmail.com","subject":"[GSoC][PATCH v2] t/: port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c","fromName":"Ghanshyam Thakkar","fromEmail":"shyamthakkar001@gmail.com","sentAt":"2024-05-19T20:44:42Z","receivedAt":"2024-05-19T20:46:45Z","isPatch":true,"sender":{"key":"shyamthakkar001@gmail.com","avatar":"https://avatars.githubusercontent.com/u/72698233?v=4"},"body":"In the recent codebase update (8bf6fbd (Merge branch\n'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was\nmerged, providing a standardized approach for testing C code. Prior to\nthis update, some unit tests relied on the test helper mechanism,\nlacking a dedicated unit testing framework. It's more natural to perform\nthese unit tests using the new unit test framework.\n\nLet's migrate the unit tests for strcmp-offset functionality from the\nlegacy approach using the test-tool command `test-tool strcmp-offset` in\nhelper/test-strcmp-offset.c to the new unit testing framework\n(t/unit-tests/test-lib.h).\n\nThe migration involves refactoring the tests to utilize the testing\nmacros provided by the framework (TEST() and check_*()).\n\nHelped-by: Patrick Steinhardt <ps@pks.im>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nMentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\nCo-authored-by: Achu Luma <ach.lumap@gmail.com>\nSigned-off-by: Achu Luma <ach.lumap@gmail.com>\nSigned-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n---\nThe v2 only adjusts the formatting to be in line with the style\ndescribed in CodingGuidelines. And it is also rebased on 'next' to\navoid Makefile conflicts.\n\nCI for v2: https://github.com/spectre10/git/actions/runs/9150288967\n\nRange-diff against v1:\n1:  47e7df1d22 ! 1:  3c2a6f7e9b Port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n    @@\n      ## Metadata ##\n    -Author: Achu Luma <ach.lumap@gmail.com>\n    +Author: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n     \n      ## Commit message ##\n    -    Port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n    +    t/: port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c\n     \n         In the recent codebase update (8bf6fbd (Merge branch\n         'js/doc-unit-tests', 2023-12-09)), a new unit testing framework was\n    @@ Commit message\n         The migration involves refactoring the tests to utilize the testing\n         macros provided by the framework (TEST() and check_*()).\n     \n    +    Helped-by: Patrick Steinhardt <ps@pks.im>\n         Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n    +    Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n    +    Co-authored-by: Achu Luma <ach.lumap@gmail.com>\n         Signed-off-by: Achu Luma <ach.lumap@gmail.com>\n    +    Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n     \n      ## Makefile ##\n     @@ Makefile: TEST_BUILTINS_OBJS += test-sha1.o\n    @@ Makefile: TEST_BUILTINS_OBJS += test-sha1.o\n      TEST_BUILTINS_OBJS += test-string-list.o\n      TEST_BUILTINS_OBJS += test-submodule-config.o\n      TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n    -@@ Makefile: UNIT_TEST_PROGRAMS += t-mem-pool\n    - UNIT_TEST_PROGRAMS += t-strbuf\n    - UNIT_TEST_PROGRAMS += t-ctype\n    +@@ Makefile: UNIT_TEST_PROGRAMS += t-ctype\n    + UNIT_TEST_PROGRAMS += t-mem-pool\n      UNIT_TEST_PROGRAMS += t-prio-queue\n    + UNIT_TEST_PROGRAMS += t-strbuf\n     +UNIT_TEST_PROGRAMS += t-strcmp-offset\n    + UNIT_TEST_PROGRAMS += t-trailer\n      UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n      UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))\n    - UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o\n     \n      ## t/helper/test-strcmp-offset.c (deleted) ##\n     @@\n    @@ t/unit-tests/t-strcmp-offset.c (new)\n     +#include \"test-lib.h\"\n     +#include \"read-cache-ll.h\"\n     +\n    -+static void check_strcmp_offset(const char *string1, const char *string2, int expect_result,  uintmax_t expect_offset)\n    ++static void check_strcmp_offset(const char *string1, const char *string2,\n    ++\t\t\t\tint expect_result, uintmax_t expect_offset)\n     +{\n    -+\tint result;\n     +\tsize_t offset;\n    ++\tint result = strcmp_offset(string1, string2, &offset);\n     +\n    -+\tresult = strcmp_offset(string1, string2, &offset);\n    -+\n    -+\t/* Because different CRTs behave differently, only rely on signs of the result values. */\n    ++\t/*\n    ++\t * Because different CRTs behave differently, only rely on signs of the\n    ++\t * result values.\n    ++\t */\n     +\tresult = (result < 0 ? -1 :\n    -+\t\t\t  result > 0 ? 1 :\n    -+\t\t\t  0);\n    ++\t\t\tresult > 0 ? 1 :\n    ++\t\t\t0);\n     +\n     +\tcheck_int(result, ==, expect_result);\n     +\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n     +}\n     +\n     +#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \\\n    -+\t\tTEST(check_strcmp_offset(string1, string2, expect_result, expect_offset), \\\n    -+\t\t\t\"strcmp_offset(%s, %s) works\", #string1, #string2)\n    ++\tTEST(check_strcmp_offset(string1, string2, expect_result,          \\\n    ++\t\t\t\t expect_offset),                           \\\n    ++\t     \"strcmp_offset(%s, %s) works\", #string1, #string2)\n     +\n    -+int cmd_main(int argc, const char **argv) {\n    ++int cmd_main(int argc, const char **argv)\n    ++{\n     +\tTEST_STRCMP_OFFSET(\"abc\", \"abc\", 0, 3);\n     +\tTEST_STRCMP_OFFSET(\"abc\", \"def\", -1, 0);\n     +\tTEST_STRCMP_OFFSET(\"abc\", \"abz\", -1, 2);\n\n Makefile                       |  2 +-\n t/helper/test-strcmp-offset.c  | 23 ----------------------\n t/helper/test-tool.c           |  1 -\n t/helper/test-tool.h           |  1 -\n t/t0065-strcmp-offset.sh       | 22 ---------------------\n t/unit-tests/t-strcmp-offset.c | 35 ++++++++++++++++++++++++++++++++++\n 6 files changed, 36 insertions(+), 48 deletions(-)\n delete mode 100644 t/helper/test-strcmp-offset.c\n delete mode 100755 t/t0065-strcmp-offset.sh\n create mode 100644 t/unit-tests/t-strcmp-offset.c\n\ndiff --git a/Makefile b/Makefile\nindex 8f4432ae57..59d98ba688 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -839,7 +839,6 @@ TEST_BUILTINS_OBJS += test-sha1.o\n TEST_BUILTINS_OBJS += test-sha256.o\n TEST_BUILTINS_OBJS += test-sigchain.o\n TEST_BUILTINS_OBJS += test-simple-ipc.o\n-TEST_BUILTINS_OBJS += test-strcmp-offset.o\n TEST_BUILTINS_OBJS += test-string-list.o\n TEST_BUILTINS_OBJS += test-submodule-config.o\n TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n@@ -1338,6 +1337,7 @@ UNIT_TEST_PROGRAMS += t-ctype\n UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-prio-queue\n UNIT_TEST_PROGRAMS += t-strbuf\n+UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGRAMS += t-trailer\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))\ndiff --git a/t/helper/test-strcmp-offset.c b/t/helper/test-strcmp-offset.c\ndeleted file mode 100644\nindex d8473cf2fc..0000000000\n--- a/t/helper/test-strcmp-offset.c\n+++ /dev/null\n@@ -1,23 +0,0 @@\n-#include \"test-tool.h\"\n-#include \"read-cache-ll.h\"\n-\n-int cmd__strcmp_offset(int argc UNUSED, const char **argv)\n-{\n-\tint result;\n-\tsize_t offset;\n-\n-\tif (!argv[1] || !argv[2])\n-\t\tdie(\"usage: %s <string1> <string2>\", argv[0]);\n-\n-\tresult = strcmp_offset(argv[1], argv[2], &offset);\n-\n-\t/*\n-\t * Because different CRTs behave differently, only rely on signs\n-\t * of the result values.\n-\t */\n-\tresult = (result < 0 ? -1 :\n-\t\t\t  result > 0 ? 1 :\n-\t\t\t  0);\n-\tprintf(\"%d %\"PRIuMAX\"\\n\", result, (uintmax_t)offset);\n-\treturn 0;\n-}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex f6fd0fe491..7ad7d07018 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -78,7 +78,6 @@ static struct test_cmd cmds[] = {\n \t{ \"sha256\", cmd__sha256 },\n \t{ \"sigchain\", cmd__sigchain },\n \t{ \"simple-ipc\", cmd__simple_ipc },\n-\t{ \"strcmp-offset\", cmd__strcmp_offset },\n \t{ \"string-list\", cmd__string_list },\n \t{ \"submodule\", cmd__submodule },\n \t{ \"submodule-config\", cmd__submodule_config },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 868f33453c..d14b3072bd 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -71,7 +71,6 @@ int cmd__oid_array(int argc, const char **argv);\n int cmd__sha256(int argc, const char **argv);\n int cmd__sigchain(int argc, const char **argv);\n int cmd__simple_ipc(int argc, const char **argv);\n-int cmd__strcmp_offset(int argc, const char **argv);\n int cmd__string_list(int argc, const char **argv);\n int cmd__submodule(int argc, const char **argv);\n int cmd__submodule_config(int argc, const char **argv);\ndiff --git a/t/t0065-strcmp-offset.sh b/t/t0065-strcmp-offset.sh\ndeleted file mode 100755\nindex 94e34c83ed..0000000000\n--- a/t/t0065-strcmp-offset.sh\n+++ /dev/null\n@@ -1,22 +0,0 @@\n-#!/bin/sh\n-\n-test_description='Test strcmp_offset functionality'\n-\n-TEST_PASSES_SANITIZE_LEAK=true\n-. ./test-lib.sh\n-\n-while read s1 s2 expect\n-do\n-\ttest_expect_success \"strcmp_offset($s1, $s2)\" '\n-\t\techo \"$expect\" >expect &&\n-\t\ttest-tool strcmp-offset \"$s1\" \"$s2\" >actual &&\n-\t\ttest_cmp expect actual\n-\t'\n-done <<-EOF\n-abc abc 0 3\n-abc def -1 0\n-abc abz -1 2\n-abc abcdef -1 3\n-EOF\n-\n-test_done\ndiff --git a/t/unit-tests/t-strcmp-offset.c b/t/unit-tests/t-strcmp-offset.c\nnew file mode 100644\nindex 0000000000..fe4c2706b1\n--- /dev/null\n+++ b/t/unit-tests/t-strcmp-offset.c\n@@ -0,0 +1,35 @@\n+#include \"test-lib.h\"\n+#include \"read-cache-ll.h\"\n+\n+static void check_strcmp_offset(const char *string1, const char *string2,\n+\t\t\t\tint expect_result, uintmax_t expect_offset)\n+{\n+\tsize_t offset;\n+\tint result = strcmp_offset(string1, string2, &offset);\n+\n+\t/*\n+\t * Because different CRTs behave differently, only rely on signs of the\n+\t * result values.\n+\t */\n+\tresult = (result < 0 ? -1 :\n+\t\t\tresult > 0 ? 1 :\n+\t\t\t0);\n+\n+\tcheck_int(result, ==, expect_result);\n+\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n+}\n+\n+#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \\\n+\tTEST(check_strcmp_offset(string1, string2, expect_result,          \\\n+\t\t\t\t expect_offset),                           \\\n+\t     \"strcmp_offset(%s, %s) works\", #string1, #string2)\n+\n+int cmd_main(int argc, const char **argv)\n+{\n+\tTEST_STRCMP_OFFSET(\"abc\", \"abc\", 0, 3);\n+\tTEST_STRCMP_OFFSET(\"abc\", \"def\", -1, 0);\n+\tTEST_STRCMP_OFFSET(\"abc\", \"abz\", -1, 2);\n+\tTEST_STRCMP_OFFSET(\"abc\", \"abcdef\", -1, 3);\n+\n+\treturn test_done();\n+}\n-- \n2.45.1\n\n"},{"id":"495084","messageId":"xmqq1q5wmpqh.fsf@gitster.g","threadId":"61083","inReplyTo":"20240519204530.12258-3-shyamthakkar001@gmail.com","subject":"Re: [GSoC][PATCH v2] t/: port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-20T16:07:18Z","receivedAt":"2024-05-20T16:07:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:\n\n> The v2 only adjusts the formatting to be in line with the style\n> described in CodingGuidelines. And it is also rebased on 'next' to\n> avoid Makefile conflicts.\n\nPlease do not base a new topic on 'next', as I will NOT be applying\nit on top of 'next'.\n\nImagine what it takes for a topic to graduate to 'master', if it\nstarted from the tip of 'next'?  You have to wait for ALL other\ntopics that are in 'next' to graduate.\n\nInstead, new things are to be built on 'master' and fixes are to be\nbuilt on the oldest maintenance track the fix is relevant.\n\nAfter building such a base, you can and should make a trial merge of\nyour topic into 'next' and 'seen' to see if your work interferes\nwith work by somebody else, which often gives you a good learning\nopportunity.\n\nUnless the conflicts are severe and is impractical, in which case\nsee Documentation/SubmittingPatches and look for \"Under truly\nexceptional circumstances\".  But the conflict in Makefile about\nUNIT_TEST_PROGRAMS in this case hadly qualifies as one.\n\nAnyway, thanks for a patch.\n\nNow for the patch text itself.\n\nThe original looked like this.\n\n> index d8473cf2fc..0000000000\n> --- a/t/helper/test-strcmp-offset.c\n> +++ /dev/null\n> @@ -1,23 +0,0 @@\n> -#include \"test-tool.h\"\n> -#include \"read-cache-ll.h\"\n> -\n> -int cmd__strcmp_offset(int argc UNUSED, const char **argv)\n> -{\n> -\tint result;\n> -\tsize_t offset;\n> -\n> -\tif (!argv[1] || !argv[2])\n> -\t\tdie(\"usage: %s <string1> <string2>\", argv[0]);\n> -\n> -\tresult = strcmp_offset(argv[1], argv[2], &offset);\n> -\n> -\t/*\n> -\t * Because different CRTs behave differently, only rely on signs\n> -\t * of the result values.\n> -\t */\n> -\tresult = (result < 0 ? -1 :\n> -\t\t\t  result > 0 ? 1 :\n> -\t\t\t  0);\n> -\tprintf(\"%d %\"PRIuMAX\"\\n\", result, (uintmax_t)offset);\n> -\treturn 0;\n> -}\n\nIt used to print the result and the discovered offset to the\nstandard output, which is used in the comparison of the calling test\nscript.  Now we are doing the check ourselves here, that part needs\nto be different.  But other than that, there shouldn't be any change.\n\n> +static void check_strcmp_offset(const char *string1, const char *string2,\n> +\t\t\t\tint expect_result, uintmax_t expect_offset)\n> +{\n> +\tsize_t offset;\n> +\tint result = strcmp_offset(string1, string2, &offset);\n> +\n> +\t/*\n> +\t * Because different CRTs behave differently, only rely on signs of the\n> +\t * result values.\n> +\t */\n> +\tresult = (result < 0 ? -1 :\n> +\t\t\tresult > 0 ? 1 :\n> +\t\t\t0);\n> +\n> +\tcheck_int(result, ==, expect_result);\n> +\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n> +}\n\nSubtle differences that do not seem to have any good reason to be\nthere relative to the original, namely, the order of declarations of\ntwo local variables, how 'result' is initialized, how the exact same\ncomment is line-wrapped differently.  If there weren't such changes,\nthe output from \"git show --color-moved\" would have allowed readers'\neyes coast over them, but because of them, they need to read most of\nthe lines.  That's an inefficient use of their time.\n\nThe test used to be\n\n> -while read s1 s2 expect\n> -do\n> -\ttest_expect_success \"strcmp_offset($s1, $s2)\" '\n> -\t\techo \"$expect\" >expect &&\n> -\t\ttest-tool strcmp-offset \"$s1\" \"$s2\" >actual &&\n> -\t\ttest_cmp expect actual\n> -\t'\n> -done <<-EOF\n> -abc abc 0 3\n> -abc def -1 0\n> -abc abz -1 2\n> -abc abcdef -1 3\n> -EOF\n\nso a misbehaving \"strcmp-offset abc abc\" that gives different result\ncan be seen by showing say\n\n    -abc abc 0 3\n    +abc abc -1 2\n\nif it incorrectly say that the first difference is at the offset 2\nand reported that the first \"abc\" sorts before the second \"abc\".\n\nWith the new code, how would the failing test look like?\n\n> +#define TEST_STRCMP_OFFSET(string1, string2, expect_result, expect_offset) \\\n> +\tTEST(check_strcmp_offset(string1, string2, expect_result,          \\\n> +\t\t\t\t expect_offset),                           \\\n> +\t     \"strcmp_offset(%s, %s) works\", #string1, #string2)\n\nThat's a neat way to use the #stringification.  Without it the\nmessage will say\n\n\tstrcmp_offset(abc, abc) works\n\nbut it is properly C-quoted, i.e.\n\n\tstrcmp_offset(\"abc\", \"abc\") works\n\nIf the second string were \"abc\\t\", then the benefit of using\n#string2  would become even more prominent.\n\nHaving said that, output from overly generic:\n\n> +\tcheck_int(result, ==, expect_result);\n> +\tcheck_uint((uintmax_t)offset, ==, expect_offset);\n\nused in the check_strcmp_offset() function is not all that readable,\nespecially given that it comes _before_ the test title.  It shows\nsomething like\n\n    # check \"result == expect_result\" failed\n    #    left: -1\n    #   right: 0\n\nHopefully it would automatically improve as the framework gets\nimproved, perhaps to say\n\n    # check \"result == expect_result\" failed\n    # result = -1, expect_result = 0\n\nIn any case, it is a general problem of the current unit test\nframework and outside the scope of this patch.\n\n> +int cmd_main(int argc, const char **argv)\nn> +{\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"abc\", 0, 3);\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"def\", -1, 0);\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"abz\", -1, 2);\n> +\tTEST_STRCMP_OFFSET(\"abc\", \"abcdef\", -1, 3);\n> +\n> +\treturn test_done();\n> +}\n\nLooking good.\n\n"},{"id":"495117","messageId":"xmqqseycdxe9.fsf@gitster.g","threadId":"61083","inReplyTo":"xmqq1q5wmpqh.fsf@gitster.g","subject":"Re: [GSoC][PATCH v2] t/: port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-20T20:46:38Z","receivedAt":"2024-05-20T20:46:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Please do not base a new topic on 'next', as I will NOT be applying\n> it on top of 'next'.\n> ...\n> Unless the conflicts are severe and is impractical, in which case\n> see Documentation/SubmittingPatches and look for \"Under truly\n> exceptional circumstances\".  But the conflict in Makefile about\n> UNIT_TEST_PROGRAMS in this case hadly qualifies as one.\n>\n> Anyway, thanks for a patch.\n\nI've backported the patch to apply to \"master\" and queued it on its\nown topic, so that it no longer has to wait for all other topic in\n'next'.  The Makefile looks like the attached, which is just with\ntrivial difference in the context.  We only need to remove\nstrcmp-offset from the TEST_BUILTIN_OBJS and instead add a\ncorresponding one to UNIT_TEST_PROGRAMS, and that does not change no\nmatter what other test-*.o are added to the former or t-* are added\nto the latter.\n\nWe may want to sort the UNIT_TEST_PROGRAMS list alphabetically at\nsome point, by the way.\n\nThanks.\n\ndiff --git a/Makefile b/Makefile\nindex cf504963c2..1afa112706 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -839,7 +839,6 @@ TEST_BUILTINS_OBJS += test-sha1.o\n TEST_BUILTINS_OBJS += test-sha256.o\n TEST_BUILTINS_OBJS += test-sigchain.o\n TEST_BUILTINS_OBJS += test-simple-ipc.o\n-TEST_BUILTINS_OBJS += test-strcmp-offset.o\n TEST_BUILTINS_OBJS += test-string-list.o\n TEST_BUILTINS_OBJS += test-submodule-config.o\n TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n@@ -1338,6 +1337,7 @@ UNIT_TEST_PROGRAMS += t-mem-pool\n UNIT_TEST_PROGRAMS += t-strbuf\n UNIT_TEST_PROGRAMS += t-ctype\n UNIT_TEST_PROGRAMS += t-prio-queue\n+UNIT_TEST_PROGRAMS += t-strcmp-offset\n UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))\n UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o\n"},{"id":"495118","messageId":"7o3cyisx3suyqhe24xkfbraaxq4vzgy5er2tbshbtgfinkugjk@gmjtg7bhudwo","threadId":"61083","inReplyTo":"xmqqseycdxe9.fsf@gitster.g","subject":"Re: [GSoC][PATCH v2] t/: port helper/test-strcmp-offset.c to unit-tests/t-strcmp-offset.c","fromName":"Ghanshyam Thakkar","fromEmail":"shyamthakkar001@gmail.com","sentAt":"2024-05-20T20:55:26Z","receivedAt":"2024-05-20T20:55:29Z","isPatch":true,"sender":{"key":"shyamthakkar001@gmail.com","avatar":"https://avatars.githubusercontent.com/u/72698233?v=4"},"body":"On Mon, 20 May 2024, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Please do not base a new topic on 'next', as I will NOT be applying\n> > it on top of 'next'.\n> > ...\n> > Unless the conflicts are severe and is impractical, in which case\n> > see Documentation/SubmittingPatches and look for \"Under truly\n> > exceptional circumstances\".  But the conflict in Makefile about\n> > UNIT_TEST_PROGRAMS in this case hadly qualifies as one.\n\nWill make sure for the next time.\n\n> > Anyway, thanks for a patch.\n> \n> I've backported the patch to apply to \"master\" and queued it on its\n> own topic, so that it no longer has to wait for all other topic in\n> 'next'.  The Makefile looks like the attached, which is just with\n> trivial difference in the context.  We only need to remove\n> strcmp-offset from the TEST_BUILTIN_OBJS and instead add a\n> corresponding one to UNIT_TEST_PROGRAMS, and that does not change no\n> matter what other test-*.o are added to the former or t-* are added\n> to the latter.\n> \n> We may want to sort the UNIT_TEST_PROGRAMS list alphabetically at\n> some point, by the way.\n\nThat was aleady done in 'la/hide-trailer-info' (next).\n\nThanks.\n"}]}