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

>
> 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