Re: [PATCH 3/5] t/unit-tests: convert oidmap test to use clar

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Seyi

On 20/02/2025 08:29, Seyi Kuforiji wrote:
Adapt oidmap test script to clar framework by using clar assertions
where necessary. `cl_parse_any_oid` handles the necessary checks needed
for the test to run smoothly.

I'm not sure what the last half of this sentence means. What checks are performed and how does that lead to the test running smoothly?

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.

This streamlines the test suite, making individual tests self-contained
and reducing redundant code.

This seems to be saying that by sharing global state we're making the tests self-contained - I'm not sure how that can be true. We need to move to sharing a single oidmap between all the tests because clar's setup and teardown functions don't take a context pointer. That's fine but I don't see how it makes the tests self-contained.

Everything up to this point looks good.

  	while ((entry = oidmap_iter_next(&iter))) {
-		int ret;
-		if (!check_int((ret = key_val_contains(entry, seen)), ==, 0)) {
-			switch (ret) {
-			case -1:
-				break; /* error message handled by get_oid_arbitrary_hex() */
-			case 1:
-				test_msg("obtained entry was not given in the input\n"
-					 "  name: %s\n   oid: %s\n",
-					 entry->name, oid_to_hex(&entry->entry.oid));
-				break;
-			case 2:
-				test_msg("duplicate entry detected\n"
-					 "  name: %s\n   oid: %s\n",
-					 entry->name, oid_to_hex(&entry->entry.oid));
-				break;
-			default:
-				test_msg("BUG: invalid return value (%d) from key_val_contains()",
-					 ret);
-				break;
-			}
-		} else {
-			count++;
-		}
+		cl_assert_equal_i(key_val_contains(entry, seen), 0);

I think wed' be better to use clar_fail_f() so that we can keep the helpful error messages. Using cl_assert_equal_i() isn't terrible as if the test fails at least we know the error code but as we already have the logic in place to provide better messages lets adapt it.

There is a change of behavior here as before we'd loop through the whole list of entries detecting all the errors. Now we quit on the first error. I don't think that matters but it would be good to point out the change in the commit message.

Best Wishes

Phillip




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux