Re: [PATCH v2 1/4] t/unit-tests: convert hashmap test to use clar test framework
- From
- phillip.wood123@gmail.com <phillip.wood123@gmail.com>
- Date
- Feb 3, 2025, 14:56 UTC
- Message-ID
- <288fbcf3-f6c1-46e4-8124-f4e566abe8af@gmail.com>
- In-Reply-To
- <Z6Bwr6hM54nu8nSS@pks.im>
Hi Patrick
On 03/02/2025 07:30, Patrick Steinhardt wrote:
Show 24 quoted lines
> On Sun, Feb 02, 2025 at 11:09:25AM +0000, phillip.wood123@gmail.com wrote:
>> On 31/01/2025 22:14, Seyi Kuforiji wrote:
>
>> diff --git a/t/unit-tests/clar/clar.c b/t/unit-tests/clar/clar.c
>> index d54e4553674..16f86c952f7 100644
>> --- a/t/unit-tests/clar/clar.c
>> +++ b/t/unit-tests/clar/clar.c
>> @@ -754,7 +754,12 @@ void clar__assert_equal(
>> p_snprintf(buf, sizeof(buf), "'%s' != '%s' (at byte %d)",
>> s1, s2, pos);
>> } else {
>> - p_snprintf(buf, sizeof(buf), "'%s' != '%s'", s1, s2);
>> + const char *q1 = s1 ? "'" : "";
>> + const char *q2 = s2 ? "'" : "";
>> + s1 = s1 ? s1 : "NULL";
>> + s2 = s2 ? s2 : "NULL";
>> + p_snprintf(buf, sizeof(buf), "%s%s%s != %s%s%s",
>> + q1, s1, q1, q2, s2, q2);
>> }
>> }
>> }
>
> Would you mind creating an upstream pull request with these changes? I'm
> happy to review, and then we can update our embedded version of clar.I've opened a PR at https://github.com/clar-test/clar/pull/114
Show 16 quoted lines
>>> for (size_t i = 0; i < ARRAY_SIZE(query); i++) {
>>> entry = get_test_entry(map, query[i][0], ignore_case);
>>> - if (check(entry != NULL))
>>> - check_str(get_value(entry), query[i][1]);
>>> - else
>>> - test_msg("query key: %s", query[i][0]);
>>
>> It is a shame that we're removing all of the helpful debugging messages
>> from this test. It would be much nicer if we could keep them by using an
>> if statement and cl_failf() as we do in u-ctype.c
>
> I honestly think that the debug messages don't add much and only add to
> the noise. You shouldn't ever see them, and if you do something is
> broken and you'll likely end up pulling out the debugger anyway. So I'm
> more in the camp of writing unit tests in a concise way rather than the
> needlessly-verbose style we previously had.If I'm firing up the debugger I'd rather as much detail as I can about what went wrong so I can see where to set my breakpoints. Otherwise I need to waste time repeating the test to find out exactly what went wrong before I can make any progress. My experience with debugging our integration tests is that those tests that take care to print helpful diagnostic messages when they fail are a lot easier to debug than those that don't.
Best Wishes
Phillip