t/unit-tests: convert oid-array test to use clar test framework #1892

Seyi007 · 2025-02-12T08:34:43Z

Adapt oid-array test script to clar framework by using clar assertions where necessary.

Mentored-by: Patrick Steinhardt ps@pks.im
Signed-off-by: Seyi Kuforiji kuforiji98@gmail.com

pks-t · 2025-02-13T06:46:36Z

t/unit-tests/u-oid-array.c


+void test_oid_array__enumerate_unique(void)
+{
+	setup();


Instead of calling this explicitly you can use a test_oid_array__initialize() function.

pks-t · 2025-02-13T06:46:58Z

t/unit-tests/u-oid-array.c

+	cl_assert_equal_i(actual.nr, expect.nr);
+
+	for (i = 0; i < actual.nr; i++)
+		cl_assert(oideq(&actual.oid[i], &expect.oid[i]));

 	oid_array_clear(&actual);
 	oid_array_clear(&input);
 	oid_array_clear(&expect);
 }

 #define TEST_ENUMERATION(input, expect, desc)                                     \


The description isn't used, so it can be dropped.

pks-t · 2025-02-13T06:47:53Z

t/unit-tests/unit-test.c

@@ -2,6 +2,9 @@
 #include "parse-options.h"
 #include "string-list.h"
 #include "strvec.h"
+#include "strbuf.h"
+#include "hex.h"
+#include "unit-test.h"


Let's keep headers sorted alphabetically (except for "unit-test.h", which is expected to come first. You also got that one duplicated now.

pks-t · 2025-02-13T06:48:59Z

t/unit-tests/unit-test.c

+	return algo;
+}
+
+static int get_oid_arbitrary_hex_algop(const char *hex, struct object_id *oid,


I wonder whether we want to give these functions names which give a bit of a better hint that they are part of the test suite. E.g. cl_parse_oid().

pks-t · 2025-02-13T06:49:32Z

t/unit-tests/unit-test.c

+	cl_assert_equal_i(ret, 0);
+
+	strbuf_release(&buf);
+	return ret;


The return value can be removed as you already assert that it's always 0.

pks-t · 2025-02-13T06:49:57Z

t/unit-tests/unit-test.c

+}
+
+
+int get_oid_arbitrary_hex(const char *hex, struct object_id *oid)


Same here. How about cl_parse_any_oid()?

pks-t · 2025-02-13T06:50:46Z

t/unit-tests/unit-test.c

+	int hash_algo = init_hash_algo();
+
+	cl_assert(hash_algo != GIT_HASH_UNKNOWN);
+	return get_oid_arbitrary_hex_algop(hex, oid, &hash_algos[hash_algo]);


Do we want to also assert that the return value is 0? I guess that should be a safe assumption given that this is a helper. We can then drop the return value again.

pks-t · 2025-02-17T06:16:36Z

t/unit-tests/u-oidmap.c

@@ -0,0 +1,152 @@
+#include "unit-test.h"
+#include "lib-oid.h"


This header does not need to be included as it only contains the "old" assertions used by the other unit testing framework, right?

pks-t · 2025-02-17T06:18:39Z

t/unit-tests/u-oidmap.c

+		FLEX_ALLOC_STR(entry, name, key_val[i][1]);
+		cl_parse_any_oid(key_val[i][0], &entry->entry.oid);
+		entry = oidmap_put(&map, entry);
+		cl_assert(entry == NULL);


You can probably drop the entry variable and directly cl_assert_equal_p(oidmap_put(&map, entry), NULL). The same is also true in other cases.

pks-t · 2025-02-17T06:21:09Z

t/unit-tests/u-oidmap.c

+					  { "22", "two" },
+					  { "33", "three" } };
+
+static void setup(void (*f)(struct oidmap *map))


Instead of using a setup() function, let's use a test_oidmap__initialize() function that gets executed automatically and initializes a global variable, as well as a test_oidmap__cleanup() function that cleans up state. When we have that we can get rid of setup() and also of functions like test_oidmap__replace(), where t_replace_() itself would be renamed.

pks-t · 2025-02-17T06:21:39Z

t/unit-tests/u-oidmap.c

