{"thread":{"id":"61947","subject":"[GSoC][PATCH 00/10] t: port reftable/block_test.c to the unit testing framework","startedAt":"2024-08-14T12:12:07Z","lastAt":"2024-08-22T18:01:07Z","messageCount":53,"participants":["Chandra Pratap","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"500876","messageId":"20240814121122.4642-1-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":null,"subject":"[GSoC][PATCH 00/10] t: port reftable/block_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:08Z","receivedAt":"2024-08-14T12:12:07Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"The reftable library comes with self tests, which are exercised\nas part of the usual end-to-end tests and are designed to\nobserve the end-user visible effects of Git commands. What it\nexercises, however, is a better match for the unit-testing\nframework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests',\n2023-12-09), which is designed to observe how low level\nimplementation details, at the level of sequences of individual\nfunction calls, behave.\n\nHence, port reftable/block_test.c to the unit testing framework and\nimprove upon the ported test. The first patch in the series moves\nthe test to the unit testing framework, and the rest of the patches\nimprove upon the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n\n---\nCI/PR: https://github.com/gitgitgadget/git/pull/1749\n\nChandra Pratap(10):\nt: move reftable/block_test.c to the unit testing framework\nt-reftable-block: release used block reader\nt-reftable-block: use reftable_record_equal() instead of check_str()\nt-reftable-block: use reftable_record_key() instead of strbuf_addstr()\nt-reftable-block: use block_iter_reset() instead of block_iter_close()\nt-reftable-block: use xstrfmt() instead of xstrdup()\nt-reftable-block: remove unnecessary variable 'j'\nt-reftable-block: add tests for log blocks\nt-reftable-block: add tests for obj blocks\nt-reftable-block: add tests for index blocks\n\nMakefile | 2 +-\nreftable/block_test.c | 123 --------------\nreftable/reftable-tests.h | 1 -\nt/helper/test-reftable.c | 1 -\nt/unit-tests/t-reftable-block.c | 359 ++++++++++++++++++++++++++++++++++++++++\n5 files changed, 360 insertions(+), 126 deletions(-)\n"},{"id":"500877","messageId":"20240814121122.4642-2-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 01/10] t: move reftable/block_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:09Z","receivedAt":"2024-08-14T12:12:10Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/block_test.c exercises the functions defined in\nreftable/block.{c, h}. Migrate reftable/block_test.c to the unit\ntesting framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework and renaming the tests to follow the unit-tests'\nnaming conventions.\n\nWhile at it, ensure structs are 0-initialized with '= { 0 }'\ninstead of '= { NULL }' and array indices have type 'size_t'\ninstead of 'int'.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n Makefile | 2 +-\n reftable/reftable-tests.h | 1 -\n t/helper/test-reftable.c | 1 -\n .../unit-tests/t-reftable-block.c | 63 +++++++++----------\n 4 files changed, 30 insertions(+), 37 deletions(-)\n rename reftable/block_test.c => t/unit-tests/t-reftable-block.c (67%)\n\ndiff --git a/Makefile b/Makefile\nindex 3863e60b66..f030283447 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1340,6 +1340,7 @@ 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+UNIT_TEST_PROGRAMS += t-reftable-block\n UNIT_TEST_PROGRAMS += t-reftable-merged\n UNIT_TEST_PROGRAMS += t-reftable-record\n UNIT_TEST_PROGRAMS += t-strbuf\n@@ -2679,7 +2680,6 @@ REFTABLE_OBJS += reftable/stack.o\n REFTABLE_OBJS += reftable/tree.o\n REFTABLE_OBJS += reftable/writer.o\n \n-REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n REFTABLE_TEST_OBJS += reftable/pq_test.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\ndiff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h\nindex d5e03dcc1b..e25ca86ede 100644\n--- a/reftable/reftable-tests.h\n+++ b/reftable/reftable-tests.h\n@@ -10,7 +10,6 @@ license that can be found in the LICENSE file or at\n #define REFTABLE_TESTS_H\n \n int basics_test_main(int argc, const char **argv);\n-int block_test_main(int argc, const char **argv);\n int pq_test_main(int argc, const char **argv);\n int record_test_main(int argc, const char **argv);\n int readwrite_test_main(int argc, const char **argv);\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 9d378427da..4eda545fad 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -5,7 +5,6 @@\n int cmd__reftable(int argc, const char **argv)\n {\n \t/* test from simple to complex. */\n-\tblock_test_main(argc, argv);\n \ttree_test_main(argc, argv);\n \tpq_test_main(argc, argv);\n \treadwrite_test_main(argc, argv);\ndiff --git a/reftable/block_test.c b/t/unit-tests/t-reftable-block.c\nsimilarity index 67%\nrename from reftable/block_test.c\nrename to t/unit-tests/t-reftable-block.c\nindex 90aecd5a7c..8ab3ff9ebe 100644\n--- a/reftable/block_test.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -6,34 +6,30 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"block.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/block.h\"\n+#include \"reftable/blocksource.h\"\n+#include \"reftable/constants.h\"\n+#include \"reftable/reftable-error.h\"\n \n-#include \"system.h\"\n-#include \"blocksource.h\"\n-#include \"basics.h\"\n-#include \"constants.h\"\n-#include \"record.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-tests.h\"\n-\n-static void test_block_read_write(void)\n+static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tchar *names[30];\n-\tconst int N = ARRAY_SIZE(names);\n-\tconst int block_size = 1024;\n-\tstruct reftable_block block = { NULL };\n+\tconst size_t N = ARRAY_SIZE(names);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n \tstruct block_writer bw = {\n \t\t.last_key = STRBUF_INIT,\n \t};\n \tstruct reftable_record rec = {\n \t\t.type = BLOCK_TYPE_REF,\n \t};\n-\tint i = 0;\n+\tsize_t i = 0;\n \tint n;\n \tstruct block_reader br = { 0 };\n \tstruct block_iter it = BLOCK_ITER_INIT;\n-\tint j = 0;\n+\tsize_t j = 0;\n \tstruct strbuf want = STRBUF_INIT;\n \n \tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n@@ -45,11 +41,11 @@ static void test_block_read_write(void)\n \trec.u.ref.refname = (char *) \"\";\n \trec.u.ref.value_type = REFTABLE_REF_DELETION;\n \tn = block_writer_add(&bw, &rec);\n-\tEXPECT(n == REFTABLE_API_ERROR);\n+\tcheck_int(n, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n-\t\tsnprintf(name, sizeof(name), \"branch%02d\", i);\n+\t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX , (uintmax_t)i);\n \n \t\trec.u.ref.refname = name;\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n@@ -59,11 +55,11 @@ static void test_block_read_write(void)\n \t\tn = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \t}\n \n \tn = block_writer_finish(&bw);\n-\tEXPECT(n > 0);\n+\tcheck_int(n, >, 0);\n \n \tblock_writer_release(&bw);\n \n@@ -73,11 +69,10 @@ static void test_block_read_write(void)\n \n \twhile (1) {\n \t\tint r = block_iter_next(&it, &rec);\n-\t\tEXPECT(r >= 0);\n-\t\tif (r > 0) {\n+\t\tcheck_int(r, >=, 0);\n+\t\tif (r > 0)\n \t\t\tbreak;\n-\t\t}\n-\t\tEXPECT_STREQ(names[j], rec.u.ref.refname);\n+\t\tcheck_str(names[j], rec.u.ref.refname);\n \t\tj++;\n \t}\n \n@@ -90,20 +85,20 @@ static void test_block_read_write(void)\n \t\tstrbuf_addstr(&want, names[i]);\n \n \t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n \t\tn = block_iter_next(&it, &rec);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n-\t\tEXPECT_STREQ(names[i], rec.u.ref.refname);\n+\t\tcheck_str(names[i], rec.u.ref.refname);\n \n \t\twant.len--;\n \t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n \t\tn = block_iter_next(&it, &rec);\n-\t\tEXPECT(n == 0);\n-\t\tEXPECT_STREQ(names[10 * (i / 10)], rec.u.ref.refname);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n \n \t\tblock_iter_close(&it);\n \t}\n@@ -111,13 +106,13 @@ static void test_block_read_write(void)\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-\tfor (i = 0; i < N; i++) {\n+\tfor (i = 0; i < N; i++)\n \t\treftable_free(names[i]);\n-\t}\n }\n \n-int block_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_block_read_write);\n-\treturn 0;\n+\tTEST(t_block_read_write(), \"read-write operations on blocks work\");\n+\n+\treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"500878","messageId":"20240814121122.4642-3-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 02/10] t-reftable-block: release used block reader","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:10Z","receivedAt":"2024-08-14T12:12:13Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Used block readers must be released using block_reader_release() to\nprevent the occurence of a memory leak. Make test_block_read_write()\nconform to this statement.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 8ab3ff9ebe..31d179a50a 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -103,6 +103,7 @@ static void t_block_read_write(void)\n \t\tblock_iter_close(&it);\n \t}\n \n+\tblock_reader_release(&br);\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-- \n2.45.GIT\n\n"},{"id":"500879","messageId":"20240814121122.4642-4-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 03/10] t-reftable-block: use reftable_record_equal() instead of check_str()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:11Z","receivedAt":"2024-08-14T12:12:15Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, operations like read and write for\nreftable blocks as defined by reftable/block.{c, h} are verified by\ncomparing only the keys of input and output reftable records. This is\nnot ideal because there can exist inequal reftable records with the\nsame key. Use the dedicated function for record comparison,\nreftable_record_equal() instead of key-based comparison.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 31d179a50a..baeb9c8b07 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -15,8 +15,8 @@ license that can be found in the LICENSE file or at\n static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n-\tchar *names[30];\n-\tconst size_t N = ARRAY_SIZE(names);\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n \tconst size_t block_size = 1024;\n \tstruct reftable_block block = { 0 };\n \tstruct block_writer bw = {\n@@ -47,11 +47,11 @@ static void t_block_read_write(void)\n \t\tchar name[100];\n \t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX , (uintmax_t)i);\n \n-\t\trec.u.ref.refname = name;\n+\t\trec.u.ref.refname = xstrdup(name);\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n-\t\tnames[i] = xstrdup(name);\n+\t\trecs[i] = rec;\n \t\tn = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n@@ -72,7 +72,7 @@ static void t_block_read_write(void)\n \t\tcheck_int(r, >=, 0);\n \t\tif (r > 0)\n \t\t\tbreak;\n-\t\tcheck_str(names[j], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n \t\tj++;\n \t}\n \n@@ -82,7 +82,7 @@ static void t_block_read_write(void)\n \tfor (i = 0; i < N; i++) {\n \t\tstruct block_iter it = BLOCK_ITER_INIT;\n \t\tstrbuf_reset(&want);\n-\t\tstrbuf_addstr(&want, names[i]);\n+\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n \n \t\tn = block_iter_seek_key(&it, &br, &want);\n \t\tcheck_int(n, ==, 0);\n@@ -90,7 +90,7 @@ static void t_block_read_write(void)\n \t\tn = block_iter_next(&it, &rec);\n \t\tcheck_int(n, ==, 0);\n \n-\t\tcheck_str(names[i], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n \n \t\twant.len--;\n \t\tn = block_iter_seek_key(&it, &br, &want);\n@@ -98,7 +98,7 @@ static void t_block_read_write(void)\n \n \t\tn = block_iter_next(&it, &rec);\n \t\tcheck_int(n, ==, 0);\n-\t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n \n \t\tblock_iter_close(&it);\n \t}\n@@ -108,7 +108,7 @@ static void t_block_read_write(void)\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n \tfor (i = 0; i < N; i++)\n-\t\treftable_free(names[i]);\n+\t\treftable_record_release(&recs[i]);\n }\n \n int cmd_main(int argc, const char *argv[])\n-- \n2.45.GIT\n\n"},{"id":"500880","messageId":"20240814121122.4642-5-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 04/10] t-reftable-block: use reftable_record_key() instead of strbuf_addstr()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:12Z","receivedAt":"2024-08-14T12:12:18Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, the record key required for many block\niterator functions is manually stored in a strbuf struct and then\npassed to these functions. This is not ideal when there exists a\ndedicated function to encode a record's key into a strbuf, namely\nreftable_record_key(). Use this function instead of manual encoding.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex baeb9c8b07..0d73fb98d6 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -81,8 +81,7 @@ static void t_block_read_write(void)\n \n \tfor (i = 0; i < N; i++) {\n \t\tstruct block_iter it = BLOCK_ITER_INIT;\n-\t\tstrbuf_reset(&want);\n-\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n+\t\treftable_record_key(&recs[i], &want);\n \n \t\tn = block_iter_seek_key(&it, &br, &want);\n \t\tcheck_int(n, ==, 0);\n-- \n2.45.GIT\n\n"},{"id":"500881","messageId":"20240814121122.4642-6-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 05/10] t-reftable-block: use block_iter_reset() instead of block_iter_close()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:13Z","receivedAt":"2024-08-14T12:12:21Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"block_iter_reset() restores a block iterator to its state at the time\nof initialization without freeing any memory while block_iter_close()\ndeallocates the memory for the iterator.\n\nIn the current testing setup, a block iterator is allocated and\ndeallocated for every iteration of a loop, which hurts performance.\nImprove upon this by using block_iter_reset() at the start of each\niteration instead. This has the added benifit of testing\nblock_iter_reset(), which currently remains untested.\n\nSimilarly, remove reftable_record_release() for a reftable record\nthat is still in use.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 8 ++------\n 1 file changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 0d73fb98d6..dfb7262a65 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -76,11 +76,8 @@ static void t_block_read_write(void)\n \t\tj++;\n \t}\n \n-\treftable_record_release(&rec);\n-\tblock_iter_close(&it);\n-\n \tfor (i = 0; i < N; i++) {\n-\t\tstruct block_iter it = BLOCK_ITER_INIT;\n+\t\tblock_iter_reset(&it);\n \t\treftable_record_key(&recs[i], &want);\n \n \t\tn = block_iter_seek_key(&it, &br, &want);\n@@ -98,11 +95,10 @@ static void t_block_read_write(void)\n \t\tn = block_iter_next(&it, &rec);\n \t\tcheck_int(n, ==, 0);\n \t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n-\n-\t\tblock_iter_close(&it);\n \t}\n \n \tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-- \n2.45.GIT\n\n"},{"id":"500882","messageId":"20240814121122.4642-7-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 06/10] t-reftable-block: use xstrfmt() instead of xstrdup()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:14Z","receivedAt":"2024-08-14T12:12:25Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Use xstrfmt() to assign a formatted string to a ref record's\nrefname instead of xstrdup(). This helps save the overhead of\na local 'char' buffer as well as makes the test more compact.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex dfb7262a65..d762980589 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -44,10 +44,7 @@ static void t_block_read_write(void)\n \tcheck_int(n, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n-\t\tchar name[100];\n-\t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX , (uintmax_t)i);\n-\n-\t\trec.u.ref.refname = xstrdup(name);\n+\t\trec.u.ref.refname = xstrfmt(\"branch%02\"PRIuMAX , (uintmax_t)i);\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n-- \n2.45.GIT\n\n"},{"id":"500883","messageId":"20240814121122.4642-8-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 07/10] t-reftable-block: remove unnecessary variable 'j'","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:15Z","receivedAt":"2024-08-14T12:12:29Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Currently, there are two variables for array indices, 'i' and 'j'.\nThe variable 'j' is used only once and can be easily replaced with\n'i'. Get rid of 'j' and replace its occurence with 'i'.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex d762980589..fa289e10f2 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -29,7 +29,6 @@ static void t_block_read_write(void)\n \tint n;\n \tstruct block_reader br = { 0 };\n \tstruct block_iter it = BLOCK_ITER_INIT;\n-\tsize_t j = 0;\n \tstruct strbuf want = STRBUF_INIT;\n \n \tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n@@ -64,13 +63,12 @@ static void t_block_read_write(void)\n \n \tblock_iter_seek_start(&it, &br);\n \n-\twhile (1) {\n+\tfor (i = 0; ; i++) {\n \t\tint r = block_iter_next(&it, &rec);\n \t\tcheck_int(r, >=, 0);\n \t\tif (r > 0)\n \t\t\tbreak;\n-\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n-\t\tj++;\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n \t}\n \n \tfor (i = 0; i < N; i++) {\n-- \n2.45.GIT\n\n"},{"id":"500884","messageId":"20240814121122.4642-9-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 08/10] t-reftable-block: add tests for log blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:16Z","receivedAt":"2024-08-14T12:12:32Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are only exercised\nfor ref blocks. Add another test that exercises these operations\nfor log blocks as well.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 90 ++++++++++++++++++++++++++++++++-\n 1 file changed, 88 insertions(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex fa289e10f2..01ef10e7a6 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -12,7 +12,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/constants.h\"\n #include \"reftable/reftable-error.h\"\n \n-static void t_block_read_write(void)\n+static void t_ref_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tstruct reftable_record recs[30];\n@@ -101,9 +101,95 @@ static void t_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_log_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 2048;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_LOG,\n+\t};\n+\tsize_t i = 0;\n+\tint n;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock.source = malloc_block_source();\n+\tblock_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\trec.u.log.refname = xstrfmt(\"branch%02\"PRIuMAX , (uintmax_t)i);\n+\t\trec.u.log.update_index = i;\n+\t\trec.u.log.value_type = REFTABLE_LOG_UPDATE;\n+\n+\t\trecs[i] = rec;\n+\t\tn = block_writer_add(&bw, &rec);\n+\t\trec.u.log.refname = NULL;\n+\t\trec.u.log.value_type = REFTABLE_LOG_DELETION;\n+\t\tcheck_int(n, ==, 0);\n+\t}\n+\n+\tn = block_writer_finish(&bw);\n+\tcheck_int(n, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tint r = block_iter_next(&it, &rec);\n+\t\tcheck_int(r, >=, 0);\n+\t\tif (r > 0)\n+\t\t\tbreak;\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\tstrbuf_reset(&want);\n+\t\tstrbuf_addstr(&want, recs[i].u.log.refname);\n+\n+\t\tn = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tn = block_iter_next(&it, &rec);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\n+\t\twant.len--;\n+\t\tn = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tn = block_iter_next(&it, &rec);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n-\tTEST(t_block_read_write(), \"read-write operations on blocks work\");\n+\tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n+\tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"500885","messageId":"20240814121122.4642-10-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 09/10] t-reftable-block: add tests for obj blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:17Z","receivedAt":"2024-08-14T12:12:34Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are left unexercised\nfor obj blocks. Add a test that exercises these operations for obj\nblocks.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 79 +++++++++++++++++++++++++++++++++\n 1 file changed, 79 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 01ef10e7a6..34d37fe1a7 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -186,9 +186,88 @@ static void t_log_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_obj_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_OBJ,\n+\t};\n+\tsize_t i = 0;\n+\tint n;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock.source = malloc_block_source();\n+\tblock_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tuint8_t *bytes = reftable_malloc(sizeof(uint8_t[5]));\n+\t\tmemcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5]));\n+\n+\t\trec.u.obj.hash_prefix = bytes;\n+\t\trec.u.obj.hash_prefix_len = 5;\n+\n+\t\trecs[i] = rec;\n+\t\tn = block_writer_add(&bw, &rec);\n+\t\trec.u.obj.hash_prefix = NULL;\n+\t\trec.u.obj.hash_prefix_len = 0;\n+\t\tcheck_int(n, ==, 0);\n+\t}\n+\n+\tn = block_writer_finish(&bw);\n+\tcheck_int(n, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tint r = block_iter_next(&it, &rec);\n+\t\tcheck_int(r, >=, 0);\n+\t\tif (r > 0)\n+\t\t\tbreak;\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\treftable_record_key(&recs[i], &want);\n+\n+\t\tn = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tn = block_iter_next(&it, &rec);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n \tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n+\tTEST(t_obj_block_read_write(), \"read-write operations on obj blocks work\");\n \tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n \n \treturn test_done();\n-- \n2.45.GIT\n\n"},{"id":"500886","messageId":"20240814121122.4642-11-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[PATCH 10/10] t-reftable-block: add tests for index blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-14T12:03:18Z","receivedAt":"2024-08-14T12:12:38Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are left unexercised\nfor index blocks. Add a test that exercises these operations for\nindex blocks.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 85 +++++++++++++++++++++++++++++++++\n 1 file changed, 85 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 34d37fe1a7..bbf6a5371e 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -264,8 +264,93 @@ static void t_obj_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_index_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_INDEX,\n+\t\t.u.idx.last_key = STRBUF_INIT,\n+\t};\n+\tsize_t i = 0;\n+\tint n;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock.source = malloc_block_source();\n+\tblock_writer_init(&bw, BLOCK_TYPE_INDEX, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tstrbuf_init(&recs[i].u.idx.last_key, 9);\n+\n+\t\trecs[i].type = BLOCK_TYPE_INDEX;\n+\t\tstrbuf_addf(&recs[i].u.idx.last_key, \"branch%02\"PRIuMAX, (uintmax_t)i);\n+\t\trecs[i].u.idx.offset = i;\n+\n+\t\tn = block_writer_add(&bw, &recs[i]);\n+\t\tcheck_int(n, ==, 0);\n+\t}\n+\n+\tn = block_writer_finish(&bw);\n+\tcheck_int(n, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tint r = block_iter_next(&it, &rec);\n+\t\tcheck_int(r, >=, 0);\n+\t\tif (r > 0)\n+\t\t\tbreak;\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\treftable_record_key(&recs[i], &want);\n+\n+\t\tn = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tn = block_iter_next(&it, &rec);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\n+\t\twant.len--;\n+\t\tn = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(n, ==, 0);\n+\n+\t\tn = block_iter_next(&it, &rec);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n+\tTEST(t_index_block_read_write(), \"read-write operations on index blocks work\");\n \tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n \tTEST(t_obj_block_read_write(), \"read-write operations on obj blocks work\");\n \tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n-- \n2.45.GIT\n\n"},{"id":"500964","messageId":"Zr3NOUA-Ok0wKodL@tanuki","threadId":"61947","inReplyTo":"20240814121122.4642-11-chandrapratap3519@gmail.com","subject":"Re: [PATCH 10/10] t-reftable-block: add tests for index blocks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-15T09:41:13Z","receivedAt":"2024-08-15T09:51:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 14, 2024 at 05:33:18PM +0530, Chandra Pratap wrote:\n> In the current testing setup, block operations are left unexercised\n> for index blocks. Add a test that exercises these operations for\n> index blocks.\n\nAgain, the same remarks as for the preceding two patches apply here,\ntoo.\n\nPatrick\n"},{"id":"500965","messageId":"Zr3NGugrgjrJlPUO@tanuki","threadId":"61947","inReplyTo":"20240814121122.4642-3-chandrapratap3519@gmail.com","subject":"Re: [PATCH 02/10] t-reftable-block: release used block reader","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-15T09:40:42Z","receivedAt":"2024-08-15T09:51:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 14, 2024 at 05:33:10PM +0530, Chandra Pratap wrote:\n> Used block readers must be released using block_reader_release() to\n> prevent the occurence of a memory leak. Make test_block_read_write()\n> conform to this statement.\n\nInteresting. Didn't the old tests run with the leak checker enabled?\n\nPatrick\n"},{"id":"500966","messageId":"Zr3NNBTXNXSCrTpJ@tanuki","threadId":"61947","inReplyTo":"20240814121122.4642-10-chandrapratap3519@gmail.com","subject":"Re: [PATCH 09/10] t-reftable-block: add tests for obj blocks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-15T09:41:08Z","receivedAt":"2024-08-15T09:51:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 14, 2024 at 05:33:17PM +0530, Chandra Pratap wrote:\n> In the current testing setup, block operations are left unexercised\n> for obj blocks. Add a test that exercises these operations for obj\n> blocks.\n\nSame remarks here as for the preceding commit.\n\n> @@ -186,9 +186,88 @@ static void t_log_block_read_write(void)\n> \t\treftable_record_release(&recs[i]);\n> }\n> \n> +static void t_obj_block_read_write(void)\n> +{\n> +\tconst int header_off = 21;\n> +\tstruct reftable_record recs[30];\n> +\tconst size_t N = ARRAY_SIZE(recs);\n> +\tconst size_t block_size = 1024;\n> +\tstruct reftable_block block = { 0 };\n> +\tstruct block_writer bw = {\n> +\t\t.last_key = STRBUF_INIT,\n> +\t};\n> +\tstruct reftable_record rec = {\n> +\t\t.type = BLOCK_TYPE_OBJ,\n> +\t};\n> +\tsize_t i = 0;\n> +\tint n;\n> +\tstruct block_reader br = { 0 };\n> +\tstruct block_iter it = BLOCK_ITER_INIT;\n> +\tstruct strbuf want = STRBUF_INIT;\n> +\n> +\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n> +\tblock.len = block_size;\n> +\tblock.source = malloc_block_source();\n> +\tblock_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n> +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> +\n> +\tfor (i = 0; i < N; i++) {\n> +\t\tuint8_t *bytes = reftable_malloc(sizeof(uint8_t[5]));\n> +\t\tmemcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5]));\n\nFrom the top of my head I'm not sure whether we use inline-array\ndeclarations like this anywhere. I'd rather just make it a separate\nvariable, which also allows us to get rid of the magic 5 via\n`ARRAY_SIZE()`.\n\nPatrick\n"},{"id":"500979","messageId":"Zr3NMOQWYDOufHXg@tanuki","threadId":"61947","inReplyTo":"20240814121122.4642-9-chandrapratap3519@gmail.com","subject":"Re: [PATCH 08/10] t-reftable-block: add tests for log blocks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-15T09:41:04Z","receivedAt":"2024-08-15T11:22:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 14, 2024 at 05:33:16PM +0530, Chandra Pratap wrote:\n> @@ -101,9 +101,95 @@ static void t_block_read_write(void)\n> \t\treftable_record_release(&recs[i]);\n> }\n> \n> +static void t_log_block_read_write(void)\n> +{\n> +\tconst int header_off = 21;\n> +\tstruct reftable_record recs[30];\n> +\tconst size_t N = ARRAY_SIZE(recs);\n> +\tconst size_t block_size = 2048;\n> +\tstruct reftable_block block = { 0 };\n> +\tstruct block_writer bw = {\n> +\t\t.last_key = STRBUF_INIT,\n> +\t};\n> +\tstruct reftable_record rec = {\n> +\t\t.type = BLOCK_TYPE_LOG,\n> +\t};\n> +\tsize_t i = 0;\n> +\tint n;\n> +\tstruct block_reader br = { 0 };\n> +\tstruct block_iter it = BLOCK_ITER_INIT;\n> +\tstruct strbuf want = STRBUF_INIT;\n> +\n> +\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n> +\tblock.len = block_size;\n> +\tblock.source = malloc_block_source();\n> +\tblock_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n> +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> +\n> +\tfor (i = 0; i < N; i++) {\n> +\t\trec.u.log.refname = xstrfmt(\"branch%02\"PRIuMAX , (uintmax_t)i);\n> +\t\trec.u.log.update_index = i;\n> +\t\trec.u.log.value_type = REFTABLE_LOG_UPDATE;\n> +\n> +\t\trecs[i] = rec;\n> +\t\tn = block_writer_add(&bw, &rec);\n> +\t\trec.u.log.refname = NULL;\n> +\t\trec.u.log.value_type = REFTABLE_LOG_DELETION;\n> +\t\tcheck_int(n, ==, 0);\n> +\t}\n> +\n> +\tn = block_writer_finish(&bw);\n> +\tcheck_int(n, >, 0);\n\nDo we maybe want to rename `n` to `ret`? That's way more customary in\nour codebase.\n\n> +\tblock_writer_release(&bw);\n> +\n> +\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n> +\n> +\tblock_iter_seek_start(&it, &br);\n> +\n> +\tfor (i = 0; ; i++) {\n> +\t\tint r = block_iter_next(&it, &rec);\n> +\t\tcheck_int(r, >=, 0);\n> +\t\tif (r > 0)\n> +\t\t\tbreak;\n\nWe can also reuse `n` (or `ret`) here, right?\n\n> +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n> +\t}\n\nOne thing that this loop doesn't verify is whether we actually got the\nexpected number of log records. It could be that the first iteration\nalready returns `r > 0`, which is not our expectation. So we should\nlikely add a check for `i == N` after the loop.\n\nPatrick\n"},{"id":"500981","messageId":"Zr3NJCRQ7W8KjXxb@tanuki","threadId":"61947","inReplyTo":"20240814121122.4642-5-chandrapratap3519@gmail.com","subject":"Re: [PATCH 04/10] t-reftable-block: use reftable_record_key() instead of strbuf_addstr()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-15T09:40:52Z","receivedAt":"2024-08-15T11:22:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 14, 2024 at 05:33:12PM +0530, Chandra Pratap wrote:\n> In the current testing setup, the record key required for many block\n> iterator functions is manually stored in a strbuf struct and then\n> passed to these functions. This is not ideal when there exists a\n> dedicated function to encode a record's key into a strbuf, namely\n> reftable_record_key(). Use this function instead of manual encoding.\n> \n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n> ---\n> t/unit-tests/t-reftable-block.c | 3 +--\n> 1 file changed, 1 insertion(+), 2 deletions(-)\n> \n> diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\n> index baeb9c8b07..0d73fb98d6 100644\n> --- a/t/unit-tests/t-reftable-block.c\n> +++ b/t/unit-tests/t-reftable-block.c\n> @@ -81,8 +81,7 @@ static void t_block_read_write(void)\n> \n> \tfor (i = 0; i < N; i++) {\n> \t\tstruct block_iter it = BLOCK_ITER_INIT;\n> -\t\tstrbuf_reset(&want);\n> -\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n> +\t\treftable_record_key(&recs[i], &want);\n> \n> \t\tn = block_iter_seek_key(&it, &br, &want);\n> \t\tcheck_int(n, ==, 0);\n\nYup, for ref records this is equivalent indeed, as their key only\nconsists of of the refname. It would be different for log records, which\nalso include the log index.\n\nPatrick\n"},{"id":"500990","messageId":"Zr3NHmKfslOzXlUY@tanuki","threadId":"61947","inReplyTo":"20240814121122.4642-4-chandrapratap3519@gmail.com","subject":"Re: [PATCH 03/10] t-reftable-block: use reftable_record_equal() instead of check_str()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-15T09:40:46Z","receivedAt":"2024-08-15T11:22:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 14, 2024 at 05:33:11PM +0530, Chandra Pratap wrote:\n> In the current testing setup, operations like read and write for\n> reftable blocks as defined by reftable/block.{c, h} are verified by\n> comparing only the keys of input and output reftable records. This is\n> not ideal because there can exist inequal reftable records with the\n> same key. Use the dedicated function for record comparison,\n> reftable_record_equal() instead of key-based comparison.\n\nNit: there should probably be a comma after the closing brace.\n\n> diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\n> index 31d179a50a..baeb9c8b07 100644\n> --- a/t/unit-tests/t-reftable-block.c\n> +++ b/t/unit-tests/t-reftable-block.c\n> @@ -15,8 +15,8 @@ license that can be found in the LICENSE file or at\n> static void t_block_read_write(void)\n> {\n> \tconst int header_off = 21; /* random */\n> -\tchar *names[30];\n> -\tconst size_t N = ARRAY_SIZE(names);\n> +\tstruct reftable_record recs[30];\n> +\tconst size_t N = ARRAY_SIZE(recs);\n> \tconst size_t block_size = 1024;\n> \tstruct reftable_block block = { 0 };\n> \tstruct block_writer bw = {\n> @@ -47,11 +47,11 @@ static void t_block_read_write(void)\n> \t\tchar name[100];\n> \t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX , (uintmax_t)i);\n> \n> -\t\trec.u.ref.refname = name;\n> +\t\trec.u.ref.refname = xstrdup(name);\n> \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n> \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n> \n> -\t\tnames[i] = xstrdup(name);\n> +\t\trecs[i] = rec;\n> \t\tn = block_writer_add(&bw, &rec);\n> \t\trec.u.ref.refname = NULL;\n> \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n> @@ -72,7 +72,7 @@ static void t_block_read_write(void)\n> \t\tcheck_int(r, >=, 0);\n> \t\tif (r > 0)\n> \t\t\tbreak;\n> -\t\tcheck_str(names[j], rec.u.ref.refname);\n> +\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n> \t\tj++;\n> \t}\n\nOkay. Because we're not only checking for the refname anymore, we now\nneed to store the expected records as full records, which also requires\nus to allocate the refname. Makes sense.\n\n> @@ -90,7 +90,7 @@ static void t_block_read_write(void)\n> \t\tn = block_iter_next(&it, &rec);\n> \t\tcheck_int(n, ==, 0);\n> \n> -\t\tcheck_str(names[i], rec.u.ref.refname);\n> +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n> \n> \t\twant.len--;\n> \t\tn = block_iter_seek_key(&it, &br, &want);\n\nIt would of course be great if we didn't only verify that SHA1 works as\nexpected, but that we can also read and write SHA256 records. But that\nwould be a new addition to the test suite that doesn't have to be part\nof this patch series.\n\nPatrick\n"},{"id":"500993","messageId":"Zr3NKWOFlZlkkOSn@tanuki","threadId":"61947","inReplyTo":"20240814121122.4642-6-chandrapratap3519@gmail.com","subject":"Re: [PATCH 05/10] t-reftable-block: use block_iter_reset() instead of block_iter_close()","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-15T09:40:57Z","receivedAt":"2024-08-15T11:22:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 14, 2024 at 05:33:13PM +0530, Chandra Pratap wrote:\n> block_iter_reset() restores a block iterator to its state at the time\n> of initialization without freeing any memory while block_iter_close()\n> deallocates the memory for the iterator.\n> \n> In the current testing setup, a block iterator is allocated and\n> deallocated for every iteration of a loop, which hurts performance.\n> Improve upon this by using block_iter_reset() at the start of each\n> iteration instead. This has the added benifit of testing\n> block_iter_reset(), which currently remains untested.\n\nI don't think that performance is a good argument, but exercising the\nreset function certainly is.\n\n> Similarly, remove reftable_record_release() for a reftable record\n> that is still in use.\n\nThis is a welcome change, too, to verify that reading into the same\nrecord multiple times does not leak memory and otherwise works as\nexpected.\n\nPatrick\n"},{"id":"501032","messageId":"CA+J6zkT-aXhB7p-iBxVErAJ4jYtk9t98YGuCWgu82LX_3WUvyg@mail.gmail.com","threadId":"61947","inReplyTo":"Zr3NGugrgjrJlPUO@tanuki","subject":"Re: [PATCH 02/10] t-reftable-block: release used block reader","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-15T18:22:55Z","receivedAt":"2024-08-15T18:23:23Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"On Thu, 15 Aug 2024 at 15:10, Patrick Steinhardt wrote:\n>\n> On Wed, Aug 14, 2024 at 05:33:10PM +0530, Chandra Pratap wrote:\n> > Used block readers must be released using block_reader_release() to\n> > prevent the occurence of a memory leak. Make test_block_read_write()\n> > conform to this statement.\n>\n> Interesting. Didn't the old tests run with the leak checker enabled?\n\nI think not, I was able to find this error due to the GitHub CI.\n"},{"id":"501033","messageId":"CA+J6zkSOF-RtrBvgunMsTdL4Qd8H10zUHesXYtYG8DW+kJht-w@mail.gmail.com","threadId":"61947","inReplyTo":"Zr3NMOQWYDOufHXg@tanuki","subject":"Re: [PATCH 08/10] t-reftable-block: add tests for log blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-15T18:36:47Z","receivedAt":"2024-08-15T18:37:15Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"On Thu, 15 Aug 2024 at 15:11, Patrick Steinhardt wrote:\n>\n> On Wed, Aug 14, 2024 at 05:33:16PM +0530, Chandra Pratap wrote:\n> > @@ -101,9 +101,95 @@ static void t_block_read_write(void)\n> > reftable_record_release(&recs[i]);\n> > }\n> >\n> > +static void t_log_block_read_write(void)\n> > +{\n> > + const int header_off = 21;\n> > + struct reftable_record recs[30];\n> > + const size_t N = ARRAY_SIZE(recs);\n> > + const size_t block_size = 2048;\n> > + struct reftable_block block = { 0 };\n> > + struct block_writer bw = {\n> > + .last_key = STRBUF_INIT,\n> > + };\n> > + struct reftable_record rec = {\n> > + .type = BLOCK_TYPE_LOG,\n> > + };\n> > + size_t i = 0;\n> > + int n;\n> > + struct block_reader br = { 0 };\n> > + struct block_iter it = BLOCK_ITER_INIT;\n> > + struct strbuf want = STRBUF_INIT;\n> > +\n> > + REFTABLE_CALLOC_ARRAY(block.data, block_size);\n> > + block.len = block_size;\n> > + block.source = malloc_block_source();\n> > + block_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n> > + header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> > +\n> > + for (i = 0; i < N; i++) {\n> > + rec.u.log.refname = xstrfmt(\"branch%02\"PRIuMAX , (uintmax_t)i);\n> > + rec.u.log.update_index = i;\n> > + rec.u.log.value_type = REFTABLE_LOG_UPDATE;\n> > +\n> > + recs[i] = rec;\n> > + n = block_writer_add(&bw, &rec);\n> > + rec.u.log.refname = NULL;\n> > + rec.u.log.value_type = REFTABLE_LOG_DELETION;\n> > + check_int(n, ==, 0);\n> > + }\n> > +\n> > + n = block_writer_finish(&bw);\n> > + check_int(n, >, 0);\n>\n> Do we maybe want to rename `n` to `ret`? That's way more customary in\n> our codebase.\n\nSure thing, but then I would want to change the existing test (which gets\nrenamed as t_ref_block_read_write) and I'm unsure of which patch would\nbe the most suitable for that change. Would it be fine to include that\nchange as a part of this patch?\n\n> > + block_writer_release(&bw);\n> > +\n> > + block_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n> > +\n> > + block_iter_seek_start(&it, &br);\n> > +\n> > + for (i = 0; ; i++) {\n> > + int r = block_iter_next(&it, &rec);\n> > + check_int(r, >=, 0);\n> > + if (r > 0)\n> > + break;\n>\n> We can also reuse `n` (or `ret`) here, right?\n>\n> > + check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n> > + }\n>\n> One thing that this loop doesn't verify is whether we actually got the\n> expected number of log records. It could be that the first iteration\n> already returns `r > 0`, which is not our expectation. So we should\n> likely add a check for `i == N` after the loop.\n\nWhat about something like\nif (r > 0) {\n check_int(i, ==, N);\n break;\n}\nThat should achieve the same results if I'm not wrong.\n"},{"id":"501039","messageId":"CA+J6zkRHZ3NF8QV_oFBVEO8gCy-Pvf6zerdiY7aKCqkofRPktA@mail.gmail.com","threadId":"61947","inReplyTo":"Zr3NNBTXNXSCrTpJ@tanuki","subject":"Re: [PATCH 09/10] t-reftable-block: add tests for obj blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-15T19:11:34Z","receivedAt":"2024-08-15T19:12:02Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"On Thu, 15 Aug 2024 at 15:11, Patrick Steinhardt wrote:\n>\n> On Wed, Aug 14, 2024 at 05:33:17PM +0530, Chandra Pratap wrote:\n> > In the current testing setup, block operations are left unexercised\n> > for obj blocks. Add a test that exercises these operations for obj\n> > blocks.\n>\n> Same remarks here as for the preceding commit.\n>\n> > @@ -186,9 +186,88 @@ static void t_log_block_read_write(void)\n> > reftable_record_release(&recs[i]);\n> > }\n> >\n> > +static void t_obj_block_read_write(void)\n> > +{\n> > + const int header_off = 21;\n> > + struct reftable_record recs[30];\n> > + const size_t N = ARRAY_SIZE(recs);\n> > + const size_t block_size = 1024;\n> > + struct reftable_block block = { 0 };\n> > + struct block_writer bw = {\n> > + .last_key = STRBUF_INIT,\n> > + };\n> > + struct reftable_record rec = {\n> > + .type = BLOCK_TYPE_OBJ,\n> > + };\n> > + size_t i = 0;\n> > + int n;\n> > + struct block_reader br = { 0 };\n> > + struct block_iter it = BLOCK_ITER_INIT;\n> > + struct strbuf want = STRBUF_INIT;\n> > +\n> > + REFTABLE_CALLOC_ARRAY(block.data, block_size);\n> > + block.len = block_size;\n> > + block.source = malloc_block_source();\n> > + block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n> > + header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> > +\n> > + for (i = 0; i < N; i++) {\n> > + uint8_t *bytes = reftable_malloc(sizeof(uint8_t[5]));\n> > + memcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5]));\n>\n> From the top of my head I'm not sure whether we use inline-array\n> declarations like this anywhere. I'd rather just make it a separate\n> variable, which also allows us to get rid of the magic 5 via\n> `ARRAY_SIZE()`.\n\nWe _do_ use inline array declarations like this, here's an example from\nt/unit-tests/t-prio-queue.c:\nTEST(TEST_INPUT(((int []){ STACK, 1, 2, 3, 4, 5, 6, REVERSE, DUMP }),\n ((int []){ 1, 2, 3, 4, 5, 6 })), \"prio-queue works when LIFO\nstack is reversed\");\n\nI did implement bytes[] as a local variable array when I first worked\non this patch but that turned out to be tricky due to variable scoping\nand pointer semantics, so I ultimately settled on this approach.\n"},{"id":"501097","messageId":"Zr8C0ir5UHzaHfUq@tanuki","threadId":"61947","inReplyTo":"CA+J6zkSOF-RtrBvgunMsTdL4Qd8H10zUHesXYtYG8DW+kJht-w@mail.gmail.com","subject":"Re: [PATCH 08/10] t-reftable-block: add tests for log blocks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-16T07:42:10Z","receivedAt":"2024-08-16T07:42:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Aug 16, 2024 at 12:06:47AM +0530, Chandra Pratap wrote:\n> On Thu, 15 Aug 2024 at 15:11, Patrick Steinhardt wrote:\n> >\n> > On Wed, Aug 14, 2024 at 05:33:16PM +0530, Chandra Pratap wrote:\n> > > @@ -101,9 +101,95 @@ static void t_block_read_write(void)\n> > > reftable_record_release(&recs[i]);\n> > > }\n> > >\n> > > +static void t_log_block_read_write(void)\n> > > +{\n> > > + const int header_off = 21;\n> > > + struct reftable_record recs[30];\n> > > + const size_t N = ARRAY_SIZE(recs);\n> > > + const size_t block_size = 2048;\n> > > + struct reftable_block block = { 0 };\n> > > + struct block_writer bw = {\n> > > + .last_key = STRBUF_INIT,\n> > > + };\n> > > + struct reftable_record rec = {\n> > > + .type = BLOCK_TYPE_LOG,\n> > > + };\n> > > + size_t i = 0;\n> > > + int n;\n> > > + struct block_reader br = { 0 };\n> > > + struct block_iter it = BLOCK_ITER_INIT;\n> > > + struct strbuf want = STRBUF_INIT;\n> > > +\n> > > + REFTABLE_CALLOC_ARRAY(block.data, block_size);\n> > > + block.len = block_size;\n> > > + block.source = malloc_block_source();\n> > > + block_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n> > > + header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> > > +\n> > > + for (i = 0; i < N; i++) {\n> > > + rec.u.log.refname = xstrfmt(\"branch%02\"PRIuMAX , (uintmax_t)i);\n> > > + rec.u.log.update_index = i;\n> > > + rec.u.log.value_type = REFTABLE_LOG_UPDATE;\n> > > +\n> > > + recs[i] = rec;\n> > > + n = block_writer_add(&bw, &rec);\n> > > + rec.u.log.refname = NULL;\n> > > + rec.u.log.value_type = REFTABLE_LOG_DELETION;\n> > > + check_int(n, ==, 0);\n> > > + }\n> > > +\n> > > + n = block_writer_finish(&bw);\n> > > + check_int(n, >, 0);\n> >\n> > Do we maybe want to rename `n` to `ret`? That's way more customary in\n> > our codebase.\n> \n> Sure thing, but then I would want to change the existing test (which gets\n> renamed as t_ref_block_read_write) and I'm unsure of which patch would\n> be the most suitable for that change. Would it be fine to include that\n> change as a part of this patch?\n\nThe way I'd do it is to first do the minimum required changes to port\nthe old test to the new testing framework, without any of the cleanups\nto align code style. Then I'd put another commit on top that does all\nthe changes like removing braces, converting types and also adapting\nnames.\n\n> > > + block_writer_release(&bw);\n> > > +\n> > > + block_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n> > > +\n> > > + block_iter_seek_start(&it, &br);\n> > > +\n> > > + for (i = 0; ; i++) {\n> > > + int r = block_iter_next(&it, &rec);\n> > > + check_int(r, >=, 0);\n> > > + if (r > 0)\n> > > + break;\n> >\n> > We can also reuse `n` (or `ret`) here, right?\n> >\n> > > + check(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n> > > + }\n> >\n> > One thing that this loop doesn't verify is whether we actually got the\n> > expected number of log records. It could be that the first iteration\n> > already returns `r > 0`, which is not our expectation. So we should\n> > likely add a check for `i == N` after the loop.\n> \n> What about something like\n> if (r > 0) {\n> check_int(i, ==, N);\n> break;\n> }\n> That should achieve the same results if I'm not wrong.\n\nYup, looks good to me.\n\nPatrick\n"},{"id":"501098","messageId":"Zr8Dcvfg01f3mKd9@tanuki","threadId":"61947","inReplyTo":"CA+J6zkRHZ3NF8QV_oFBVEO8gCy-Pvf6zerdiY7aKCqkofRPktA@mail.gmail.com","subject":"Re: [PATCH 09/10] t-reftable-block: add tests for obj blocks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-16T07:44:56Z","receivedAt":"2024-08-16T07:45:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Aug 16, 2024 at 12:41:34AM +0530, Chandra Pratap wrote:\n> On Thu, 15 Aug 2024 at 15:11, Patrick Steinhardt wrote:\n> >\n> > On Wed, Aug 14, 2024 at 05:33:17PM +0530, Chandra Pratap wrote:\n> > > In the current testing setup, block operations are left unexercised\n> > > for obj blocks. Add a test that exercises these operations for obj\n> > > blocks.\n> >\n> > Same remarks here as for the preceding commit.\n> >\n> > > @@ -186,9 +186,88 @@ static void t_log_block_read_write(void)\n> > > reftable_record_release(&recs[i]);\n> > > }\n> > >\n> > > +static void t_obj_block_read_write(void)\n> > > +{\n> > > + const int header_off = 21;\n> > > + struct reftable_record recs[30];\n> > > + const size_t N = ARRAY_SIZE(recs);\n> > > + const size_t block_size = 1024;\n> > > + struct reftable_block block = { 0 };\n> > > + struct block_writer bw = {\n> > > + .last_key = STRBUF_INIT,\n> > > + };\n> > > + struct reftable_record rec = {\n> > > + .type = BLOCK_TYPE_OBJ,\n> > > + };\n> > > + size_t i = 0;\n> > > + int n;\n> > > + struct block_reader br = { 0 };\n> > > + struct block_iter it = BLOCK_ITER_INIT;\n> > > + struct strbuf want = STRBUF_INIT;\n> > > +\n> > > + REFTABLE_CALLOC_ARRAY(block.data, block_size);\n> > > + block.len = block_size;\n> > > + block.source = malloc_block_source();\n> > > + block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n> > > + header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> > > +\n> > > + for (i = 0; i < N; i++) {\n> > > + uint8_t *bytes = reftable_malloc(sizeof(uint8_t[5]));\n> > > + memcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5]));\n> >\n> > From the top of my head I'm not sure whether we use inline-array\n> > declarations like this anywhere. I'd rather just make it a separate\n> > variable, which also allows us to get rid of the magic 5 via\n> > `ARRAY_SIZE()`.\n> \n> We _do_ use inline array declarations like this, here's an example from\n> t/unit-tests/t-prio-queue.c:\n> TEST(TEST_INPUT(((int []){ STACK, 1, 2, 3, 4, 5, 6, REVERSE, DUMP }),\n> ((int []){ 1, 2, 3, 4, 5, 6 })), \"prio-queue works when LIFO\n> stack is reversed\");\n> \n> I did implement bytes[] as a local variable array when I first worked\n> on this patch but that turned out to be tricky due to variable scoping\n> and pointer semantics, so I ultimately settled on this approach.\n\nOh, I didn't mean to say that you should _only_ use the local array.\nRather something like this:\n\n uint8_t[] bytes = { i, i + 1, i + 2, i + 3, i + 5 }, *allocated;\n DUP_ARRAY(allocated, bytes, ARRAY_SIZE(bytes));\n\nPatrick\n"},{"id":"501147","messageId":"20240816175414.5169-1-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240814121122.4642-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v2 00/11] t: port reftable/block_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:23Z","receivedAt":"2024-08-16T17:54:53Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"The reftable library comes with self tests, which are exercised\nas part of the usual end-to-end tests and are designed to\nobserve the end-user visible effects of Git commands. What it\nexercises, however, is a better match for the unit-testing\nframework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests',\n2023-12-09), which is designed to observe how low level\nimplementation details, at the level of sequences of individual\nfunction calls, behave.\n\nHence, port reftable/block_test.c to the unit testing framework and\nimprove upon the ported test. The first patch in the series moves\nthe test to the unit testing framework, and the rest of the patches\nimprove upon the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n\n---\nChanges in v2:\n- Split the first patch into two, one that moves the test to the\n unit testing framework and another that adds improvements.\n- Fix a comma-error in the commit message of patch 4\n (previously patch 3).\n- Use 'ret' as the name for a variable that stores return codes\n rather than 'n' in all the tests.\n- Add a check for 'i == N' in the block iterator loop in all\n the tests.\n- Modify the obj test (patch 10) to no longer use magic numbers.\n- Rebase the branch on top of the latest 'master' branch.\n\nCI/PR: https://github.com/gitgitgadget/git/pull/1749\n\nChandra Pratap(11):\nt: move reftable/block_test.c to the unit testing framework\nt: harmonize t-reftable-block.c with coding guidelines\nt-reftable-block: release used block reader\nt-reftable-block: use reftable_record_equal() instead of check_str()\nt-reftable-block: use reftable_record_key() instead of strbuf_addstr()\nt-reftable-block: use block_iter_reset() instead of block_iter_close()\nt-reftable-block: use xstrfmt() instead of xstrdup()\nt-reftable-block: remove unnecessary variable 'j'\nt-reftable-block: add tests for log blocks\nt-reftable-block: add tests for obj blocks\nt-reftable-block: add tests for index blocks\n\nMakefile | 2 +-\nreftable/block_test.c | 123 ------------------------------\nreftable/reftable-tests.h | 1 -\nt/helper/test-reftable.c | 1 -\nt/unit-tests/t-reftable-block.c | 368 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n5 files changed, 369 insertions(+), 126 deletions(-)\n\nRange-diff against v2:\n\n 1: 0712c3df22 ! 1: c0d8c4149f t: move reftable/block_test.c to the unit testing framework\n @@ Commit message\n framework and renaming the tests to follow the unit-tests'\n naming conventions.\n\n - While at it, ensure structs are 0-initialized with '= { 0 }'\n - instead of '= { NULL }' and array indices have type 'size_t'\n - instead of 'int'.\n -\n Mentored-by: Patrick Steinhardt \n Mentored-by: Christian Couder \n Signed-off-by: Chandra Pratap \n @@ t/unit-tests/t-reftable-block.c: license that can be found in the LICENSE file o\n {\n \tconst int header_off = 21; /* random */\n \tchar *names[30];\n --\tconst int N = ARRAY_SIZE(names);\n --\tconst int block_size = 1024;\n --\tstruct reftable_block block = { NULL };\n -+\tconst size_t N = ARRAY_SIZE(names);\n -+\tconst size_t block_size = 1024;\n -+\tstruct reftable_block block = { 0 };\n - \tstruct block_writer bw = {\n - \t\t.last_key = STRBUF_INIT,\n - \t};\n - \tstruct reftable_record rec = {\n - \t\t.type = BLOCK_TYPE_REF,\n - \t};\n --\tint i = 0;\n -+\tsize_t i = 0;\n - \tint n;\n - \tstruct block_reader br = { 0 };\n - \tstruct block_iter it = BLOCK_ITER_INIT;\n --\tint j = 0;\n -+\tsize_t j = 0;\n - \tstruct strbuf want = STRBUF_INIT;\n -\n - \tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n @@ t/unit-tests/t-reftable-block.c: static void test_block_read_write(void)\n \trec.u.ref.refname = (char *) \"\";\n \trec.u.ref.value_type = REFTABLE_REF_DELETION;\n @@ t/unit-tests/t-reftable-block.c: static void test_block_read_write(void)\n\n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n --\t\tsnprintf(name, sizeof(name), \"branch%02d\", i);\n -+\t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX , (uintmax_t)i);\n -\n - \t\trec.u.ref.refname = name;\n - \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n @@ t/unit-tests/t-reftable-block.c: static void test_block_read_write(void)\n \t\tn = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n @@ t/unit-tests/t-reftable-block.c: static void test_block_read_write(void)\n \twhile (1) {\n \t\tint r = block_iter_next(&it, &rec);\n -\t\tEXPECT(r >= 0);\n --\t\tif (r > 0) {\n +\t\tcheck_int(r, >=, 0);\n -+\t\tif (r > 0)\n + \t\tif (r > 0) {\n \t\t\tbreak;\n --\t\t}\n + \t\t}\n -\t\tEXPECT_STREQ(names[j], rec.u.ref.refname);\n +\t\tcheck_str(names[j], rec.u.ref.refname);\n \t\tj++;\n @@ t/unit-tests/t-reftable-block.c: static void test_block_read_write(void)\n \t\tblock_iter_close(&it);\n \t}\n @@ t/unit-tests/t-reftable-block.c: static void test_block_read_write(void)\n - \treftable_record_release(&rec);\n - \treftable_block_done(&br.block);\n - \tstrbuf_release(&want);\n --\tfor (i = 0; i < N; i++) {\n -+\tfor (i = 0; i < N; i++)\n - \t\treftable_free(names[i]);\n --\t}\n + \t}\n }\n\n -int block_test_main(int argc, const char *argv[])\n -: ---------- > 2: 017658ddb3 t: harmonize t-reftable-block.c with coding guidelines\n 2: 3282dbeba1 = 3: 2042e2fde0 t-reftable-block: release used block reader\n 3: 5a3fea003a ! 4: 853efb0c0b t-reftable-block: use reftable_record_equal() instead of check_str()\n @@ Commit message\n comparing only the keys of input and output reftable records. This is\n not ideal because there can exist inequal reftable records with the\n same key. Use the dedicated function for record comparison,\n - reftable_record_equal() instead of key-based comparison.\n + reftable_record_equal(), instead of key-based comparison.\n\n Mentored-by: Patrick Steinhardt \n Mentored-by: Christian Couder \n @@ t/unit-tests/t-reftable-block.c: license that can be found in the LICENSE file o\n \tstruct block_writer bw = {\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n \t\tchar name[100];\n - \t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX , (uintmax_t)i);\n + \t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX, (uintmax_t)i);\n\n -\t\trec.u.ref.refname = name;\n +\t\trec.u.ref.refname = xstrdup(name);\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n\n -\t\tnames[i] = xstrdup(name);\n +\t\trecs[i] = rec;\n - \t\tn = block_writer_add(&bw, &rec);\n + \t\tret = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n - \t\tcheck_int(r, >=, 0);\n - \t\tif (r > 0)\n + \t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n + \t\t}\n -\t\tcheck_str(names[j], rec.u.ref.refname);\n +\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n \t\tj++;\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n -\t\tstrbuf_addstr(&want, names[i]);\n +\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n\n - \t\tn = block_iter_seek_key(&it, &br, &want);\n - \t\tcheck_int(n, ==, 0);\n + \t\tret = block_iter_seek_key(&it, &br, &want);\n + \t\tcheck_int(ret, ==, 0);\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n - \t\tn = block_iter_next(&it, &rec);\n - \t\tcheck_int(n, ==, 0);\n + \t\tret = block_iter_next(&it, &rec);\n + \t\tcheck_int(ret, ==, 0);\n\n -\t\tcheck_str(names[i], rec.u.ref.refname);\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n\n \t\twant.len--;\n - \t\tn = block_iter_seek_key(&it, &br, &want);\n + \t\tret = block_iter_seek_key(&it, &br, &want);\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n\n - \t\tn = block_iter_next(&it, &rec);\n - \t\tcheck_int(n, ==, 0);\n + \t\tret = block_iter_next(&it, &rec);\n + \t\tcheck_int(ret, ==, 0);\n -\t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n +\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n\n 4: 7ca77be75c ! 5: 0de1e0cbe2 t-reftable-block: use reftable_record_key() instead of strbuf_addstr()\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n -\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n +\t\treftable_record_key(&recs[i], &want);\n\n - \t\tn = block_iter_seek_key(&it, &br, &want);\n - \t\tcheck_int(n, ==, 0);\n + \t\tret = block_iter_seek_key(&it, &br, &want);\n + \t\tcheck_int(ret, ==, 0);\n 5: 0016d8828a ! 6: 74187586c9 t-reftable-block: use block_iter_reset() instead of block_iter_close()\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n +\t\tblock_iter_reset(&it);\n \t\treftable_record_key(&recs[i], &want);\n\n - \t\tn = block_iter_seek_key(&it, &br, &want);\n + \t\tret = block_iter_seek_key(&it, &br, &want);\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n - \t\tn = block_iter_next(&it, &rec);\n - \t\tcheck_int(n, ==, 0);\n + \t\tret = block_iter_next(&it, &rec);\n + \t\tcheck_int(ret, ==, 0);\n \t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n -\n -\t\tblock_iter_close(&it);\n 6: 49fe881f0e < -: ---------- t-reftable-block: use xstrfmt() instead of xstrdup()\n -: ---------- > 7: 5e35cae987 t-reftable-block: use xstrfmt() instead of xstrdup()\n 7: 5dbc756799 ! 8: bacd52a236 t-reftable-block: remove unnecessary variable 'j'\n @@ Commit message\n\n ## t/unit-tests/t-reftable-block.c ##\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n - \tint n;\n + \tint ret;\n \tstruct block_reader br = { 0 };\n \tstruct block_iter it = BLOCK_ITER_INIT;\n -\tsize_t j = 0;\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n\n -\twhile (1) {\n +\tfor (i = 0; ; i++) {\n - \t\tint r = block_iter_next(&it, &rec);\n - \t\tcheck_int(r, >=, 0);\n - \t\tif (r > 0)\n + \t\tret = block_iter_next(&it, &rec);\n + \t\tcheck_int(ret, >=, 0);\n + \t\tif (ret > 0) {\n + \t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n + \t\t}\n -\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n -\t\tj++;\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n 8: 0916b8b2ca ! 9: 6e8632d3e6 t-reftable-block: add tests for log blocks\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n +\t\t.type = BLOCK_TYPE_LOG,\n +\t};\n +\tsize_t i = 0;\n -+\tint n;\n ++\tint ret;\n +\tstruct block_reader br = { 0 };\n +\tstruct block_iter it = BLOCK_ITER_INIT;\n +\tstruct strbuf want = STRBUF_INIT;\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n +\t\trec.u.log.value_type = REFTABLE_LOG_UPDATE;\n +\n +\t\trecs[i] = rec;\n -+\t\tn = block_writer_add(&bw, &rec);\n ++\t\tret = block_writer_add(&bw, &rec);\n +\t\trec.u.log.refname = NULL;\n +\t\trec.u.log.value_type = REFTABLE_LOG_DELETION;\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tcheck_int(ret, ==, 0);\n +\t}\n +\n -+\tn = block_writer_finish(&bw);\n -+\tcheck_int(n, >, 0);\n ++\tret = block_writer_finish(&bw);\n ++\tcheck_int(ret, >, 0);\n +\n +\tblock_writer_release(&bw);\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n +\tblock_iter_seek_start(&it, &br);\n +\n +\tfor (i = 0; ; i++) {\n -+\t\tint r = block_iter_next(&it, &rec);\n -+\t\tcheck_int(r, >=, 0);\n -+\t\tif (r > 0)\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, >=, 0);\n ++\t\tif (ret > 0) {\n ++\t\t\tcheck_int(i, ==, N);\n +\t\t\tbreak;\n ++\t\t}\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n +\t}\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n +\t\tstrbuf_reset(&want);\n +\t\tstrbuf_addstr(&want, recs[i].u.log.refname);\n +\n -+\t\tn = block_iter_seek_key(&it, &br, &want);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_seek_key(&it, &br, &want);\n ++\t\tcheck_int(ret, ==, 0);\n +\n -+\t\tn = block_iter_next(&it, &rec);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, ==, 0);\n +\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n +\n +\t\twant.len--;\n -+\t\tn = block_iter_seek_key(&it, &br, &want);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_seek_key(&it, &br, &want);\n ++\t\tcheck_int(ret, ==, 0);\n +\n -+\t\tn = block_iter_next(&it, &rec);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, ==, 0);\n +\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n +\t}\n +\n 9: 4794f2d610 ! 10: d147d5e9a9 t-reftable-block: add tests for obj blocks\n @@ t/unit-tests/t-reftable-block.c: static void t_log_block_read_write(void)\n +\t\t.type = BLOCK_TYPE_OBJ,\n +\t};\n +\tsize_t i = 0;\n -+\tint n;\n ++\tint ret;\n +\tstruct block_reader br = { 0 };\n +\tstruct block_iter it = BLOCK_ITER_INIT;\n +\tstruct strbuf want = STRBUF_INIT;\n @@ t/unit-tests/t-reftable-block.c: static void t_log_block_read_write(void)\n +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n +\n +\tfor (i = 0; i < N; i++) {\n -+\t\tuint8_t *bytes = reftable_malloc(sizeof(uint8_t[5]));\n -+\t\tmemcpy(bytes, (uint8_t[]){i, i+1, i+2, i+3, i+5}, sizeof(uint8_t[5]));\n ++\t\tuint8_t bytes[] = { i, i + 1, i + 2, i + 3, i + 5 }, *allocated;\n ++\t\tallocated = reftable_malloc(ARRAY_SIZE(bytes));\n ++\t\tDUP_ARRAY(allocated, bytes, ARRAY_SIZE(bytes));\n +\n -+\t\trec.u.obj.hash_prefix = bytes;\n ++\t\trec.u.obj.hash_prefix = allocated;\n +\t\trec.u.obj.hash_prefix_len = 5;\n +\n +\t\trecs[i] = rec;\n -+\t\tn = block_writer_add(&bw, &rec);\n ++\t\tret = block_writer_add(&bw, &rec);\n +\t\trec.u.obj.hash_prefix = NULL;\n +\t\trec.u.obj.hash_prefix_len = 0;\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tcheck_int(ret, ==, 0);\n +\t}\n +\n -+\tn = block_writer_finish(&bw);\n -+\tcheck_int(n, >, 0);\n ++\tret = block_writer_finish(&bw);\n ++\tcheck_int(ret, >, 0);\n +\n +\tblock_writer_release(&bw);\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_log_block_read_write(void)\n +\tblock_iter_seek_start(&it, &br);\n +\n +\tfor (i = 0; ; i++) {\n -+\t\tint r = block_iter_next(&it, &rec);\n -+\t\tcheck_int(r, >=, 0);\n -+\t\tif (r > 0)\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, >=, 0);\n ++\t\tif (ret > 0) {\n ++\t\t\tcheck_int(i, ==, N);\n +\t\t\tbreak;\n ++\t\t}\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n +\t}\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_log_block_read_write(void)\n +\t\tblock_iter_reset(&it);\n +\t\treftable_record_key(&recs[i], &want);\n +\n -+\t\tn = block_iter_seek_key(&it, &br, &want);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_seek_key(&it, &br, &want);\n ++\t\tcheck_int(ret, ==, 0);\n +\n -+\t\tn = block_iter_next(&it, &rec);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, ==, 0);\n +\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n +\t}\n10: 40fcc98173 ! 11: 2b43b1ac9a t-reftable-block: add tests for index blocks\n @@ t/unit-tests/t-reftable-block.c: static void t_obj_block_read_write(void)\n +\t\t.u.idx.last_key = STRBUF_INIT,\n +\t};\n +\tsize_t i = 0;\n -+\tint n;\n ++\tint ret;\n +\tstruct block_reader br = { 0 };\n +\tstruct block_iter it = BLOCK_ITER_INIT;\n +\tstruct strbuf want = STRBUF_INIT;\n @@ t/unit-tests/t-reftable-block.c: static void t_obj_block_read_write(void)\n +\t\tstrbuf_addf(&recs[i].u.idx.last_key, \"branch%02\"PRIuMAX, (uintmax_t)i);\n +\t\trecs[i].u.idx.offset = i;\n +\n -+\t\tn = block_writer_add(&bw, &recs[i]);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_writer_add(&bw, &recs[i]);\n ++\t\tcheck_int(ret, ==, 0);\n +\t}\n +\n -+\tn = block_writer_finish(&bw);\n -+\tcheck_int(n, >, 0);\n ++\tret = block_writer_finish(&bw);\n ++\tcheck_int(ret, >, 0);\n +\n +\tblock_writer_release(&bw);\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_obj_block_read_write(void)\n +\tblock_iter_seek_start(&it, &br);\n +\n +\tfor (i = 0; ; i++) {\n -+\t\tint r = block_iter_next(&it, &rec);\n -+\t\tcheck_int(r, >=, 0);\n -+\t\tif (r > 0)\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, >=, 0);\n ++\t\tif (ret > 0) {\n ++\t\t\tcheck_int(i, ==, N);\n +\t\t\tbreak;\n ++\t\t}\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n +\t}\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_obj_block_read_write(void)\n +\t\tblock_iter_reset(&it);\n +\t\treftable_record_key(&recs[i], &want);\n +\n -+\t\tn = block_iter_seek_key(&it, &br, &want);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_seek_key(&it, &br, &want);\n ++\t\tcheck_int(ret, ==, 0);\n +\n -+\t\tn = block_iter_next(&it, &rec);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, ==, 0);\n +\n +\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n +\n +\t\twant.len--;\n -+\t\tn = block_iter_seek_key(&it, &br, &want);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_seek_key(&it, &br, &want);\n ++\t\tcheck_int(ret, ==, 0);\n +\n -+\t\tn = block_iter_next(&it, &rec);\n -+\t\tcheck_int(n, ==, 0);\n ++\t\tret = block_iter_next(&it, &rec);\n ++\t\tcheck_int(ret, ==, 0);\n +\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n +\t}\n +\n"},{"id":"501148","messageId":"20240816175414.5169-2-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 01/11] t: move reftable/block_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:24Z","receivedAt":"2024-08-16T17:54:56Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/block_test.c exercises the functions defined in\nreftable/block.{c, h}. Migrate reftable/block_test.c to the unit\ntesting framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework and renaming the tests to follow the unit-tests'\nnaming conventions.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n Makefile | 2 +-\n reftable/reftable-tests.h | 1 -\n t/helper/test-reftable.c | 1 -\n .../unit-tests/t-reftable-block.c | 45 +++++++++----------\n 4 files changed, 22 insertions(+), 27 deletions(-)\n rename reftable/block_test.c => t/unit-tests/t-reftable-block.c (76%)\n\ndiff --git a/Makefile b/Makefile\nindex 13890710f8..a30cd636f8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1340,6 +1340,7 @@ 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+UNIT_TEST_PROGRAMS += t-reftable-block\n UNIT_TEST_PROGRAMS += t-reftable-merged\n UNIT_TEST_PROGRAMS += t-reftable-pq\n UNIT_TEST_PROGRAMS += t-reftable-record\n@@ -2681,7 +2682,6 @@ REFTABLE_OBJS += reftable/stack.o\n REFTABLE_OBJS += reftable/tree.o\n REFTABLE_OBJS += reftable/writer.o\n \n-REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n REFTABLE_TEST_OBJS += reftable/stack_test.o\ndiff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h\nindex 4b666810af..3d9118b91b 100644\n--- a/reftable/reftable-tests.h\n+++ b/reftable/reftable-tests.h\n@@ -10,7 +10,6 @@ license that can be found in the LICENSE file or at\n #define REFTABLE_TESTS_H\n \n int basics_test_main(int argc, const char **argv);\n-int block_test_main(int argc, const char **argv);\n int record_test_main(int argc, const char **argv);\n int readwrite_test_main(int argc, const char **argv);\n int stack_test_main(int argc, const char **argv);\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 623cf3f0f5..7bdd18430b 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -5,7 +5,6 @@\n int cmd__reftable(int argc, const char **argv)\n {\n \t/* test from simple to complex. */\n-\tblock_test_main(argc, argv);\n \treadwrite_test_main(argc, argv);\n \tstack_test_main(argc, argv);\n \treturn 0;\ndiff --git a/reftable/block_test.c b/t/unit-tests/t-reftable-block.c\nsimilarity index 76%\nrename from reftable/block_test.c\nrename to t/unit-tests/t-reftable-block.c\nindex 90aecd5a7c..f2b9a8a6f4 100644\n--- a/reftable/block_test.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -6,17 +6,13 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"block.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/block.h\"\n+#include \"reftable/blocksource.h\"\n+#include \"reftable/constants.h\"\n+#include \"reftable/reftable-error.h\"\n \n-#include \"system.h\"\n-#include \"blocksource.h\"\n-#include \"basics.h\"\n-#include \"constants.h\"\n-#include \"record.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-tests.h\"\n-\n-static void test_block_read_write(void)\n+static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tchar *names[30];\n@@ -45,7 +41,7 @@ static void test_block_read_write(void)\n \trec.u.ref.refname = (char *) \"\";\n \trec.u.ref.value_type = REFTABLE_REF_DELETION;\n \tn = block_writer_add(&bw, &rec);\n-\tEXPECT(n == REFTABLE_API_ERROR);\n+\tcheck_int(n, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n@@ -59,11 +55,11 @@ static void test_block_read_write(void)\n \t\tn = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \t}\n \n \tn = block_writer_finish(&bw);\n-\tEXPECT(n > 0);\n+\tcheck_int(n, >, 0);\n \n \tblock_writer_release(&bw);\n \n@@ -73,11 +69,11 @@ static void test_block_read_write(void)\n \n \twhile (1) {\n \t\tint r = block_iter_next(&it, &rec);\n-\t\tEXPECT(r >= 0);\n+\t\tcheck_int(r, >=, 0);\n \t\tif (r > 0) {\n \t\t\tbreak;\n \t\t}\n-\t\tEXPECT_STREQ(names[j], rec.u.ref.refname);\n+\t\tcheck_str(names[j], rec.u.ref.refname);\n \t\tj++;\n \t}\n \n@@ -90,20 +86,20 @@ static void test_block_read_write(void)\n \t\tstrbuf_addstr(&want, names[i]);\n \n \t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n \t\tn = block_iter_next(&it, &rec);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n-\t\tEXPECT_STREQ(names[i], rec.u.ref.refname);\n+\t\tcheck_str(names[i], rec.u.ref.refname);\n \n \t\twant.len--;\n \t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n \t\tn = block_iter_next(&it, &rec);\n-\t\tEXPECT(n == 0);\n-\t\tEXPECT_STREQ(names[10 * (i / 10)], rec.u.ref.refname);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n \n \t\tblock_iter_close(&it);\n \t}\n@@ -116,8 +112,9 @@ static void test_block_read_write(void)\n \t}\n }\n \n-int block_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_block_read_write);\n-\treturn 0;\n+\tTEST(t_block_read_write(), \"read-write operations on blocks work\");\n+\n+\treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"501149","messageId":"20240816175414.5169-3-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 02/11] t: harmonize t-reftable-block.c with coding guidelines","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:25Z","receivedAt":"2024-08-16T17:54:59Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Harmonize the newly ported test unit-tests/t-reftable-block.c\nwith the following guidelines:\n- Single line 'for' statements must omit curly braces.\n- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n- Array sizes and indices should preferably be of type 'size_t'and\n not 'int'.\n- Return code variable should preferably be named 'ret', not 'n'.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 52 ++++++++++++++++-----------------\n 1 file changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex f2b9a8a6f4..b1b238ac2a 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -16,20 +16,20 @@ static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tchar *names[30];\n-\tconst int N = ARRAY_SIZE(names);\n-\tconst int block_size = 1024;\n-\tstruct reftable_block block = { NULL };\n+\tconst size_t N = ARRAY_SIZE(names);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n \tstruct block_writer bw = {\n \t\t.last_key = STRBUF_INIT,\n \t};\n \tstruct reftable_record rec = {\n \t\t.type = BLOCK_TYPE_REF,\n \t};\n-\tint i = 0;\n-\tint n;\n+\tsize_t i = 0;\n+\tint ret;\n \tstruct block_reader br = { 0 };\n \tstruct block_iter it = BLOCK_ITER_INIT;\n-\tint j = 0;\n+\tsize_t j = 0;\n \tstruct strbuf want = STRBUF_INIT;\n \n \tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n@@ -40,26 +40,26 @@ static void t_block_read_write(void)\n \n \trec.u.ref.refname = (char *) \"\";\n \trec.u.ref.value_type = REFTABLE_REF_DELETION;\n-\tn = block_writer_add(&bw, &rec);\n-\tcheck_int(n, ==, REFTABLE_API_ERROR);\n+\tret = block_writer_add(&bw, &rec);\n+\tcheck_int(ret, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n-\t\tsnprintf(name, sizeof(name), \"branch%02d\", i);\n+\t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX, (uintmax_t)i);\n \n \t\trec.u.ref.refname = name;\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n \t\tnames[i] = xstrdup(name);\n-\t\tn = block_writer_add(&bw, &rec);\n+\t\tret = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n-\t\tcheck_int(n, ==, 0);\n+\t\tcheck_int(ret, ==, 0);\n \t}\n \n-\tn = block_writer_finish(&bw);\n-\tcheck_int(n, >, 0);\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n \n \tblock_writer_release(&bw);\n \n@@ -68,9 +68,10 @@ static void t_block_read_write(void)\n \tblock_iter_seek_start(&it, &br);\n \n \twhile (1) {\n-\t\tint r = block_iter_next(&it, &rec);\n-\t\tcheck_int(r, >=, 0);\n-\t\tif (r > 0) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n \t\t}\n \t\tcheck_str(names[j], rec.u.ref.refname);\n@@ -85,20 +86,20 @@ static void t_block_read_write(void)\n \t\tstrbuf_reset(&want);\n \t\tstrbuf_addstr(&want, names[i]);\n \n-\t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n \n-\t\tn = block_iter_next(&it, &rec);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n \n \t\tcheck_str(names[i], rec.u.ref.refname);\n \n \t\twant.len--;\n-\t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n \n-\t\tn = block_iter_next(&it, &rec);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n \t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n \n \t\tblock_iter_close(&it);\n@@ -107,9 +108,8 @@ static void t_block_read_write(void)\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-\tfor (i = 0; i < N; i++) {\n+\tfor (i = 0; i < N; i++)\n \t\treftable_free(names[i]);\n-\t}\n }\n \n int cmd_main(int argc, const char *argv[])\n-- \n2.45.GIT\n\n"},{"id":"501150","messageId":"20240816175414.5169-4-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 03/11] t-reftable-block: release used block reader","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:26Z","receivedAt":"2024-08-16T17:55:01Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Used block readers must be released using block_reader_release() to\nprevent the occurence of a memory leak. Make test_block_read_write()\nconform to this statement.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex b1b238ac2a..eafe1fdee9 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -105,6 +105,7 @@ static void t_block_read_write(void)\n \t\tblock_iter_close(&it);\n \t}\n \n+\tblock_reader_release(&br);\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-- \n2.45.GIT\n\n"},{"id":"501151","messageId":"20240816175414.5169-5-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 04/11] t-reftable-block: use reftable_record_equal() instead of check_str()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:27Z","receivedAt":"2024-08-16T17:55:04Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, operations like read and write for\nreftable blocks as defined by reftable/block.{c, h} are verified by\ncomparing only the keys of input and output reftable records. This is\nnot ideal because there can exist inequal reftable records with the\nsame key. Use the dedicated function for record comparison,\nreftable_record_equal(), instead of key-based comparison.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex eafe1fdee9..b106d3c1e4 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -15,8 +15,8 @@ license that can be found in the LICENSE file or at\n static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n-\tchar *names[30];\n-\tconst size_t N = ARRAY_SIZE(names);\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n \tconst size_t block_size = 1024;\n \tstruct reftable_block block = { 0 };\n \tstruct block_writer bw = {\n@@ -47,11 +47,11 @@ static void t_block_read_write(void)\n \t\tchar name[100];\n \t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX, (uintmax_t)i);\n \n-\t\trec.u.ref.refname = name;\n+\t\trec.u.ref.refname = xstrdup(name);\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n-\t\tnames[i] = xstrdup(name);\n+\t\trecs[i] = rec;\n \t\tret = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n@@ -74,7 +74,7 @@ static void t_block_read_write(void)\n \t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n \t\t}\n-\t\tcheck_str(names[j], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n \t\tj++;\n \t}\n \n@@ -84,7 +84,7 @@ static void t_block_read_write(void)\n \tfor (i = 0; i < N; i++) {\n \t\tstruct block_iter it = BLOCK_ITER_INIT;\n \t\tstrbuf_reset(&want);\n-\t\tstrbuf_addstr(&want, names[i]);\n+\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n \n \t\tret = block_iter_seek_key(&it, &br, &want);\n \t\tcheck_int(ret, ==, 0);\n@@ -92,7 +92,7 @@ static void t_block_read_write(void)\n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, ==, 0);\n \n-\t\tcheck_str(names[i], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n \n \t\twant.len--;\n \t\tret = block_iter_seek_key(&it, &br, &want);\n@@ -100,7 +100,7 @@ static void t_block_read_write(void)\n \n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, ==, 0);\n-\t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n \n \t\tblock_iter_close(&it);\n \t}\n@@ -110,7 +110,7 @@ static void t_block_read_write(void)\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n \tfor (i = 0; i < N; i++)\n-\t\treftable_free(names[i]);\n+\t\treftable_record_release(&recs[i]);\n }\n \n int cmd_main(int argc, const char *argv[])\n-- \n2.45.GIT\n\n"},{"id":"501152","messageId":"20240816175414.5169-6-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 05/11] t-reftable-block: use reftable_record_key() instead of strbuf_addstr()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:28Z","receivedAt":"2024-08-16T17:55:07Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, the record key required for many block\niterator functions is manually stored in a strbuf struct and then\npassed to these functions. This is not ideal when there exists a\ndedicated function to encode a record's key into a strbuf, namely\nreftable_record_key(). Use this function instead of manual encoding.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex b106d3c1e4..5887e9205d 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -83,8 +83,7 @@ static void t_block_read_write(void)\n \n \tfor (i = 0; i < N; i++) {\n \t\tstruct block_iter it = BLOCK_ITER_INIT;\n-\t\tstrbuf_reset(&want);\n-\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n+\t\treftable_record_key(&recs[i], &want);\n \n \t\tret = block_iter_seek_key(&it, &br, &want);\n \t\tcheck_int(ret, ==, 0);\n-- \n2.45.GIT\n\n"},{"id":"501153","messageId":"20240816175414.5169-7-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 06/11] t-reftable-block: use block_iter_reset() instead of block_iter_close()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:29Z","receivedAt":"2024-08-16T17:55:10Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"block_iter_reset() restores a block iterator to its state at the time\nof initialization without freeing any memory while block_iter_close()\ndeallocates the memory for the iterator.\n\nIn the current testing setup, a block iterator is allocated and\ndeallocated for every iteration of a loop, which hurts performance.\nImprove upon this by using block_iter_reset() at the start of each\niteration instead. This has the added benifit of testing\nblock_iter_reset(), which currently remains untested.\n\nSimilarly, remove reftable_record_release() for a reftable record\nthat is still in use.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 8 ++------\n 1 file changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 5887e9205d..ad3d128ea7 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -78,11 +78,8 @@ static void t_block_read_write(void)\n \t\tj++;\n \t}\n \n-\treftable_record_release(&rec);\n-\tblock_iter_close(&it);\n-\n \tfor (i = 0; i < N; i++) {\n-\t\tstruct block_iter it = BLOCK_ITER_INIT;\n+\t\tblock_iter_reset(&it);\n \t\treftable_record_key(&recs[i], &want);\n \n \t\tret = block_iter_seek_key(&it, &br, &want);\n@@ -100,11 +97,10 @@ static void t_block_read_write(void)\n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, ==, 0);\n \t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n-\n-\t\tblock_iter_close(&it);\n \t}\n \n \tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-- \n2.45.GIT\n\n"},{"id":"501154","messageId":"20240816175414.5169-8-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 07/11] t-reftable-block: use xstrfmt() instead of xstrdup()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:30Z","receivedAt":"2024-08-16T17:55:14Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Use xstrfmt() to assign a formatted string to a ref record's\nrefname instead of xstrdup(). This helps save the overhead of\na local 'char' buffer as well as makes the test more compact.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex ad3d128ea7..81484bc646 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -44,10 +44,7 @@ static void t_block_read_write(void)\n \tcheck_int(ret, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n-\t\tchar name[100];\n-\t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX, (uintmax_t)i);\n-\n-\t\trec.u.ref.refname = xstrdup(name);\n+\t\trec.u.ref.refname = xstrfmt(\"branch%02\"PRIuMAX, (uintmax_t)i);\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n-- \n2.45.GIT\n\n"},{"id":"501155","messageId":"20240816175414.5169-9-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 08/11] t-reftable-block: remove unnecessary variable 'j'","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:31Z","receivedAt":"2024-08-16T17:55:16Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Currently, there are two variables for array indices, 'i' and 'j'.\nThe variable 'j' is used only once and can be easily replaced with\n'i'. Get rid of 'j' and replace its occurence with 'i'.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 81484bc646..6aa86a3edf 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -29,7 +29,6 @@ static void t_block_read_write(void)\n \tint ret;\n \tstruct block_reader br = { 0 };\n \tstruct block_iter it = BLOCK_ITER_INIT;\n-\tsize_t j = 0;\n \tstruct strbuf want = STRBUF_INIT;\n \n \tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n@@ -64,15 +63,14 @@ static void t_block_read_write(void)\n \n \tblock_iter_seek_start(&it, &br);\n \n-\twhile (1) {\n+\tfor (i = 0; ; i++) {\n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, >=, 0);\n \t\tif (ret > 0) {\n \t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n \t\t}\n-\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n-\t\tj++;\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n \t}\n \n \tfor (i = 0; i < N; i++) {\n-- \n2.45.GIT\n\n"},{"id":"501156","messageId":"20240816175414.5169-10-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 09/11] t-reftable-block: add tests for log blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:32Z","receivedAt":"2024-08-16T17:55:19Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are only exercised\nfor ref blocks. Add another test that exercises these operations\nfor log blocks as well.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 92 ++++++++++++++++++++++++++++++++-\n 1 file changed, 90 insertions(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 6aa86a3edf..1256c7df6a 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -12,7 +12,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/constants.h\"\n #include \"reftable/reftable-error.h\"\n \n-static void t_block_read_write(void)\n+static void t_ref_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tstruct reftable_record recs[30];\n@@ -103,9 +103,97 @@ static void t_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_log_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 2048;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_LOG,\n+\t};\n+\tsize_t i = 0;\n+\tint ret;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock.source = malloc_block_source();\n+\tblock_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\trec.u.log.refname = xstrfmt(\"branch%02\"PRIuMAX , (uintmax_t)i);\n+\t\trec.u.log.update_index = i;\n+\t\trec.u.log.value_type = REFTABLE_LOG_UPDATE;\n+\n+\t\trecs[i] = rec;\n+\t\tret = block_writer_add(&bw, &rec);\n+\t\trec.u.log.refname = NULL;\n+\t\trec.u.log.value_type = REFTABLE_LOG_DELETION;\n+\t\tcheck_int(ret, ==, 0);\n+\t}\n+\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\tstrbuf_reset(&want);\n+\t\tstrbuf_addstr(&want, recs[i].u.log.refname);\n+\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\n+\t\twant.len--;\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n-\tTEST(t_block_read_write(), \"read-write operations on blocks work\");\n+\tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n+\tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"501157","messageId":"20240816175414.5169-11-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 10/11] t-reftable-block: add tests for obj blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:33Z","receivedAt":"2024-08-16T17:55:21Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are left unexercised\nfor obj blocks. Add a test that exercises these operations for obj\nblocks.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 82 +++++++++++++++++++++++++++++++++\n 1 file changed, 82 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 1256c7df6a..7671a40969 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -190,9 +190,91 @@ static void t_log_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_obj_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_OBJ,\n+\t};\n+\tsize_t i = 0;\n+\tint ret;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock.source = malloc_block_source();\n+\tblock_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tuint8_t bytes[] = { i, i + 1, i + 2, i + 3, i + 5 }, *allocated;\n+\t\tallocated = reftable_malloc(ARRAY_SIZE(bytes));\n+\t\tDUP_ARRAY(allocated, bytes, ARRAY_SIZE(bytes));\n+\n+\t\trec.u.obj.hash_prefix = allocated;\n+\t\trec.u.obj.hash_prefix_len = 5;\n+\n+\t\trecs[i] = rec;\n+\t\tret = block_writer_add(&bw, &rec);\n+\t\trec.u.obj.hash_prefix = NULL;\n+\t\trec.u.obj.hash_prefix_len = 0;\n+\t\tcheck_int(ret, ==, 0);\n+\t}\n+\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\treftable_record_key(&recs[i], &want);\n+\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n \tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n+\tTEST(t_obj_block_read_write(), \"read-write operations on obj blocks work\");\n \tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n \n \treturn test_done();\n-- \n2.45.GIT\n\n"},{"id":"501158","messageId":"20240816175414.5169-12-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[PATCH v2 11/11] t-reftable-block: add tests for index blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T17:25:34Z","receivedAt":"2024-08-16T17:55:24Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are left unexercised\nfor index blocks. Add a test that exercises these operations for\nindex blocks.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 87 +++++++++++++++++++++++++++++++++\n 1 file changed, 87 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 7671a40969..c2b67a11bc 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -271,8 +271,95 @@ static void t_obj_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_index_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_INDEX,\n+\t\t.u.idx.last_key = STRBUF_INIT,\n+\t};\n+\tsize_t i = 0;\n+\tint ret;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock.source = malloc_block_source();\n+\tblock_writer_init(&bw, BLOCK_TYPE_INDEX, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tstrbuf_init(&recs[i].u.idx.last_key, 9);\n+\n+\t\trecs[i].type = BLOCK_TYPE_INDEX;\n+\t\tstrbuf_addf(&recs[i].u.idx.last_key, \"branch%02\"PRIuMAX, (uintmax_t)i);\n+\t\trecs[i].u.idx.offset = i;\n+\n+\t\tret = block_writer_add(&bw, &recs[i]);\n+\t\tcheck_int(ret, ==, 0);\n+\t}\n+\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\treftable_record_key(&recs[i], &want);\n+\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\n+\t\twant.len--;\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n+\tTEST(t_index_block_read_write(), \"read-write operations on index blocks work\");\n \tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n \tTEST(t_obj_block_read_write(), \"read-write operations on obj blocks work\");\n \tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n-- \n2.45.GIT\n\n"},{"id":"501159","messageId":"CA+J6zkQa9=7C_f=NqGHEEVhnAJZQ7q9gDMhFQ_F2QDn3RDJ+cA@mail.gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-11-chandrapratap3519@gmail.com","subject":"Re: [PATCH v2 10/11] t-reftable-block: add tests for obj blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-16T18:11:15Z","receivedAt":"2024-08-16T18:11:45Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"On Fri, 16 Aug 2024 at 23:25, Chandra Pratap\n wrote:\n>\n> In the current testing setup, block operations are left unexercised\n> for obj blocks. Add a test that exercises these operations for obj\n> blocks.\n>\n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n> ---\n> t/unit-tests/t-reftable-block.c | 82 +++++++++++++++++++++++++++++++++\n> 1 file changed, 82 insertions(+)\n>\n> diff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\n> index 1256c7df6a..7671a40969 100644\n> --- a/t/unit-tests/t-reftable-block.c\n> +++ b/t/unit-tests/t-reftable-block.c\n> @@ -190,9 +190,91 @@ static void t_log_block_read_write(void)\n> reftable_record_release(&recs[i]);\n> }\n>\n> +static void t_obj_block_read_write(void)\n> +{\n> + const int header_off = 21;\n> + struct reftable_record recs[30];\n> + const size_t N = ARRAY_SIZE(recs);\n> + const size_t block_size = 1024;\n> + struct reftable_block block = { 0 };\n> + struct block_writer bw = {\n> + .last_key = STRBUF_INIT,\n> + };\n> + struct reftable_record rec = {\n> + .type = BLOCK_TYPE_OBJ,\n> + };\n> + size_t i = 0;\n> + int ret;\n> + struct block_reader br = { 0 };\n> + struct block_iter it = BLOCK_ITER_INIT;\n> + struct strbuf want = STRBUF_INIT;\n> +\n> + REFTABLE_CALLOC_ARRAY(block.data, block_size);\n> + block.len = block_size;\n> + block.source = malloc_block_source();\n> + block_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n> + header_off, hash_size(GIT_SHA1_FORMAT_ID));\n> +\n> + for (i = 0; i < N; i++) {\n> + uint8_t bytes[] = { i, i + 1, i + 2, i + 3, i + 5 }, *allocated;\n> + allocated = reftable_malloc(ARRAY_SIZE(bytes));\n> + DUP_ARRAY(allocated, bytes, ARRAY_SIZE(bytes));\n\nThe second line of this loop is redundant and causes a memory leak\nin the GitHub CI. I'll fix this in the next iteration.\n\n---snip---\n"},{"id":"501406","messageId":"CA+J6zkRf5D5a7j2T_Ro3PLCAAXeYJ7Nz6rC3V1gR67mcbO0ajA@mail.gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH v2 00/11] t: port reftable/block_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T07:30:00Z","receivedAt":"2024-08-21T07:30:24Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Reminder for reviews/acks.\n"},{"id":"501412","messageId":"ZsWXF_zJTIsp8XOE@tanuki","threadId":"61947","inReplyTo":"20240816175414.5169-10-chandrapratap3519@gmail.com","subject":"Re: [PATCH v2 09/11] t-reftable-block: add tests for log blocks","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-21T07:28:34Z","receivedAt":"2024-08-21T10:23:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Aug 16, 2024 at 10:55:32PM +0530, Chandra Pratap wrote:\n> @@ -103,9 +103,97 @@ static void t_block_read_write(void)\n> \t\treftable_record_release(&recs[i]);\n> }\n> \n> +static void t_log_block_read_write(void)\n> +{\n> +\tconst int header_off = 21;\n> +\tstruct reftable_record recs[30];\n> +\tconst size_t N = ARRAY_SIZE(recs);\n> +\tconst size_t block_size = 2048;\n> +\tstruct reftable_block block = { 0 };\n> +\tstruct block_writer bw = {\n> +\t\t.last_key = STRBUF_INIT,\n> +\t};\n> +\tstruct reftable_record rec = {\n> +\t\t.type = BLOCK_TYPE_LOG,\n> +\t};\n> +\tsize_t i = 0;\n> +\tint ret;\n> +\tstruct block_reader br = { 0 };\n> +\tstruct block_iter it = BLOCK_ITER_INIT;\n> +\tstruct strbuf want = STRBUF_INIT;\n> +\n> +\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n> +\tblock.len = block_size;\n> +\tblock.source = malloc_block_source();\n> +\tblock_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n> +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n\nNit: instead of a `malloc_block_source()`, you may use\n`block_source_from_strbuf()`. The former will go away with the patch\nseries at [1].\n\nI'm also happy to rebase my patch series once yours lands and do this\nmyself. Guess yours will land faster anyway, and there are conflicts\nregardless of whether you do or don't update the test here. The same\napplies to the subsequent patches which use a `malloc_block_source()`.\n\nSo this isn't really worth a reroll by itself, and other than that this\npatch looks good to me.\n\nPatrick\n\n[1]: \n"},{"id":"501422","messageId":"20240821124150.4463-1-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240816175414.5169-1-chandrapratap3519@gmail.com","subject":"[GSoC][PATCH v3 00/11] t: port reftable/block_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:50Z","receivedAt":"2024-08-21T12:42:16Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"The reftable library comes with self tests, which are exercised\nas part of the usual end-to-end tests and are designed to\nobserve the end-user visible effects of Git commands. What it\nexercises, however, is a better match for the unit-testing\nframework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests',\n2023-12-09), which is designed to observe how low level\nimplementation details, at the level of sequences of individual\nfunction calls, behave.\n\nHence, port reftable/block_test.c to the unit testing framework and\nimprove upon the ported test. The first patch in the series moves\nthe test to the unit testing framework, and the rest of the patches\nimprove upon the ported test.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n\n---\nChanges in v3:\n- Use block_source_from_strbuf() instead of malloc_block_source()\n in log, obj, and index block tests (patches 9, 10, 11).\n- Remove a line that causes a memory leak in obj test (patch 10).\n\nCI/PR: https://github.com/gitgitgadget/git/pull/1749\n\nChandra Pratap(11):\nt: move reftable/block_test.c to the unit testing framework\nt: harmonize t-reftable-block.c with coding guidelines\nt-reftable-block: release used block reader\nt-reftable-block: use reftable_record_equal() instead of check_str()\nt-reftable-block: use reftable_record_key() instead of strbuf_addstr()\nt-reftable-block: use block_iter_reset() instead of block_iter_close()\nt-reftable-block: use xstrfmt() instead of xstrdup()\nt-reftable-block: remove unnecessary variable 'j'\nt-reftable-block: add tests for log blocks\nt-reftable-block: add tests for obj blocks\nt-reftable-block: add tests for index blocks\n\nMakefile | 2 +-\nreftable/block_test.c | 123 ------------------------------\nreftable/reftable-tests.h | 1 -\nt/helper/test-reftable.c | 1 -\nt/unit-tests/t-reftable-block.c | 370 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n5 files changed, 371 insertions(+), 126 deletions(-)\n\nRange-diff against v2:\n1: 502fcc1374 ! 1: 4cd1981016 t-reftable-block: add tests for log blocks\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n +\tint ret;\n +\tstruct block_reader br = { 0 };\n +\tstruct block_iter it = BLOCK_ITER_INIT;\n -+\tstruct strbuf want = STRBUF_INIT;\n ++\tstruct strbuf want = STRBUF_INIT, buf = STRBUF_INIT;\n +\n +\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n +\tblock.len = block_size;\n -+\tblock.source = malloc_block_source();\n ++\tblock_source_from_strbuf(&block.source ,&buf);\n +\tblock_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_block_read_write(void)\n +\treftable_record_release(&rec);\n +\treftable_block_done(&br.block);\n +\tstrbuf_release(&want);\n ++\tstrbuf_release(&buf);\n +\tfor (i = 0; i < N; i++)\n +\t\treftable_record_release(&recs[i]);\n +}\n2: e3cefa7e3d ! 2: 4e9752e72a t-reftable-block: add tests for obj blocks\n @@ t/unit-tests/t-reftable-block.c: static void t_log_block_read_write(void)\n +\tint ret;\n +\tstruct block_reader br = { 0 };\n +\tstruct block_iter it = BLOCK_ITER_INIT;\n -+\tstruct strbuf want = STRBUF_INIT;\n ++\tstruct strbuf want = STRBUF_INIT, buf = STRBUF_INIT;\n +\n +\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n +\tblock.len = block_size;\n -+\tblock.source = malloc_block_source();\n ++\tblock_source_from_strbuf(&block.source, &buf);\n +\tblock_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n +\n +\tfor (i = 0; i < N; i++) {\n +\t\tuint8_t bytes[] = { i, i + 1, i + 2, i + 3, i + 5 }, *allocated;\n -+\t\tallocated = reftable_malloc(ARRAY_SIZE(bytes));\n +\t\tDUP_ARRAY(allocated, bytes, ARRAY_SIZE(bytes));\n +\n +\t\trec.u.obj.hash_prefix = allocated;\n @@ t/unit-tests/t-reftable-block.c: static void t_log_block_read_write(void)\n +\treftable_record_release(&rec);\n +\treftable_block_done(&br.block);\n +\tstrbuf_release(&want);\n ++\tstrbuf_release(&buf);\n +\tfor (i = 0; i < N; i++)\n +\t\treftable_record_release(&recs[i]);\n +}\n3: 4be7749c4b ! 3: db62f23594 t-reftable-block: add tests for index blocks\n @@ t/unit-tests/t-reftable-block.c: static void t_obj_block_read_write(void)\n +\tint ret;\n +\tstruct block_reader br = { 0 };\n +\tstruct block_iter it = BLOCK_ITER_INIT;\n -+\tstruct strbuf want = STRBUF_INIT;\n ++\tstruct strbuf want = STRBUF_INIT, buf = STRBUF_INIT;\n +\n +\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n +\tblock.len = block_size;\n -+\tblock.source = malloc_block_source();\n ++\tblock_source_from_strbuf(&block.source, &buf);\n +\tblock_writer_init(&bw, BLOCK_TYPE_INDEX, block.data, block_size,\n +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n +\n @@ t/unit-tests/t-reftable-block.c: static void t_obj_block_read_write(void)\n +\treftable_record_release(&rec);\n +\treftable_block_done(&br.block);\n +\tstrbuf_release(&want);\n ++\tstrbuf_release(&buf);\n +\tfor (i = 0; i < N; i++)\n +\t\treftable_record_release(&recs[i]);\n +}\n\n"},{"id":"501423","messageId":"20240821124150.4463-2-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 01/11] t: move reftable/block_test.c to the unit testing framework","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:51Z","receivedAt":"2024-08-21T12:42:19Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"reftable/block_test.c exercises the functions defined in\nreftable/block.{c, h}. Migrate reftable/block_test.c to the unit\ntesting framework. Migration involves refactoring the tests\nto use the unit testing framework instead of reftable's test\nframework and renaming the tests to follow the unit-tests'\nnaming conventions.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n Makefile | 2 +-\n reftable/reftable-tests.h | 1 -\n t/helper/test-reftable.c | 1 -\n .../unit-tests/t-reftable-block.c | 45 +++++++++----------\n 4 files changed, 22 insertions(+), 27 deletions(-)\n rename reftable/block_test.c => t/unit-tests/t-reftable-block.c (76%)\n\ndiff --git a/Makefile b/Makefile\nindex 13890710f8..a30cd636f8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1340,6 +1340,7 @@ 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+UNIT_TEST_PROGRAMS += t-reftable-block\n UNIT_TEST_PROGRAMS += t-reftable-merged\n UNIT_TEST_PROGRAMS += t-reftable-pq\n UNIT_TEST_PROGRAMS += t-reftable-record\n@@ -2681,7 +2682,6 @@ REFTABLE_OBJS += reftable/stack.o\n REFTABLE_OBJS += reftable/tree.o\n REFTABLE_OBJS += reftable/writer.o\n \n-REFTABLE_TEST_OBJS += reftable/block_test.o\n REFTABLE_TEST_OBJS += reftable/dump.o\n REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n REFTABLE_TEST_OBJS += reftable/stack_test.o\ndiff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h\nindex 4b666810af..3d9118b91b 100644\n--- a/reftable/reftable-tests.h\n+++ b/reftable/reftable-tests.h\n@@ -10,7 +10,6 @@ license that can be found in the LICENSE file or at\n #define REFTABLE_TESTS_H\n \n int basics_test_main(int argc, const char **argv);\n-int block_test_main(int argc, const char **argv);\n int record_test_main(int argc, const char **argv);\n int readwrite_test_main(int argc, const char **argv);\n int stack_test_main(int argc, const char **argv);\ndiff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c\nindex 623cf3f0f5..7bdd18430b 100644\n--- a/t/helper/test-reftable.c\n+++ b/t/helper/test-reftable.c\n@@ -5,7 +5,6 @@\n int cmd__reftable(int argc, const char **argv)\n {\n \t/* test from simple to complex. */\n-\tblock_test_main(argc, argv);\n \treadwrite_test_main(argc, argv);\n \tstack_test_main(argc, argv);\n \treturn 0;\ndiff --git a/reftable/block_test.c b/t/unit-tests/t-reftable-block.c\nsimilarity index 76%\nrename from reftable/block_test.c\nrename to t/unit-tests/t-reftable-block.c\nindex 90aecd5a7c..f2b9a8a6f4 100644\n--- a/reftable/block_test.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -6,17 +6,13 @@ license that can be found in the LICENSE file or at\n https://developers.google.com/open-source/licenses/bsd\n */\n \n-#include \"block.h\"\n+#include \"test-lib.h\"\n+#include \"reftable/block.h\"\n+#include \"reftable/blocksource.h\"\n+#include \"reftable/constants.h\"\n+#include \"reftable/reftable-error.h\"\n \n-#include \"system.h\"\n-#include \"blocksource.h\"\n-#include \"basics.h\"\n-#include \"constants.h\"\n-#include \"record.h\"\n-#include \"test_framework.h\"\n-#include \"reftable-tests.h\"\n-\n-static void test_block_read_write(void)\n+static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tchar *names[30];\n@@ -45,7 +41,7 @@ static void test_block_read_write(void)\n \trec.u.ref.refname = (char *) \"\";\n \trec.u.ref.value_type = REFTABLE_REF_DELETION;\n \tn = block_writer_add(&bw, &rec);\n-\tEXPECT(n == REFTABLE_API_ERROR);\n+\tcheck_int(n, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n@@ -59,11 +55,11 @@ static void test_block_read_write(void)\n \t\tn = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \t}\n \n \tn = block_writer_finish(&bw);\n-\tEXPECT(n > 0);\n+\tcheck_int(n, >, 0);\n \n \tblock_writer_release(&bw);\n \n@@ -73,11 +69,11 @@ static void test_block_read_write(void)\n \n \twhile (1) {\n \t\tint r = block_iter_next(&it, &rec);\n-\t\tEXPECT(r >= 0);\n+\t\tcheck_int(r, >=, 0);\n \t\tif (r > 0) {\n \t\t\tbreak;\n \t\t}\n-\t\tEXPECT_STREQ(names[j], rec.u.ref.refname);\n+\t\tcheck_str(names[j], rec.u.ref.refname);\n \t\tj++;\n \t}\n \n@@ -90,20 +86,20 @@ static void test_block_read_write(void)\n \t\tstrbuf_addstr(&want, names[i]);\n \n \t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n \t\tn = block_iter_next(&it, &rec);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n-\t\tEXPECT_STREQ(names[i], rec.u.ref.refname);\n+\t\tcheck_str(names[i], rec.u.ref.refname);\n \n \t\twant.len--;\n \t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tEXPECT(n == 0);\n+\t\tcheck_int(n, ==, 0);\n \n \t\tn = block_iter_next(&it, &rec);\n-\t\tEXPECT(n == 0);\n-\t\tEXPECT_STREQ(names[10 * (i / 10)], rec.u.ref.refname);\n+\t\tcheck_int(n, ==, 0);\n+\t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n \n \t\tblock_iter_close(&it);\n \t}\n@@ -116,8 +112,9 @@ static void test_block_read_write(void)\n \t}\n }\n \n-int block_test_main(int argc, const char *argv[])\n+int cmd_main(int argc, const char *argv[])\n {\n-\tRUN_TEST(test_block_read_write);\n-\treturn 0;\n+\tTEST(t_block_read_write(), \"read-write operations on blocks work\");\n+\n+\treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"501424","messageId":"20240821124150.4463-3-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 02/11] t: harmonize t-reftable-block.c with coding guidelines","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:52Z","receivedAt":"2024-08-21T12:42:22Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Harmonize the newly ported test unit-tests/t-reftable-block.c\nwith the following guidelines:\n- Single line 'for' statements must omit curly braces.\n- Structs must be 0-initialized with '= { 0 }' instead of '= { NULL }'.\n- Array sizes and indices should preferably be of type 'size_t'and\n not 'int'.\n- Return code variable should preferably be named 'ret', not 'n'.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 52 ++++++++++++++++-----------------\n 1 file changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex f2b9a8a6f4..b1b238ac2a 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -16,20 +16,20 @@ static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tchar *names[30];\n-\tconst int N = ARRAY_SIZE(names);\n-\tconst int block_size = 1024;\n-\tstruct reftable_block block = { NULL };\n+\tconst size_t N = ARRAY_SIZE(names);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n \tstruct block_writer bw = {\n \t\t.last_key = STRBUF_INIT,\n \t};\n \tstruct reftable_record rec = {\n \t\t.type = BLOCK_TYPE_REF,\n \t};\n-\tint i = 0;\n-\tint n;\n+\tsize_t i = 0;\n+\tint ret;\n \tstruct block_reader br = { 0 };\n \tstruct block_iter it = BLOCK_ITER_INIT;\n-\tint j = 0;\n+\tsize_t j = 0;\n \tstruct strbuf want = STRBUF_INIT;\n \n \tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n@@ -40,26 +40,26 @@ static void t_block_read_write(void)\n \n \trec.u.ref.refname = (char *) \"\";\n \trec.u.ref.value_type = REFTABLE_REF_DELETION;\n-\tn = block_writer_add(&bw, &rec);\n-\tcheck_int(n, ==, REFTABLE_API_ERROR);\n+\tret = block_writer_add(&bw, &rec);\n+\tcheck_int(ret, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n \t\tchar name[100];\n-\t\tsnprintf(name, sizeof(name), \"branch%02d\", i);\n+\t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX, (uintmax_t)i);\n \n \t\trec.u.ref.refname = name;\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n \t\tnames[i] = xstrdup(name);\n-\t\tn = block_writer_add(&bw, &rec);\n+\t\tret = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n-\t\tcheck_int(n, ==, 0);\n+\t\tcheck_int(ret, ==, 0);\n \t}\n \n-\tn = block_writer_finish(&bw);\n-\tcheck_int(n, >, 0);\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n \n \tblock_writer_release(&bw);\n \n@@ -68,9 +68,10 @@ static void t_block_read_write(void)\n \tblock_iter_seek_start(&it, &br);\n \n \twhile (1) {\n-\t\tint r = block_iter_next(&it, &rec);\n-\t\tcheck_int(r, >=, 0);\n-\t\tif (r > 0) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n \t\t}\n \t\tcheck_str(names[j], rec.u.ref.refname);\n@@ -85,20 +86,20 @@ static void t_block_read_write(void)\n \t\tstrbuf_reset(&want);\n \t\tstrbuf_addstr(&want, names[i]);\n \n-\t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n \n-\t\tn = block_iter_next(&it, &rec);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n \n \t\tcheck_str(names[i], rec.u.ref.refname);\n \n \t\twant.len--;\n-\t\tn = block_iter_seek_key(&it, &br, &want);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n \n-\t\tn = block_iter_next(&it, &rec);\n-\t\tcheck_int(n, ==, 0);\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n \t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n \n \t\tblock_iter_close(&it);\n@@ -107,9 +108,8 @@ static void t_block_read_write(void)\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-\tfor (i = 0; i < N; i++) {\n+\tfor (i = 0; i < N; i++)\n \t\treftable_free(names[i]);\n-\t}\n }\n \n int cmd_main(int argc, const char *argv[])\n-- \n2.45.GIT\n\n"},{"id":"501425","messageId":"20240821124150.4463-4-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 03/11] t-reftable-block: release used block reader","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:53Z","receivedAt":"2024-08-21T12:42:24Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Used block readers must be released using block_reader_release() to\nprevent the occurence of a memory leak. Make test_block_read_write()\nconform to this statement.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex b1b238ac2a..eafe1fdee9 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -105,6 +105,7 @@ static void t_block_read_write(void)\n \t\tblock_iter_close(&it);\n \t}\n \n+\tblock_reader_release(&br);\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-- \n2.45.GIT\n\n"},{"id":"501426","messageId":"20240821124150.4463-5-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 04/11] t-reftable-block: use reftable_record_equal() instead of check_str()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:54Z","receivedAt":"2024-08-21T12:42:27Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, operations like read and write for\nreftable blocks as defined by reftable/block.{c, h} are verified by\ncomparing only the keys of input and output reftable records. This is\nnot ideal because there can exist inequal reftable records with the\nsame key. Use the dedicated function for record comparison,\nreftable_record_equal(), instead of key-based comparison.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 18 +++++++++---------\n 1 file changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex eafe1fdee9..b106d3c1e4 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -15,8 +15,8 @@ license that can be found in the LICENSE file or at\n static void t_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n-\tchar *names[30];\n-\tconst size_t N = ARRAY_SIZE(names);\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n \tconst size_t block_size = 1024;\n \tstruct reftable_block block = { 0 };\n \tstruct block_writer bw = {\n@@ -47,11 +47,11 @@ static void t_block_read_write(void)\n \t\tchar name[100];\n \t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX, (uintmax_t)i);\n \n-\t\trec.u.ref.refname = name;\n+\t\trec.u.ref.refname = xstrdup(name);\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n-\t\tnames[i] = xstrdup(name);\n+\t\trecs[i] = rec;\n \t\tret = block_writer_add(&bw, &rec);\n \t\trec.u.ref.refname = NULL;\n \t\trec.u.ref.value_type = REFTABLE_REF_DELETION;\n@@ -74,7 +74,7 @@ static void t_block_read_write(void)\n \t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n \t\t}\n-\t\tcheck_str(names[j], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n \t\tj++;\n \t}\n \n@@ -84,7 +84,7 @@ static void t_block_read_write(void)\n \tfor (i = 0; i < N; i++) {\n \t\tstruct block_iter it = BLOCK_ITER_INIT;\n \t\tstrbuf_reset(&want);\n-\t\tstrbuf_addstr(&want, names[i]);\n+\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n \n \t\tret = block_iter_seek_key(&it, &br, &want);\n \t\tcheck_int(ret, ==, 0);\n@@ -92,7 +92,7 @@ static void t_block_read_write(void)\n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, ==, 0);\n \n-\t\tcheck_str(names[i], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n \n \t\twant.len--;\n \t\tret = block_iter_seek_key(&it, &br, &want);\n@@ -100,7 +100,7 @@ static void t_block_read_write(void)\n \n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, ==, 0);\n-\t\tcheck_str(names[10 * (i / 10)], rec.u.ref.refname);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n \n \t\tblock_iter_close(&it);\n \t}\n@@ -110,7 +110,7 @@ static void t_block_read_write(void)\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n \tfor (i = 0; i < N; i++)\n-\t\treftable_free(names[i]);\n+\t\treftable_record_release(&recs[i]);\n }\n \n int cmd_main(int argc, const char *argv[])\n-- \n2.45.GIT\n\n"},{"id":"501427","messageId":"20240821124150.4463-6-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 05/11] t-reftable-block: use reftable_record_key() instead of strbuf_addstr()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:55Z","receivedAt":"2024-08-21T12:42:30Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, the record key required for many block\niterator functions is manually stored in a strbuf struct and then\npassed to these functions. This is not ideal when there exists a\ndedicated function to encode a record's key into a strbuf, namely\nreftable_record_key(). Use this function instead of manual encoding.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex b106d3c1e4..5887e9205d 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -83,8 +83,7 @@ static void t_block_read_write(void)\n \n \tfor (i = 0; i < N; i++) {\n \t\tstruct block_iter it = BLOCK_ITER_INIT;\n-\t\tstrbuf_reset(&want);\n-\t\tstrbuf_addstr(&want, recs[i].u.ref.refname);\n+\t\treftable_record_key(&recs[i], &want);\n \n \t\tret = block_iter_seek_key(&it, &br, &want);\n \t\tcheck_int(ret, ==, 0);\n-- \n2.45.GIT\n\n"},{"id":"501428","messageId":"20240821124150.4463-7-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 06/11] t-reftable-block: use block_iter_reset() instead of block_iter_close()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:56Z","receivedAt":"2024-08-21T12:42:33Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"block_iter_reset() restores a block iterator to its state at the time\nof initialization without freeing any memory while block_iter_close()\ndeallocates the memory for the iterator.\n\nIn the current testing setup, a block iterator is allocated and\ndeallocated for every iteration of a loop, which hurts performance.\nImprove upon this by using block_iter_reset() at the start of each\niteration instead. This has the added benifit of testing\nblock_iter_reset(), which currently remains untested.\n\nSimilarly, remove reftable_record_release() for a reftable record\nthat is still in use.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 8 ++------\n 1 file changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 5887e9205d..ad3d128ea7 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -78,11 +78,8 @@ static void t_block_read_write(void)\n \t\tj++;\n \t}\n \n-\treftable_record_release(&rec);\n-\tblock_iter_close(&it);\n-\n \tfor (i = 0; i < N; i++) {\n-\t\tstruct block_iter it = BLOCK_ITER_INIT;\n+\t\tblock_iter_reset(&it);\n \t\treftable_record_key(&recs[i], &want);\n \n \t\tret = block_iter_seek_key(&it, &br, &want);\n@@ -100,11 +97,10 @@ static void t_block_read_write(void)\n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, ==, 0);\n \t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n-\n-\t\tblock_iter_close(&it);\n \t}\n \n \tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n \treftable_record_release(&rec);\n \treftable_block_done(&br.block);\n \tstrbuf_release(&want);\n-- \n2.45.GIT\n\n"},{"id":"501429","messageId":"20240821124150.4463-8-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 07/11] t-reftable-block: use xstrfmt() instead of xstrdup()","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:57Z","receivedAt":"2024-08-21T12:42:36Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Use xstrfmt() to assign a formatted string to a ref record's\nrefname instead of xstrdup(). This helps save the overhead of\na local 'char' buffer as well as makes the test more compact.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex ad3d128ea7..81484bc646 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -44,10 +44,7 @@ static void t_block_read_write(void)\n \tcheck_int(ret, ==, REFTABLE_API_ERROR);\n \n \tfor (i = 0; i < N; i++) {\n-\t\tchar name[100];\n-\t\tsnprintf(name, sizeof(name), \"branch%02\"PRIuMAX, (uintmax_t)i);\n-\n-\t\trec.u.ref.refname = xstrdup(name);\n+\t\trec.u.ref.refname = xstrfmt(\"branch%02\"PRIuMAX, (uintmax_t)i);\n \t\trec.u.ref.value_type = REFTABLE_REF_VAL1;\n \t\tmemset(rec.u.ref.value.val1, i, GIT_SHA1_RAWSZ);\n \n-- \n2.45.GIT\n\n"},{"id":"501430","messageId":"20240821124150.4463-9-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 08/11] t-reftable-block: remove unnecessary variable 'j'","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:58Z","receivedAt":"2024-08-21T12:42:39Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"Currently, there are two variables for array indices, 'i' and 'j'.\nThe variable 'j' is used only once and can be easily replaced with\n'i'. Get rid of 'j' and replace its occurence with 'i'.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 81484bc646..6aa86a3edf 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -29,7 +29,6 @@ static void t_block_read_write(void)\n \tint ret;\n \tstruct block_reader br = { 0 };\n \tstruct block_iter it = BLOCK_ITER_INIT;\n-\tsize_t j = 0;\n \tstruct strbuf want = STRBUF_INIT;\n \n \tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n@@ -64,15 +63,14 @@ static void t_block_read_write(void)\n \n \tblock_iter_seek_start(&it, &br);\n \n-\twhile (1) {\n+\tfor (i = 0; ; i++) {\n \t\tret = block_iter_next(&it, &rec);\n \t\tcheck_int(ret, >=, 0);\n \t\tif (ret > 0) {\n \t\t\tcheck_int(i, ==, N);\n \t\t\tbreak;\n \t\t}\n-\t\tcheck(reftable_record_equal(&recs[j], &rec, GIT_SHA1_RAWSZ));\n-\t\tj++;\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n \t}\n \n \tfor (i = 0; i < N; i++) {\n-- \n2.45.GIT\n\n"},{"id":"501431","messageId":"20240821124150.4463-10-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 09/11] t-reftable-block: add tests for log blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:30:59Z","receivedAt":"2024-08-21T12:42:41Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are only exercised\nfor ref blocks. Add another test that exercises these operations\nfor log blocks as well.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 93 ++++++++++++++++++++++++++++++++-\n 1 file changed, 91 insertions(+), 2 deletions(-)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 6aa86a3edf..4c4fb39ab4 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -12,7 +12,7 @@ license that can be found in the LICENSE file or at\n #include \"reftable/constants.h\"\n #include \"reftable/reftable-error.h\"\n \n-static void t_block_read_write(void)\n+static void t_ref_block_read_write(void)\n {\n \tconst int header_off = 21; /* random */\n \tstruct reftable_record recs[30];\n@@ -103,9 +103,98 @@ static void t_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_log_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 2048;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_LOG,\n+\t};\n+\tsize_t i = 0;\n+\tint ret;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT, buf = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock_source_from_strbuf(&block.source ,&buf);\n+\tblock_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\trec.u.log.refname = xstrfmt(\"branch%02\"PRIuMAX , (uintmax_t)i);\n+\t\trec.u.log.update_index = i;\n+\t\trec.u.log.value_type = REFTABLE_LOG_UPDATE;\n+\n+\t\trecs[i] = rec;\n+\t\tret = block_writer_add(&bw, &rec);\n+\t\trec.u.log.refname = NULL;\n+\t\trec.u.log.value_type = REFTABLE_LOG_DELETION;\n+\t\tcheck_int(ret, ==, 0);\n+\t}\n+\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\tstrbuf_reset(&want);\n+\t\tstrbuf_addstr(&want, recs[i].u.log.refname);\n+\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\n+\t\twant.len--;\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tstrbuf_release(&buf);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n-\tTEST(t_block_read_write(), \"read-write operations on blocks work\");\n+\tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n+\tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n \n \treturn test_done();\n }\n-- \n2.45.GIT\n\n"},{"id":"501432","messageId":"20240821124150.4463-11-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 10/11] t-reftable-block: add tests for obj blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:31:00Z","receivedAt":"2024-08-21T12:42:44Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are left unexercised\nfor obj blocks. Add a test that exercises these operations for obj\nblocks.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 82 +++++++++++++++++++++++++++++++++\n 1 file changed, 82 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex 4c4fb39ab4..beb2d6f81b 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -191,9 +191,91 @@ static void t_log_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_obj_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_OBJ,\n+\t};\n+\tsize_t i = 0;\n+\tint ret;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT, buf = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock_source_from_strbuf(&block.source, &buf);\n+\tblock_writer_init(&bw, BLOCK_TYPE_OBJ, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tuint8_t bytes[] = { i, i + 1, i + 2, i + 3, i + 5 }, *allocated;\n+\t\tDUP_ARRAY(allocated, bytes, ARRAY_SIZE(bytes));\n+\n+\t\trec.u.obj.hash_prefix = allocated;\n+\t\trec.u.obj.hash_prefix_len = 5;\n+\n+\t\trecs[i] = rec;\n+\t\tret = block_writer_add(&bw, &rec);\n+\t\trec.u.obj.hash_prefix = NULL;\n+\t\trec.u.obj.hash_prefix_len = 0;\n+\t\tcheck_int(ret, ==, 0);\n+\t}\n+\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\treftable_record_key(&recs[i], &want);\n+\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tstrbuf_release(&buf);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n \tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n+\tTEST(t_obj_block_read_write(), \"read-write operations on obj blocks work\");\n \tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n \n \treturn test_done();\n-- \n2.45.GIT\n\n"},{"id":"501433","messageId":"20240821124150.4463-12-chandrapratap3519@gmail.com","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"[PATCH v3 11/11] t-reftable-block: add tests for index blocks","fromName":"Chandra Pratap","fromEmail":"chandrapratap3519@gmail.com","sentAt":"2024-08-21T12:31:01Z","receivedAt":"2024-08-21T12:42:47Z","isPatch":true,"sender":{"key":"chandrapratap3519@gmail.com","avatar":null},"body":"In the current testing setup, block operations are left unexercised\nfor index blocks. Add a test that exercises these operations for\nindex blocks.\n\nMentored-by: Patrick Steinhardt \nMentored-by: Christian Couder \nSigned-off-by: Chandra Pratap \n---\n t/unit-tests/t-reftable-block.c | 88 +++++++++++++++++++++++++++++++++\n 1 file changed, 88 insertions(+)\n\ndiff --git a/t/unit-tests/t-reftable-block.c b/t/unit-tests/t-reftable-block.c\nindex beb2d6f81b..582a8e6036 100644\n--- a/t/unit-tests/t-reftable-block.c\n+++ b/t/unit-tests/t-reftable-block.c\n@@ -272,8 +272,96 @@ static void t_obj_block_read_write(void)\n \t\treftable_record_release(&recs[i]);\n }\n \n+static void t_index_block_read_write(void)\n+{\n+\tconst int header_off = 21;\n+\tstruct reftable_record recs[30];\n+\tconst size_t N = ARRAY_SIZE(recs);\n+\tconst size_t block_size = 1024;\n+\tstruct reftable_block block = { 0 };\n+\tstruct block_writer bw = {\n+\t\t.last_key = STRBUF_INIT,\n+\t};\n+\tstruct reftable_record rec = {\n+\t\t.type = BLOCK_TYPE_INDEX,\n+\t\t.u.idx.last_key = STRBUF_INIT,\n+\t};\n+\tsize_t i = 0;\n+\tint ret;\n+\tstruct block_reader br = { 0 };\n+\tstruct block_iter it = BLOCK_ITER_INIT;\n+\tstruct strbuf want = STRBUF_INIT, buf = STRBUF_INIT;\n+\n+\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n+\tblock.len = block_size;\n+\tblock_source_from_strbuf(&block.source, &buf);\n+\tblock_writer_init(&bw, BLOCK_TYPE_INDEX, block.data, block_size,\n+\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tstrbuf_init(&recs[i].u.idx.last_key, 9);\n+\n+\t\trecs[i].type = BLOCK_TYPE_INDEX;\n+\t\tstrbuf_addf(&recs[i].u.idx.last_key, \"branch%02\"PRIuMAX, (uintmax_t)i);\n+\t\trecs[i].u.idx.offset = i;\n+\n+\t\tret = block_writer_add(&bw, &recs[i]);\n+\t\tcheck_int(ret, ==, 0);\n+\t}\n+\n+\tret = block_writer_finish(&bw);\n+\tcheck_int(ret, >, 0);\n+\n+\tblock_writer_release(&bw);\n+\n+\tblock_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);\n+\n+\tblock_iter_seek_start(&it, &br);\n+\n+\tfor (i = 0; ; i++) {\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, >=, 0);\n+\t\tif (ret > 0) {\n+\t\t\tcheck_int(i, ==, N);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tfor (i = 0; i < N; i++) {\n+\t\tblock_iter_reset(&it);\n+\t\treftable_record_key(&recs[i], &want);\n+\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tcheck(reftable_record_equal(&recs[i], &rec, GIT_SHA1_RAWSZ));\n+\n+\t\twant.len--;\n+\t\tret = block_iter_seek_key(&it, &br, &want);\n+\t\tcheck_int(ret, ==, 0);\n+\n+\t\tret = block_iter_next(&it, &rec);\n+\t\tcheck_int(ret, ==, 0);\n+\t\tcheck(reftable_record_equal(&recs[10 * (i / 10)], &rec, GIT_SHA1_RAWSZ));\n+\t}\n+\n+\tblock_reader_release(&br);\n+\tblock_iter_close(&it);\n+\treftable_record_release(&rec);\n+\treftable_block_done(&br.block);\n+\tstrbuf_release(&want);\n+\tstrbuf_release(&buf);\n+\tfor (i = 0; i < N; i++)\n+\t\treftable_record_release(&recs[i]);\n+}\n+\n int cmd_main(int argc, const char *argv[])\n {\n+\tTEST(t_index_block_read_write(), \"read-write operations on index blocks work\");\n \tTEST(t_log_block_read_write(), \"read-write operations on log blocks work\");\n \tTEST(t_obj_block_read_write(), \"read-write operations on obj blocks work\");\n \tTEST(t_ref_block_read_write(), \"read-write operations on ref blocks work\");\n-- \n2.45.GIT\n\n"},{"id":"501442","messageId":"xmqqle0p26s3.fsf@gitster.g","threadId":"61947","inReplyTo":"ZsWXF_zJTIsp8XOE@tanuki","subject":"Re: [PATCH v2 09/11] t-reftable-block: add tests for log blocks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-21T16:13:32Z","receivedAt":"2024-08-21T16:13:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n>> +\tREFTABLE_CALLOC_ARRAY(block.data, block_size);\n>> +\tblock.len = block_size;\n>> +\tblock.source = malloc_block_source();\n>> +\tblock_writer_init(&bw, BLOCK_TYPE_LOG, block.data, block_size,\n>> +\t\t\t header_off, hash_size(GIT_SHA1_FORMAT_ID));\n>\n> Nit: instead of a `malloc_block_source()`, you may use\n> `block_source_from_strbuf()`. The former will go away with the patch\n> series at [1].\n\nI noticed that need for rewriting while merging the topic. If the\npatch uses the surviving alternative, that's one fewer thing I have\nto worry about ;-).\n\nThanks.\n"},{"id":"501488","messageId":"ZsbdFU9aBQvqE3pb@tanuki","threadId":"61947","inReplyTo":"20240821124150.4463-1-chandrapratap3519@gmail.com","subject":"Re: [GSoC][PATCH v3 00/11] t: port reftable/block_test.c to the unit testing framework","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-08-22T06:39:17Z","receivedAt":"2024-08-22T06:39:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Aug 21, 2024 at 06:00:50PM +0530, Chandra Pratap wrote:\n> The reftable library comes with self tests, which are exercised\n> as part of the usual end-to-end tests and are designed to\n> observe the end-user visible effects of Git commands. What it\n> exercises, however, is a better match for the unit-testing\n> framework, merged at 8bf6fbd0 (Merge branch 'js/doc-unit-tests',\n> 2023-12-09), which is designed to observe how low level\n> implementation details, at the level of sequences of individual\n> function calls, behave.\n> \n> Hence, port reftable/block_test.c to the unit testing framework and\n> improve upon the ported test. The first patch in the series moves\n> the test to the unit testing framework, and the rest of the patches\n> improve upon the ported test.\n> \n> Mentored-by: Patrick Steinhardt \n> Mentored-by: Christian Couder \n> Signed-off-by: Chandra Pratap \n> \n> ---\n> Changes in v3:\n> - Use block_source_from_strbuf() instead of malloc_block_source()\n> in log, obj, and index block tests (patches 9, 10, 11).\n> - Remove a line that causes a memory leak in obj test (patch 10).\n\nThis addresses all comments I had on the preceding version and\nplugs the memory leak. So this looks good to me, thanks!\n\nPatrick\n"},{"id":"501552","messageId":"xmqqwmk8qvxe.fsf@gitster.g","threadId":"61947","inReplyTo":"ZsbdFU9aBQvqE3pb@tanuki","subject":"Re: [GSoC][PATCH v3 00/11] t: port reftable/block_test.c to the unit testing framework","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-08-22T18:01:01Z","receivedAt":"2024-08-22T18:01:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n>> - Use block_source_from_strbuf() instead of malloc_block_source()\n>> in log, obj, and index block tests (patches 9, 10, 11).\n>> - Remove a line that causes a memory leak in obj test (patch 10).\n>\n> This addresses all comments I had on the preceding version and\n> plugs the memory leak. So this looks good to me, thanks!\n\nThanks, both. Let me mark it for 'next', then.\n"}]}