{"thread":{"id":"61722","subject":"[GSoC][PATCH v3] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c","startedAt":"2024-07-03T06:31:18Z","lastAt":"2024-07-04T09:44:29Z","messageCount":4,"participants":["Ghanshyam Thakkar","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"498015","messageId":"20240703062958.23262-2-shyamthakkar001@gmail.com","threadId":"61722","inReplyTo":null,"subject":"[GSoC][PATCH v3] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c","fromName":"Ghanshyam Thakkar","fromEmail":"shyamthakkar001@gmail.com","sentAt":"2024-07-03T06:29:53Z","receivedAt":"2024-07-03T06:31:18Z","isPatch":true,"sender":{"key":"shyamthakkar001@gmail.com","avatar":"https://avatars.githubusercontent.com/u/72698233?v=4"},"body":"helper/test-oidmap.c along with t0016-oidmap.sh test the oidmap.h\nlibrary which is built on top of hashmap.h.\n\nMigrate them to the unit testing framework for better performance,\nconcise code and better debugging. Along with the migration also plug\nmemory leaks and make the test logic independent for all the tests.\nThe migration removes 'put' tests from t0016, because it is used as\nsetup to all the other tests, so testing it separately does not yield\nany benefit.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nHelped-by: Phillip Wood <phillip.wood123@gmail.com>\nReviewed-by: Josh Steadmon <steadmon@google.com>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nMentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\nSigned-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n---\nChanges in v3:\n- use 'count' to check if we iterated over all the entries (Phillip's\n  suggestion)\n- use 'seen' array instead of modifying the global array (Junio's\n  review)\n\nRange-diff against v2:\n1:  cc0c4c3b0a ! 1:  bdb3c8ebe4 t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c\n    @@ Commit message\n         setup to all the other tests, so testing it separately does not yield\n         any benefit.\n     \n    +    Helped-by: Junio C Hamano <gitster@pobox.com>\n         Helped-by: Phillip Wood <phillip.wood123@gmail.com>\n         Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n         Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n    @@ t/unit-tests/t-oidmap.c (new)\n     +\tchar name[FLEX_ARRAY];\n     +};\n     +\n    -+static const char *key_val[][2] = { { \"11\", \"one\" },\n    -+\t\t\t\t    { \"22\", \"two\" },\n    -+\t\t\t\t    { \"33\", \"three\" } };\n    ++static const char *const key_val[][2] = { { \"11\", \"one\" },\n    ++\t\t\t\t\t  { \"22\", \"two\" },\n    ++\t\t\t\t\t  { \"33\", \"three\" } };\n     +\n     +static void setup(void (*f)(struct oidmap *map))\n     +{\n    @@ t/unit-tests/t-oidmap.c (new)\n     +\tcheck(oidmap_remove(map, &oid) == NULL);\n     +}\n     +\n    -+static int key_val_contains(struct test_entry *entry)\n    ++static int key_val_contains(struct test_entry *entry, char seen[])\n     +{\n     +\tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n     +\t\tstruct object_id oid;\n    @@ t/unit-tests/t-oidmap.c (new)\n     +\t\t\treturn -1;\n     +\n     +\t\tif (oideq(&entry->entry.oid, &oid)) {\n    -+\t\t\tif (!strcmp(key_val[i][1], \"USED\"))\n    ++\t\t\tif (seen[i])\n     +\t\t\t\treturn 2;\n    -+\t\t\tkey_val[i][1] = \"USED\";\n    ++\t\t\tseen[i] = 1;\n     +\t\t\treturn 0;\n     +\t\t}\n     +\t}\n    @@ t/unit-tests/t-oidmap.c (new)\n     +{\n     +\tstruct oidmap_iter iter;\n     +\tstruct test_entry *entry;\n    ++\tchar seen[ARRAY_SIZE(key_val)] = { 0 };\n    ++\tint count = 0;\n     +\n     +\toidmap_iter_init(map, &iter);\n     +\twhile ((entry = oidmap_iter_next(&iter))) {\n     +\t\tint ret;\n    -+\t\tif (!check_int((ret = key_val_contains(entry)), ==, 0)) {\n    ++\t\tif (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n     +\t\t\tswitch (ret) {\n     +\t\t\tcase -1:\n     +\t\t\t\tbreak; /* error message handled by get_oid_arbitrary_hex() */\n    @@ t/unit-tests/t-oidmap.c (new)\n     +\t\t\t\t\t ret);\n     +\t\t\t\tbreak;\n     +\t\t\t}\n    ++\t\t} else {\n    ++\t\t\tcount++;\n     +\t\t}\n     +\t}\n    ++\tcheck_int(count, ==, ARRAY_SIZE(key_val));\n     +\tcheck_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));\n     +}\n     +\n\n Makefile                |   2 +-\n t/helper/test-oidmap.c  | 123 ---------------------------\n t/helper/test-tool.c    |   1 -\n t/helper/test-tool.h    |   1 -\n t/t0016-oidmap.sh       | 112 -------------------------\n t/unit-tests/t-oidmap.c | 181 ++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 182 insertions(+), 238 deletions(-)\n delete mode 100644 t/helper/test-oidmap.c\n delete mode 100755 t/t0016-oidmap.sh\n create mode 100644 t/unit-tests/t-oidmap.c\n\ndiff --git a/Makefile b/Makefile\nindex 3eab701b10..2a5c70d218 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -809,7 +809,6 @@ TEST_BUILTINS_OBJS += test-match-trees.o\n TEST_BUILTINS_OBJS += test-mergesort.o\n TEST_BUILTINS_OBJS += test-mktemp.o\n TEST_BUILTINS_OBJS += test-oid-array.o\n-TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-pack-mtimes.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n@@ -1337,6 +1336,7 @@ UNIT_TEST_PROGRAMS += t-ctype\n UNIT_TEST_PROGRAMS += t-example-decorate\n UNIT_TEST_PROGRAMS += t-hash\n UNIT_TEST_PROGRAMS += t-mem-pool\n+UNIT_TEST_PROGRAMS += t-oidmap\n UNIT_TEST_PROGRAMS += t-oidtree\n UNIT_TEST_PROGRAMS += t-prio-queue\n UNIT_TEST_PROGRAMS += t-reftable-basics\ndiff --git a/t/helper/test-oidmap.c b/t/helper/test-oidmap.c\ndeleted file mode 100644\nindex bd30244a54..0000000000\n--- a/t/helper/test-oidmap.c\n+++ /dev/null\n@@ -1,123 +0,0 @@\n-#include \"test-tool.h\"\n-#include \"hex.h\"\n-#include \"object-name.h\"\n-#include \"oidmap.h\"\n-#include \"repository.h\"\n-#include \"setup.h\"\n-#include \"strbuf.h\"\n-#include \"string-list.h\"\n-\n-/* key is an oid and value is a name (could be a refname for example) */\n-struct test_entry {\n-\tstruct oidmap_entry entry;\n-\tchar name[FLEX_ARRAY];\n-};\n-\n-#define DELIM \" \\t\\r\\n\"\n-\n-/*\n- * Read stdin line by line and print result of commands to stdout:\n- *\n- * hash oidkey -> sha1hash(oidkey)\n- * put oidkey namevalue -> NULL / old namevalue\n- * get oidkey -> NULL / namevalue\n- * remove oidkey -> NULL / old namevalue\n- * iterate -> oidkey1 namevalue1\\noidkey2 namevalue2\\n...\n- *\n- */\n-int cmd__oidmap(int argc UNUSED, const char **argv UNUSED)\n-{\n-\tstruct string_list parts = STRING_LIST_INIT_NODUP;\n-\tstruct strbuf line = STRBUF_INIT;\n-\tstruct oidmap map = OIDMAP_INIT;\n-\n-\tsetup_git_directory();\n-\n-\t/* init oidmap */\n-\toidmap_init(&map, 0);\n-\n-\t/* process commands from stdin */\n-\twhile (strbuf_getline(&line, stdin) != EOF) {\n-\t\tchar *cmd, *p1, *p2;\n-\t\tstruct test_entry *entry;\n-\t\tstruct object_id oid;\n-\n-\t\t/* break line into command and up to two parameters */\n-\t\tstring_list_setlen(&parts, 0);\n-\t\tstring_list_split_in_place(&parts, line.buf, DELIM, 2);\n-\t\tstring_list_remove_empty_items(&parts, 0);\n-\n-\t\t/* ignore empty lines */\n-\t\tif (!parts.nr)\n-\t\t\tcontinue;\n-\t\tif (!*parts.items[0].string || *parts.items[0].string == '#')\n-\t\t\tcontinue;\n-\n-\t\tcmd = parts.items[0].string;\n-\t\tp1 = parts.nr >= 1 ? parts.items[1].string : NULL;\n-\t\tp2 = parts.nr >= 2 ? parts.items[2].string : NULL;\n-\n-\t\tif (!strcmp(\"put\", cmd) && p1 && p2) {\n-\n-\t\t\tif (repo_get_oid(the_repository, p1, &oid)) {\n-\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\n-\t\t\t/* create entry with oid_key = p1, name_value = p2 */\n-\t\t\tFLEX_ALLOC_STR(entry, name, p2);\n-\t\t\toidcpy(&entry->entry.oid, &oid);\n-\n-\t\t\t/* add / replace entry */\n-\t\t\tentry = oidmap_put(&map, entry);\n-\n-\t\t\t/* print and free replaced entry, if any */\n-\t\t\tputs(entry ? entry->name : \"NULL\");\n-\t\t\tfree(entry);\n-\n-\t\t} else if (!strcmp(\"get\", cmd) && p1) {\n-\n-\t\t\tif (repo_get_oid(the_repository, p1, &oid)) {\n-\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\n-\t\t\t/* lookup entry in oidmap */\n-\t\t\tentry = oidmap_get(&map, &oid);\n-\n-\t\t\t/* print result */\n-\t\t\tputs(entry ? entry->name : \"NULL\");\n-\n-\t\t} else if (!strcmp(\"remove\", cmd) && p1) {\n-\n-\t\t\tif (repo_get_oid(the_repository, p1, &oid)) {\n-\t\t\t\tprintf(\"Unknown oid: %s\\n\", p1);\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\n-\t\t\t/* remove entry from oidmap */\n-\t\t\tentry = oidmap_remove(&map, &oid);\n-\n-\t\t\t/* print result and free entry*/\n-\t\t\tputs(entry ? entry->name : \"NULL\");\n-\t\t\tfree(entry);\n-\n-\t\t} else if (!strcmp(\"iterate\", cmd)) {\n-\n-\t\t\tstruct oidmap_iter iter;\n-\t\t\toidmap_iter_init(&map, &iter);\n-\t\t\twhile ((entry = oidmap_iter_next(&iter)))\n-\t\t\t\tprintf(\"%s %s\\n\", oid_to_hex(&entry->entry.oid), entry->name);\n-\n-\t\t} else {\n-\n-\t\t\tprintf(\"Unknown command %s\\n\", cmd);\n-\n-\t\t}\n-\t}\n-\n-\tstring_list_clear(&parts, 0);\n-\tstrbuf_release(&line);\n-\toidmap_free(&map, 1);\n-\treturn 0;\n-}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 93436a82ae..da3e69128a 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -44,7 +44,6 @@ static struct test_cmd cmds[] = {\n \t{ \"mergesort\", cmd__mergesort },\n \t{ \"mktemp\", cmd__mktemp },\n \t{ \"oid-array\", cmd__oid_array },\n-\t{ \"oidmap\", cmd__oidmap },\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"pack-mtimes\", cmd__pack_mtimes },\n \t{ \"parse-options\", cmd__parse_options },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex d9033d14e1..642a34578c 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -37,7 +37,6 @@ int cmd__lazy_init_name_hash(int argc, const char **argv);\n int cmd__match_trees(int argc, const char **argv);\n int cmd__mergesort(int argc, const char **argv);\n int cmd__mktemp(int argc, const char **argv);\n-int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__pack_mtimes(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\ndiff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh\ndeleted file mode 100755\nindex 0faef1f4f1..0000000000\n--- a/t/t0016-oidmap.sh\n+++ /dev/null\n@@ -1,112 +0,0 @@\n-#!/bin/sh\n-\n-test_description='test oidmap'\n-\n-TEST_PASSES_SANITIZE_LEAK=true\n-. ./test-lib.sh\n-\n-# This purposefully is very similar to t0011-hashmap.sh\n-\n-test_oidmap () {\n-\techo \"$1\" | test-tool oidmap $3 >actual &&\n-\techo \"$2\" >expect &&\n-\ttest_cmp expect actual\n-}\n-\n-\n-test_expect_success 'setup' '\n-\n-\ttest_commit one &&\n-\ttest_commit two &&\n-\ttest_commit three &&\n-\ttest_commit four\n-\n-'\n-\n-test_expect_success 'put' '\n-\n-test_oidmap \"put one 1\n-put two 2\n-put invalidOid 4\n-put three 3\" \"NULL\n-NULL\n-Unknown oid: invalidOid\n-NULL\"\n-\n-'\n-\n-test_expect_success 'replace' '\n-\n-test_oidmap \"put one 1\n-put two 2\n-put three 3\n-put invalidOid 4\n-put two deux\n-put one un\" \"NULL\n-NULL\n-NULL\n-Unknown oid: invalidOid\n-2\n-1\"\n-\n-'\n-\n-test_expect_success 'get' '\n-\n-test_oidmap \"put one 1\n-put two 2\n-put three 3\n-get two\n-get four\n-get invalidOid\n-get one\" \"NULL\n-NULL\n-NULL\n-2\n-NULL\n-Unknown oid: invalidOid\n-1\"\n-\n-'\n-\n-test_expect_success 'remove' '\n-\n-test_oidmap \"put one 1\n-put two 2\n-put three 3\n-remove one\n-remove two\n-remove invalidOid\n-remove four\" \"NULL\n-NULL\n-NULL\n-1\n-2\n-Unknown oid: invalidOid\n-NULL\"\n-\n-'\n-\n-test_expect_success 'iterate' '\n-\ttest-tool oidmap >actual.raw <<-\\EOF &&\n-\tput one 1\n-\tput two 2\n-\tput three 3\n-\titerate\n-\tEOF\n-\n-\t# sort \"expect\" too so we do not rely on the order of particular oids\n-\tsort >expect <<-EOF &&\n-\tNULL\n-\tNULL\n-\tNULL\n-\t$(git rev-parse one) 1\n-\t$(git rev-parse two) 2\n-\t$(git rev-parse three) 3\n-\tEOF\n-\n-\tsort <actual.raw >actual &&\n-\ttest_cmp expect actual\n-'\n-\n-test_done\ndiff --git a/t/unit-tests/t-oidmap.c b/t/unit-tests/t-oidmap.c\nnew file mode 100644\nindex 0000000000..b22e52d08b\n--- /dev/null\n+++ b/t/unit-tests/t-oidmap.c\n@@ -0,0 +1,181 @@\n+#include \"test-lib.h\"\n+#include \"lib-oid.h\"\n+#include \"oidmap.h\"\n+#include \"hash.h\"\n+#include \"hex.h\"\n+\n+/*\n+ * Elements we will put in oidmap structs are made of a key: the entry.oid\n+ * field, which is of type struct object_id, and a value: the name field (could\n+ * be a refname for example).\n+ */\n+struct test_entry {\n+\tstruct oidmap_entry entry;\n+\tchar name[FLEX_ARRAY];\n+};\n+\n+static const char *const key_val[][2] = { { \"11\", \"one\" },\n+\t\t\t\t\t  { \"22\", \"two\" },\n+\t\t\t\t\t  { \"33\", \"three\" } };\n+\n+static void setup(void (*f)(struct oidmap *map))\n+{\n+\tstruct oidmap map = OIDMAP_INIT;\n+\tint ret = 0;\n+\n+\tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++){\n+\t\tstruct test_entry *entry;\n+\n+\t\tFLEX_ALLOC_STR(entry, name, key_val[i][1]);\n+\t\tif ((ret = get_oid_arbitrary_hex(key_val[i][0], &entry->entry.oid))) {\n+\t\t\tfree(entry);\n+\t\t\tbreak;\n+\t\t}\n+\t\tentry = oidmap_put(&map, entry);\n+\t\tif (!check(entry == NULL))\n+\t\t\tfree(entry);\n+\t}\n+\n+\tif (!ret)\n+\t\tf(&map);\n+\toidmap_free(&map, 1);\n+}\n+\n+static void t_replace(struct oidmap *map)\n+{\n+\tstruct test_entry *entry, *prev;\n+\n+\tFLEX_ALLOC_STR(entry, name, \"un\");\n+\tif (get_oid_arbitrary_hex(\"11\", &entry->entry.oid))\n+\t\treturn;\n+\tprev = oidmap_put(map, entry);\n+\tif (!check(prev != NULL))\n+\t\treturn;\n+\tcheck_str(prev->name, \"one\");\n+\tfree(prev);\n+\n+\tFLEX_ALLOC_STR(entry, name, \"deux\");\n+\tif (get_oid_arbitrary_hex(\"22\", &entry->entry.oid))\n+\t\treturn;\n+\tprev = oidmap_put(map, entry);\n+\tif (!check(prev != NULL))\n+\t\treturn;\n+\tcheck_str(prev->name, \"two\");\n+\tfree(prev);\n+}\n+\n+static void t_get(struct oidmap *map)\n+{\n+\tstruct test_entry *entry;\n+\tstruct object_id oid;\n+\n+\tif (get_oid_arbitrary_hex(\"22\", &oid))\n+\t\treturn;\n+\tentry = oidmap_get(map, &oid);\n+\tif (!check(entry != NULL))\n+\t\treturn;\n+\tcheck_str(entry->name, \"two\");\n+\n+\tif (get_oid_arbitrary_hex(\"44\", &oid))\n+\t\treturn;\n+\tcheck(oidmap_get(map, &oid) == NULL);\n+\n+\tif (get_oid_arbitrary_hex(\"11\", &oid))\n+\t\treturn;\n+\tentry = oidmap_get(map, &oid);\n+\tif (!check(entry != NULL))\n+\t\treturn;\n+\tcheck_str(entry->name, \"one\");\n+}\n+\n+static void t_remove(struct oidmap *map)\n+{\n+\tstruct test_entry *entry;\n+\tstruct object_id oid;\n+\n+\tif (get_oid_arbitrary_hex(\"11\", &oid))\n+\t\treturn;\n+\tentry = oidmap_remove(map, &oid);\n+\tif (!check(entry != NULL))\n+\t\treturn;\n+\tcheck_str(entry->name, \"one\");\n+\tcheck(oidmap_get(map, &oid) == NULL);\n+\tfree(entry);\n+\n+\tif (get_oid_arbitrary_hex(\"22\", &oid))\n+\t\treturn;\n+\tentry = oidmap_remove(map, &oid);\n+\tif (!check(entry != NULL))\n+\t\treturn;\n+\tcheck_str(entry->name, \"two\");\n+\tcheck(oidmap_get(map, &oid) == NULL);\n+\tfree(entry);\n+\n+\tif (get_oid_arbitrary_hex(\"44\", &oid))\n+\t\treturn;\n+\tcheck(oidmap_remove(map, &oid) == NULL);\n+}\n+\n+static int key_val_contains(struct test_entry *entry, char seen[])\n+{\n+\tfor (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n+\t\tstruct object_id oid;\n+\n+\t\tif (get_oid_arbitrary_hex(key_val[i][0], &oid))\n+\t\t\treturn -1;\n+\n+\t\tif (oideq(&entry->entry.oid, &oid)) {\n+\t\t\tif (seen[i])\n+\t\t\t\treturn 2;\n+\t\t\tseen[i] = 1;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+static void t_iterate(struct oidmap *map)\n+{\n+\tstruct oidmap_iter iter;\n+\tstruct test_entry *entry;\n+\tchar seen[ARRAY_SIZE(key_val)] = { 0 };\n+\tint count = 0;\n+\n+\toidmap_iter_init(map, &iter);\n+\twhile ((entry = oidmap_iter_next(&iter))) {\n+\t\tint ret;\n+\t\tif (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n+\t\t\tswitch (ret) {\n+\t\t\tcase -1:\n+\t\t\t\tbreak; /* error message handled by get_oid_arbitrary_hex() */\n+\t\t\tcase 1:\n+\t\t\t\ttest_msg(\"obtained entry was not given in the input\\n\"\n+\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n+\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n+\t\t\t\tbreak;\n+\t\t\tcase 2:\n+\t\t\t\ttest_msg(\"duplicate entry detected\\n\"\n+\t\t\t\t\t \"  name: %s\\n   oid: %s\\n\",\n+\t\t\t\t\t entry->name, oid_to_hex(&entry->entry.oid));\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\ttest_msg(\"BUG: invalid return value (%d) from key_val_contains()\",\n+\t\t\t\t\t ret);\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tcount++;\n+\t\t}\n+\t}\n+\tcheck_int(count, ==, ARRAY_SIZE(key_val));\n+\tcheck_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));\n+}\n+\n+int cmd_main(int argc UNUSED, const char **argv UNUSED)\n+{\n+\tTEST(setup(t_replace), \"replace works\");\n+\tTEST(setup(t_get), \"get works\");\n+\tTEST(setup(t_remove), \"remove works\");\n+\tTEST(setup(t_iterate), \"iterate works\");\n+\treturn test_done();\n+}\n-- \n2.45.2\n\n"},{"id":"498016","messageId":"D2FPNR4PMYNG.AGGPEUYK2ETE@gmail.com","threadId":"61722","inReplyTo":"20240703062958.23262-2-shyamthakkar001@gmail.com","subject":"Re: [GSoC][PATCH v3] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c","fromName":"Ghanshyam Thakkar","fromEmail":"shyamthakkar001@gmail.com","sentAt":"2024-07-03T07:04:15Z","receivedAt":"2024-07-03T07:04:22Z","isPatch":true,"sender":{"key":"shyamthakkar001@gmail.com","avatar":"https://avatars.githubusercontent.com/u/72698233?v=4"},"body":"Ghanshyam Thakkar <shyamthakkar001@gmail.com> wrote:\n> helper/test-oidmap.c along with t0016-oidmap.sh test the oidmap.h\n> library which is built on top of hashmap.h.\n>\n> Migrate them to the unit testing framework for better performance,\n> concise code and better debugging. Along with the migration also plug\n> memory leaks and make the test logic independent for all the tests.\n> The migration removes 'put' tests from t0016, because it is used as\n> setup to all the other tests, so testing it separately does not yield\n> any benefit.\n>\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Helped-by: Phillip Wood <phillip.wood123@gmail.com>\n> Reviewed-by: Josh Steadmon <steadmon@google.com>\n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n> ---\n\nForgot to put in the message-id for reply. Apologies for the noise.\n\nlink to v1: https://lore.kernel.org/git/20240619175036.64291-1-shyamthakkar001@gmail.com/\nlink to v2: https://lore.kernel.org/git/20240628122030.41554-1-shyamthakkar001@gmail.com/\n\nThanks.\n\n> Changes in v3:\n> - use 'count' to check if we iterated over all the entries (Phillip's\n> suggestion)\n> - use 'seen' array instead of modifying the global array (Junio's\n> review)\n>\n> Range-diff against v2:\n> 1: cc0c4c3b0a ! 1: bdb3c8ebe4 t: migrate helper/test-oidmap.c to\n> unit-tests/t-oidmap.c\n> @@ Commit message\n> setup to all the other tests, so testing it separately does not yield\n> any benefit.\n>      \n> + Helped-by: Junio C Hamano <gitster@pobox.com>\n> Helped-by: Phillip Wood <phillip.wood123@gmail.com>\n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n> @@ t/unit-tests/t-oidmap.c (new)\n> + char name[FLEX_ARRAY];\n> +};\n> +\n> -+static const char *key_val[][2] = { { \"11\", \"one\" },\n> -+ { \"22\", \"two\" },\n> -+ { \"33\", \"three\" } };\n> ++static const char *const key_val[][2] = { { \"11\", \"one\" },\n> ++ { \"22\", \"two\" },\n> ++ { \"33\", \"three\" } };\n> +\n> +static void setup(void (*f)(struct oidmap *map))\n> +{\n> @@ t/unit-tests/t-oidmap.c (new)\n> + check(oidmap_remove(map, &oid) == NULL);\n> +}\n> +\n> -+static int key_val_contains(struct test_entry *entry)\n> ++static int key_val_contains(struct test_entry *entry, char seen[])\n> +{\n> + for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n> + struct object_id oid;\n> @@ t/unit-tests/t-oidmap.c (new)\n> + return -1;\n> +\n> + if (oideq(&entry->entry.oid, &oid)) {\n> -+ if (!strcmp(key_val[i][1], \"USED\"))\n> ++ if (seen[i])\n> + return 2;\n> -+ key_val[i][1] = \"USED\";\n> ++ seen[i] = 1;\n> + return 0;\n> + }\n> + }\n> @@ t/unit-tests/t-oidmap.c (new)\n> +{\n> + struct oidmap_iter iter;\n> + struct test_entry *entry;\n> ++ char seen[ARRAY_SIZE(key_val)] = { 0 };\n> ++ int count = 0;\n> +\n> + oidmap_iter_init(map, &iter);\n> + while ((entry = oidmap_iter_next(&iter))) {\n> + int ret;\n> -+ if (!check_int((ret = key_val_contains(entry)), ==, 0)) {\n> ++ if (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n> + switch (ret) {\n> + case -1:\n> + break; /* error message handled by get_oid_arbitrary_hex() */\n> @@ t/unit-tests/t-oidmap.c (new)\n> + ret);\n> + break;\n> + }\n> ++ } else {\n> ++ count++;\n> + }\n> + }\n> ++ check_int(count, ==, ARRAY_SIZE(key_val));\n> + check_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));\n> +}\n> +\n>\n> Makefile | 2 +-\n> t/helper/test-oidmap.c | 123 ---------------------------\n> t/helper/test-tool.c | 1 -\n> t/helper/test-tool.h | 1 -\n> t/t0016-oidmap.sh | 112 -------------------------\n> t/unit-tests/t-oidmap.c | 181 ++++++++++++++++++++++++++++++++++++++++\n> 6 files changed, 182 insertions(+), 238 deletions(-)\n> delete mode 100644 t/helper/test-oidmap.c\n> delete mode 100755 t/t0016-oidmap.sh\n> create mode 100644 t/unit-tests/t-oidmap.c\n>\n> diff --git a/Makefile b/Makefile\n> index 3eab701b10..2a5c70d218 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -809,7 +809,6 @@ TEST_BUILTINS_OBJS += test-match-trees.o\n> TEST_BUILTINS_OBJS += test-mergesort.o\n> TEST_BUILTINS_OBJS += test-mktemp.o\n> TEST_BUILTINS_OBJS += test-oid-array.o\n> -TEST_BUILTINS_OBJS += test-oidmap.o\n> TEST_BUILTINS_OBJS += test-online-cpus.o\n> TEST_BUILTINS_OBJS += test-pack-mtimes.o\n> TEST_BUILTINS_OBJS += test-parse-options.o\n> @@ -1337,6 +1336,7 @@ UNIT_TEST_PROGRAMS += t-ctype\n> UNIT_TEST_PROGRAMS += t-example-decorate\n> UNIT_TEST_PROGRAMS += t-hash\n> UNIT_TEST_PROGRAMS += t-mem-pool\n> +UNIT_TEST_PROGRAMS += t-oidmap\n> UNIT_TEST_PROGRAMS += t-oidtree\n> UNIT_TEST_PROGRAMS += t-prio-queue\n> UNIT_TEST_PROGRAMS += t-reftable-basics\n> diff --git a/t/helper/test-oidmap.c b/t/helper/test-oidmap.c\n> deleted file mode 100644\n> index bd30244a54..0000000000\n> --- a/t/helper/test-oidmap.c\n> +++ /dev/null\n> @@ -1,123 +0,0 @@\n> -#include \"test-tool.h\"\n> -#include \"hex.h\"\n> -#include \"object-name.h\"\n> -#include \"oidmap.h\"\n> -#include \"repository.h\"\n> -#include \"setup.h\"\n> -#include \"strbuf.h\"\n> -#include \"string-list.h\"\n> -\n> -/* key is an oid and value is a name (could be a refname for example)\n> */\n> -struct test_entry {\n> - struct oidmap_entry entry;\n> - char name[FLEX_ARRAY];\n> -};\n> -\n> -#define DELIM \" \\t\\r\\n\"\n> -\n> -/*\n> - * Read stdin line by line and print result of commands to stdout:\n> - *\n> - * hash oidkey -> sha1hash(oidkey)\n> - * put oidkey namevalue -> NULL / old namevalue\n> - * get oidkey -> NULL / namevalue\n> - * remove oidkey -> NULL / old namevalue\n> - * iterate -> oidkey1 namevalue1\\noidkey2 namevalue2\\n...\n> - *\n> - */\n> -int cmd__oidmap(int argc UNUSED, const char **argv UNUSED)\n> -{\n> - struct string_list parts = STRING_LIST_INIT_NODUP;\n> - struct strbuf line = STRBUF_INIT;\n> - struct oidmap map = OIDMAP_INIT;\n> -\n> - setup_git_directory();\n> -\n> - /* init oidmap */\n> - oidmap_init(&map, 0);\n> -\n> - /* process commands from stdin */\n> - while (strbuf_getline(&line, stdin) != EOF) {\n> - char *cmd, *p1, *p2;\n> - struct test_entry *entry;\n> - struct object_id oid;\n> -\n> - /* break line into command and up to two parameters */\n> - string_list_setlen(&parts, 0);\n> - string_list_split_in_place(&parts, line.buf, DELIM, 2);\n> - string_list_remove_empty_items(&parts, 0);\n> -\n> - /* ignore empty lines */\n> - if (!parts.nr)\n> - continue;\n> - if (!*parts.items[0].string || *parts.items[0].string == '#')\n> - continue;\n> -\n> - cmd = parts.items[0].string;\n> - p1 = parts.nr >= 1 ? parts.items[1].string : NULL;\n> - p2 = parts.nr >= 2 ? parts.items[2].string : NULL;\n> -\n> - if (!strcmp(\"put\", cmd) && p1 && p2) {\n> -\n> - if (repo_get_oid(the_repository, p1, &oid)) {\n> - printf(\"Unknown oid: %s\\n\", p1);\n> - continue;\n> - }\n> -\n> - /* create entry with oid_key = p1, name_value = p2 */\n> - FLEX_ALLOC_STR(entry, name, p2);\n> - oidcpy(&entry->entry.oid, &oid);\n> -\n> - /* add / replace entry */\n> - entry = oidmap_put(&map, entry);\n> -\n> - /* print and free replaced entry, if any */\n> - puts(entry ? entry->name : \"NULL\");\n> - free(entry);\n> -\n> - } else if (!strcmp(\"get\", cmd) && p1) {\n> -\n> - if (repo_get_oid(the_repository, p1, &oid)) {\n> - printf(\"Unknown oid: %s\\n\", p1);\n> - continue;\n> - }\n> -\n> - /* lookup entry in oidmap */\n> - entry = oidmap_get(&map, &oid);\n> -\n> - /* print result */\n> - puts(entry ? entry->name : \"NULL\");\n> -\n> - } else if (!strcmp(\"remove\", cmd) && p1) {\n> -\n> - if (repo_get_oid(the_repository, p1, &oid)) {\n> - printf(\"Unknown oid: %s\\n\", p1);\n> - continue;\n> - }\n> -\n> - /* remove entry from oidmap */\n> - entry = oidmap_remove(&map, &oid);\n> -\n> - /* print result and free entry*/\n> - puts(entry ? entry->name : \"NULL\");\n> - free(entry);\n> -\n> - } else if (!strcmp(\"iterate\", cmd)) {\n> -\n> - struct oidmap_iter iter;\n> - oidmap_iter_init(&map, &iter);\n> - while ((entry = oidmap_iter_next(&iter)))\n> - printf(\"%s %s\\n\", oid_to_hex(&entry->entry.oid), entry->name);\n> -\n> - } else {\n> -\n> - printf(\"Unknown command %s\\n\", cmd);\n> -\n> - }\n> - }\n> -\n> - string_list_clear(&parts, 0);\n> - strbuf_release(&line);\n> - oidmap_free(&map, 1);\n> - return 0;\n> -}\n> diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\n> index 93436a82ae..da3e69128a 100644\n> --- a/t/helper/test-tool.c\n> +++ b/t/helper/test-tool.c\n> @@ -44,7 +44,6 @@ static struct test_cmd cmds[] = {\n> { \"mergesort\", cmd__mergesort },\n> { \"mktemp\", cmd__mktemp },\n> { \"oid-array\", cmd__oid_array },\n> - { \"oidmap\", cmd__oidmap },\n> { \"online-cpus\", cmd__online_cpus },\n> { \"pack-mtimes\", cmd__pack_mtimes },\n> { \"parse-options\", cmd__parse_options },\n> diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\n> index d9033d14e1..642a34578c 100644\n> --- a/t/helper/test-tool.h\n> +++ b/t/helper/test-tool.h\n> @@ -37,7 +37,6 @@ int cmd__lazy_init_name_hash(int argc, const char\n> **argv);\n> int cmd__match_trees(int argc, const char **argv);\n> int cmd__mergesort(int argc, const char **argv);\n> int cmd__mktemp(int argc, const char **argv);\n> -int cmd__oidmap(int argc, const char **argv);\n> int cmd__online_cpus(int argc, const char **argv);\n> int cmd__pack_mtimes(int argc, const char **argv);\n> int cmd__parse_options(int argc, const char **argv);\n> diff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh\n> deleted file mode 100755\n> index 0faef1f4f1..0000000000\n> --- a/t/t0016-oidmap.sh\n> +++ /dev/null\n> @@ -1,112 +0,0 @@\n> -#!/bin/sh\n> -\n> -test_description='test oidmap'\n> -\n> -TEST_PASSES_SANITIZE_LEAK=true\n> -. ./test-lib.sh\n> -\n> -# This purposefully is very similar to t0011-hashmap.sh\n> -\n> -test_oidmap () {\n> - echo \"$1\" | test-tool oidmap $3 >actual &&\n> - echo \"$2\" >expect &&\n> - test_cmp expect actual\n> -}\n> -\n> -\n> -test_expect_success 'setup' '\n> -\n> - test_commit one &&\n> - test_commit two &&\n> - test_commit three &&\n> - test_commit four\n> -\n> -'\n> -\n> -test_expect_success 'put' '\n> -\n> -test_oidmap \"put one 1\n> -put two 2\n> -put invalidOid 4\n> -put three 3\" \"NULL\n> -NULL\n> -Unknown oid: invalidOid\n> -NULL\"\n> -\n> -'\n> -\n> -test_expect_success 'replace' '\n> -\n> -test_oidmap \"put one 1\n> -put two 2\n> -put three 3\n> -put invalidOid 4\n> -put two deux\n> -put one un\" \"NULL\n> -NULL\n> -NULL\n> -Unknown oid: invalidOid\n> -2\n> -1\"\n> -\n> -'\n> -\n> -test_expect_success 'get' '\n> -\n> -test_oidmap \"put one 1\n> -put two 2\n> -put three 3\n> -get two\n> -get four\n> -get invalidOid\n> -get one\" \"NULL\n> -NULL\n> -NULL\n> -2\n> -NULL\n> -Unknown oid: invalidOid\n> -1\"\n> -\n> -'\n> -\n> -test_expect_success 'remove' '\n> -\n> -test_oidmap \"put one 1\n> -put two 2\n> -put three 3\n> -remove one\n> -remove two\n> -remove invalidOid\n> -remove four\" \"NULL\n> -NULL\n> -NULL\n> -1\n> -2\n> -Unknown oid: invalidOid\n> -NULL\"\n> -\n> -'\n> -\n> -test_expect_success 'iterate' '\n> - test-tool oidmap >actual.raw <<-\\EOF &&\n> - put one 1\n> - put two 2\n> - put three 3\n> - iterate\n> - EOF\n> -\n> - # sort \"expect\" too so we do not rely on the order of particular oids\n> - sort >expect <<-EOF &&\n> - NULL\n> - NULL\n> - NULL\n> - $(git rev-parse one) 1\n> - $(git rev-parse two) 2\n> - $(git rev-parse three) 3\n> - EOF\n> -\n> - sort <actual.raw >actual &&\n> - test_cmp expect actual\n> -'\n> -\n> -test_done\n> diff --git a/t/unit-tests/t-oidmap.c b/t/unit-tests/t-oidmap.c\n> new file mode 100644\n> index 0000000000..b22e52d08b\n> --- /dev/null\n> +++ b/t/unit-tests/t-oidmap.c\n> @@ -0,0 +1,181 @@\n> +#include \"test-lib.h\"\n> +#include \"lib-oid.h\"\n> +#include \"oidmap.h\"\n> +#include \"hash.h\"\n> +#include \"hex.h\"\n> +\n> +/*\n> + * Elements we will put in oidmap structs are made of a key: the\n> entry.oid\n> + * field, which is of type struct object_id, and a value: the name\n> field (could\n> + * be a refname for example).\n> + */\n> +struct test_entry {\n> + struct oidmap_entry entry;\n> + char name[FLEX_ARRAY];\n> +};\n> +\n> +static const char *const key_val[][2] = { { \"11\", \"one\" },\n> + { \"22\", \"two\" },\n> + { \"33\", \"three\" } };\n> +\n> +static void setup(void (*f)(struct oidmap *map))\n> +{\n> + struct oidmap map = OIDMAP_INIT;\n> + int ret = 0;\n> +\n> + for (size_t i = 0; i < ARRAY_SIZE(key_val); i++){\n> + struct test_entry *entry;\n> +\n> + FLEX_ALLOC_STR(entry, name, key_val[i][1]);\n> + if ((ret = get_oid_arbitrary_hex(key_val[i][0], &entry->entry.oid))) {\n> + free(entry);\n> + break;\n> + }\n> + entry = oidmap_put(&map, entry);\n> + if (!check(entry == NULL))\n> + free(entry);\n> + }\n> +\n> + if (!ret)\n> + f(&map);\n> + oidmap_free(&map, 1);\n> +}\n> +\n> +static void t_replace(struct oidmap *map)\n> +{\n> + struct test_entry *entry, *prev;\n> +\n> + FLEX_ALLOC_STR(entry, name, \"un\");\n> + if (get_oid_arbitrary_hex(\"11\", &entry->entry.oid))\n> + return;\n> + prev = oidmap_put(map, entry);\n> + if (!check(prev != NULL))\n> + return;\n> + check_str(prev->name, \"one\");\n> + free(prev);\n> +\n> + FLEX_ALLOC_STR(entry, name, \"deux\");\n> + if (get_oid_arbitrary_hex(\"22\", &entry->entry.oid))\n> + return;\n> + prev = oidmap_put(map, entry);\n> + if (!check(prev != NULL))\n> + return;\n> + check_str(prev->name, \"two\");\n> + free(prev);\n> +}\n> +\n> +static void t_get(struct oidmap *map)\n> +{\n> + struct test_entry *entry;\n> + struct object_id oid;\n> +\n> + if (get_oid_arbitrary_hex(\"22\", &oid))\n> + return;\n> + entry = oidmap_get(map, &oid);\n> + if (!check(entry != NULL))\n> + return;\n> + check_str(entry->name, \"two\");\n> +\n> + if (get_oid_arbitrary_hex(\"44\", &oid))\n> + return;\n> + check(oidmap_get(map, &oid) == NULL);\n> +\n> + if (get_oid_arbitrary_hex(\"11\", &oid))\n> + return;\n> + entry = oidmap_get(map, &oid);\n> + if (!check(entry != NULL))\n> + return;\n> + check_str(entry->name, \"one\");\n> +}\n> +\n> +static void t_remove(struct oidmap *map)\n> +{\n> + struct test_entry *entry;\n> + struct object_id oid;\n> +\n> + if (get_oid_arbitrary_hex(\"11\", &oid))\n> + return;\n> + entry = oidmap_remove(map, &oid);\n> + if (!check(entry != NULL))\n> + return;\n> + check_str(entry->name, \"one\");\n> + check(oidmap_get(map, &oid) == NULL);\n> + free(entry);\n> +\n> + if (get_oid_arbitrary_hex(\"22\", &oid))\n> + return;\n> + entry = oidmap_remove(map, &oid);\n> + if (!check(entry != NULL))\n> + return;\n> + check_str(entry->name, \"two\");\n> + check(oidmap_get(map, &oid) == NULL);\n> + free(entry);\n> +\n> + if (get_oid_arbitrary_hex(\"44\", &oid))\n> + return;\n> + check(oidmap_remove(map, &oid) == NULL);\n> +}\n> +\n> +static int key_val_contains(struct test_entry *entry, char seen[])\n> +{\n> + for (size_t i = 0; i < ARRAY_SIZE(key_val); i++) {\n> + struct object_id oid;\n> +\n> + if (get_oid_arbitrary_hex(key_val[i][0], &oid))\n> + return -1;\n> +\n> + if (oideq(&entry->entry.oid, &oid)) {\n> + if (seen[i])\n> + return 2;\n> + seen[i] = 1;\n> + return 0;\n> + }\n> + }\n> + return 1;\n> +}\n> +\n> +static void t_iterate(struct oidmap *map)\n> +{\n> + struct oidmap_iter iter;\n> + struct test_entry *entry;\n> + char seen[ARRAY_SIZE(key_val)] = { 0 };\n> + int count = 0;\n> +\n> + oidmap_iter_init(map, &iter);\n> + while ((entry = oidmap_iter_next(&iter))) {\n> + int ret;\n> + if (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {\n> + switch (ret) {\n> + case -1:\n> + break; /* error message handled by get_oid_arbitrary_hex() */\n> + case 1:\n> + test_msg(\"obtained entry was not given in the input\\n\"\n> + \" name: %s\\n oid: %s\\n\",\n> + entry->name, oid_to_hex(&entry->entry.oid));\n> + break;\n> + case 2:\n> + test_msg(\"duplicate entry detected\\n\"\n> + \" name: %s\\n oid: %s\\n\",\n> + entry->name, oid_to_hex(&entry->entry.oid));\n> + break;\n> + default:\n> + test_msg(\"BUG: invalid return value (%d) from key_val_contains()\",\n> + ret);\n> + break;\n> + }\n> + } else {\n> + count++;\n> + }\n> + }\n> + check_int(count, ==, ARRAY_SIZE(key_val));\n> + check_int(hashmap_get_size(&map->map), ==, ARRAY_SIZE(key_val));\n> +}\n> +\n> +int cmd_main(int argc UNUSED, const char **argv UNUSED)\n> +{\n> + TEST(setup(t_replace), \"replace works\");\n> + TEST(setup(t_get), \"get works\");\n> + TEST(setup(t_remove), \"remove works\");\n> + TEST(setup(t_iterate), \"iterate works\");\n> + return test_done();\n> +}\n> --\n> 2.45.2\n\n"},{"id":"498040","messageId":"xmqqle2ie9b4.fsf@gitster.g","threadId":"61722","inReplyTo":"20240703062958.23262-2-shyamthakkar001@gmail.com","subject":"Re: [GSoC][PATCH v3] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-03T16:20:47Z","receivedAt":"2024-07-03T16:20:50Z","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> helper/test-oidmap.c along with t0016-oidmap.sh test the oidmap.h\n> library which is built on top of hashmap.h.\n>\n> Migrate them to the unit testing framework for better performance,\n> concise code and better debugging. Along with the migration also plug\n> memory leaks and make the test logic independent for all the tests.\n> The migration removes 'put' tests from t0016, because it is used as\n> setup to all the other tests, so testing it separately does not yield\n> any benefit.\n>\n\n> Helped-by: Junio C Hamano <gitster@pobox.com>\n> Helped-by: Phillip Wood <phillip.wood123@gmail.com>\n> Reviewed-by: Josh Steadmon <steadmon@google.com>\n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n> Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n\nThe trailer lines should come more-or-less in chronological order.\nI do not know exact sequence of events, but mentoring would have\nhappened first before the initial iteration was posted, then Phillip\nhelped to improve it during iterations, the latest was reviewed by\nJosh and then you folded the little test improvement from me and\nPhillip?  And to seal the whole thing off, you add your sign-off at\nthe end.\n\nTechnically speaking, any change after a review invalidates an\nearlier \"Reviewed-by\", but the updates we see here, relative to the\niteration that received the \"Reviewed-by\", are not significant or\nlarge enough to warrant that, so let's pretend that Josh would be\nhappy with the end result, even with our little additions.\n\nWhich leads us to the trailer lines ordered like so:\n\n    Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n    Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n    Helped-by: Phillip Wood <phillip.wood123@gmail.com>\n    Helped-by: Junio C Hamano <gitster@pobox.com>\n    Reviewed-by: Josh Steadmon <steadmon@google.com>\n    Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>\n\nThe changes since the previous round look exactly as expected.\nLooking very good.\n\nWill queue.  Let's mark it for 'next'.\n\nThanks.\n\n\n"},{"id":"498078","messageId":"6d747a7d-9091-4ec1-b059-6ecf16d89846@gmail.com","threadId":"61722","inReplyTo":"xmqqle2ie9b4.fsf@gitster.g","subject":"Re: [GSoC][PATCH v3] t: migrate helper/test-oidmap.c to unit-tests/t-oidmap.c","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-07-04T09:44:26Z","receivedAt":"2024-07-04T09:44:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 03/07/2024 17:20, Junio C Hamano wrote:\n> Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:\n> \n> The changes since the previous round look exactly as expected.\n> Looking very good.\n\nYes this is looking really good now\n\nBest Wishes\n\nPhillip\n"}]}