+	oidmap_iter_init(map, &iter);
+	while ((entry = oidmap_iter_next(&iter))) {
+		int ret;
+		cl_assert_equal_i(ret = key_val_contains(entry, seen), 0);


ret is unneeded here, so we can drop it.

gitgitgadget-git · 2025-02-19T09:08:34Z

There are issues in commit f1ab0d6:
t/unit-tests: Remove lib-oid.{c,h,o}
Prefixed commit message must be in lower case
Commit not signed off

gitgitgadget-git · 2025-02-19T09:16:05Z

There are issues in commit 504a4bb:
t/unit-tests: Remove lib-oid.{c,h,o}
Prefixed commit message must be in lower case
Commit not signed off

gitgitgadget-git · 2025-02-19T09:21:39Z

There are issues in commit 7cdea9e:
t/unit-tests: remove lib-oid.{c,h,o}
Commit not signed off

ssss1524s

معالجة جمبع الاخطا

ssss1524s · 2025-02-19T13:23:03Z

معالجة السطر 732

`get_oid_arbitrary_hex()` and `init_hash_algo()` are both required for oid-related tests to run without errors. In the current implementation, both functions are defined and declared in the `t/unit-tests/lib-oid.{c,h}` which is utilized by oid-related tests in the homegrown unit tests structure. Adapt functions in lib-oid.{c,h} to use clar. Both these functions become available for oid-related test files implemented using the clar testing framework, which requires them. This will be used by subsequent commits. Mentored-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>

Adapt oid-array test script to clar framework by using clar assertions where necessary. Remove descriptions from macros to reduce redundancy, and move test input arrays to global scope for reuse across multiple test functions. Introduce `test_oid_array__initialize()` to explicitly initialize the hash algorithm. These changes streamline the test suite, making individual tests self-contained and reducing redundant code. Mentored-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>

Adapt oidmap test script to clar framework by using clar assertions where necessary. `cl_parse_any_oid()` ensures the hash algorithm is set before parsing. This prevents issues from an uninitialized or invalid hash algorithm. Introduce 'test_oidmap__initialize` handles the to set up of the global oidmap map with predefined key-value pairs, and `test_oidmap__cleanup` frees the oidmap and its entries when all tests are completed. The test loops through all entries to detect multiple errors. With this change, it stops at the first error encountered, making it easier to address it. Mentored-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>

Adapt oidtree test script to clar framework by using clar assertions where necessary. `cl_parse_any_oid()` ensures the hash algorithm is set before parsing. This prevents issues from an uninitialized or invalid hash algorithm. Introduce 'test_oidtree__initialize` handles the to set up of the global oidtree variable and `test_oidtree__cleanup` frees the oidtree when all tests are completed. With this change, `check_each` stops at the first error encountered, making it easier to address it. Mentored-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>

Seyi007 force-pushed the convert-2-clar-batch-3 branch 2 times, most recently from 821d977 to 61cfc43 Compare February 12, 2025 12:05

pks-t reviewed Feb 13, 2025

View reviewed changes

Seyi007 force-pushed the convert-2-clar-batch-3 branch 5 times, most recently from 998ff22 to 7c9c2b8 Compare February 14, 2025 10:29

pks-t reviewed Feb 17, 2025

View reviewed changes

Seyi007 force-pushed the convert-2-clar-batch-3 branch 6 times, most recently from 98604fd to f1ab0d6 Compare February 19, 2025 09:07

Seyi007 force-pushed the convert-2-clar-batch-3 branch from f1ab0d6 to 504a4bb Compare February 19, 2025 09:15

Seyi007 force-pushed the convert-2-clar-batch-3 branch from 504a4bb to 7cdea9e Compare February 19, 2025 09:20

Seyi007 force-pushed the convert-2-clar-batch-3 branch from 7cdea9e to c7ae177 Compare February 19, 2025 09:30

ssss1524s reviewed Feb 19, 2025

View reviewed changes

Seyi007 force-pushed the convert-2-clar-batch-3 branch 3 times, most recently from c7ae177 to e81ec73 Compare February 20, 2025 08:11

Seyi007 added 4 commits February 25, 2025 10:28

Seyi007 force-pushed the convert-2-clar-batch-3 branch from e81ec73 to cda8dc1 Compare February 25, 2025 09:30

		}


		int get_oid_arbitrary_hex(const char hex, struct object_id oid)

t/unit-tests: convert oid-array test to use clar test framework #1892

Are you sure you want to change the base?

t/unit-tests: convert oid-array test to use clar test framework #1892

Uh oh!

Conversation

Seyi007 commented Feb 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitgitgadget-git bot commented Feb 19, 2025

Uh oh!

gitgitgadget-git bot commented Feb 19, 2025

Uh oh!

gitgitgadget-git bot commented Feb 19, 2025

Uh oh!

ssss1524s left a comment

Choose a reason for hiding this comment

Uh oh!

ssss1524s commented Feb 19, 2025

Uh oh!

Uh oh!