From: Phillip Wood Date: Wed, 27 Dec 2023 14:40:01 GMT Subject: Re: [PATCH] Port helper/test-ctype.c to unit-tests/t-ctype.c Message-ID: In-Reply-To: On 27/12/2023 11:57, René Scharfe wrote: > Am 27.12.23 um 11:57 schrieb Christian Couder: >> On Tue, Dec 26, 2023 at 7:46 PM Junio C Hamano wrote: >>> >>> Achu Luma writes: >> >>>> +/* Macro to test a character type */ >>>> +#define TEST_CTYPE_FUNC(func, string) \ >>>> +static void test_ctype_##func(void) \ >>>> +{ \ >>>> + int i; \ >>>> + for (i = 0; i < 256; i++) \ >>>> + check_int(func(i), ==, is_in(string, i)); \ >>>> +} >>> >>> Now, we let check_int() to do the checking for each and every byte >>> value for the class. check_int() uses different reporting and shows >>> the problematic value in a way that is more verbose and at the same >>> time is a less specific and harder to understand: >>> >>> test_msg(" left: %"PRIdMAX, a); >>> test_msg(" right: %"PRIdMAX, b); >>> >>> But that is probably the price to pay to use a more generic >>> framework, I guess. >> >> I have added Phillip and Josh in Cc: as they might have ideas about this. > > You can write custom messages for custom tests using test_assert(). Another possibility is to do for (int i = 0; i < 256; i++) { if (!check_int(func(i), ==, is_in(string, i)) test_msg(" i: %02x", i); } To print the character code as well as the actual and expected return values of check_int(). The funny spacing is intended to keep the output aligned. I did wonder if we should be using check(func(i) == is_in(string, i)) instead of check_int() but I think it is useful to have the return value printed on error in case we start returning "53" instead of "1" for "true" [1]. With the extra test_msg() above we can now see if the test fails because of a mis-categorization or because func() returned a different non-zero value when we were expecting "1". >> Also it might not be a big issue here, but when the new unit test >> framework was proposed, I commented on the fact that "left" and >> "right" were perhaps a bit less explicit than "actual" and "expected". If people are worried about this then it would be possible to change the check_xxx() macros pass the stringified relational operator into the various check_xxx_loc() functions and then print "expected" and "actual" when the operator is "==" and "left" and "right" otherwise. Best Wishes Phillip [1] As an aside I wonder if the ctype functions would make good test balloons for using _Bool by changing sane_istest() to be #define sane_istest(x,mask) ((bool)(sane_ctype[(unsigned char)(x)] & (mask))) so that we check casting to _Bool coerces non-zero values to "1"