Am 27.12.23 um 11:57 schrieb Christian Couder: > On Tue, Dec 26, 2023 at 7:46 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: >> >> Achu Luma <ach.lumap@xxxxxxxxx> 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(). > 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". True. >>> +int cmd_main(int argc, const char **argv) { >>> + /* Run all character type tests */ >>> + TEST(test_ctype_isspace(), "isspace() works as we expect"); >>> + TEST(test_ctype_isdigit(), "isdigit() works as we expect"); >>> + TEST(test_ctype_isalpha(), "isalpha() works as we expect"); >>> + TEST(test_ctype_isalnum(), "isalnum() works as we expect"); >>> + TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect"); >>> + TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect"); >>> + TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect"); >>> + TEST(test_ctype_isascii(), "isascii() works as we expect"); >>> + TEST(test_ctype_islower(), "islower() works as we expect"); >>> + TEST(test_ctype_isupper(), "isupper() works as we expect"); >>> + TEST(test_ctype_iscntrl(), "iscntrl() works as we expect"); >>> + TEST(test_ctype_ispunct(), "ispunct() works as we expect"); >>> + TEST(test_ctype_isxdigit(), "isxdigit() works as we expect"); >>> + TEST(test_ctype_isprint(), "isprint() works as we expect"); >>> + >>> + return test_done(); >>> +} >> >> As a practice to use the unit-tests framework, the patch looks OK. The added repetition is a bit grating. With a bit of setup, loop unrolling and stringification you can retain the property of only having to mention the class name once. Demo patch below. >> helper/test-ctype.c indeed is an oddball that runs once and checks >> everything it wants to check, for which the unit tests framework is >> much more suited. > > Yeah, I agree. Indeed: test-ctype does unit tests, so the unit test framework should be a perfect fit. It still feels a bit raw that this point, though, but that's to be expected. René diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c index 41189ba9f9..7903856cec 100644 --- a/t/unit-tests/t-ctype.c +++ b/t/unit-tests/t-ctype.c @@ -13,13 +13,23 @@ static int is_in(const char *s, int ch) return !!strchr(s, ch); } -/* 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)); \ +struct ctype { + const char *name; + const char *expect; + int actual[256]; +}; + +static void test_ctype(const struct ctype *class) +{ + for (int i = 0; i < 256; i++) { + int expect = is_in(class->expect, i); + int actual = class->actual[i]; + int res = test_assert(TEST_LOCATION(), class->name, + actual == expect); + if (!res) + test_msg("%s classifies char %d (0x%02x) wrongly", + class->name, i, i); + } } #define DIGIT "0123456789" @@ -40,37 +50,39 @@ static void test_ctype_##func(void) \ "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \ "\x7f" -TEST_CTYPE_FUNC(isdigit, DIGIT) -TEST_CTYPE_FUNC(isspace, " \n\r\t") -TEST_CTYPE_FUNC(isalpha, LOWER UPPER) -TEST_CTYPE_FUNC(isalnum, LOWER UPPER DIGIT) -TEST_CTYPE_FUNC(is_glob_special, "*?[\\") -TEST_CTYPE_FUNC(is_regex_special, "$()*+.?[\\^{|") -TEST_CTYPE_FUNC(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~") -TEST_CTYPE_FUNC(isascii, ASCII) -TEST_CTYPE_FUNC(islower, LOWER) -TEST_CTYPE_FUNC(isupper, UPPER) -TEST_CTYPE_FUNC(iscntrl, CNTRL) -TEST_CTYPE_FUNC(ispunct, PUNCT) -TEST_CTYPE_FUNC(isxdigit, DIGIT "abcdefABCDEF") -TEST_CTYPE_FUNC(isprint, LOWER UPPER DIGIT PUNCT " ") +#define APPLY16(f, n) \ + f(n + 0x0), f(n + 0x1), f(n + 0x2), f(n + 0x3), \ + f(n + 0x4), f(n + 0x5), f(n + 0x6), f(n + 0x7), \ + f(n + 0x8), f(n + 0x9), f(n + 0xa), f(n + 0xb), \ + f(n + 0xc), f(n + 0xd), f(n + 0xe), f(n + 0xf) +#define APPLY256(f) \ + APPLY16(f, 0x00), APPLY16(f, 0x10), APPLY16(f, 0x20), APPLY16(f, 0x30),\ + APPLY16(f, 0x40), APPLY16(f, 0x50), APPLY16(f, 0x60), APPLY16(f, 0x70),\ + APPLY16(f, 0x80), APPLY16(f, 0x90), APPLY16(f, 0xa0), APPLY16(f, 0xb0),\ + APPLY16(f, 0xc0), APPLY16(f, 0xd0), APPLY16(f, 0xe0), APPLY16(f, 0xf0),\ + +#define CTYPE(name, expect) { #name, expect, { APPLY256(name) } } int cmd_main(int argc, const char **argv) { + struct ctype classes[] = { + CTYPE(isdigit, DIGIT), + CTYPE(isspace, " \n\r\t"), + CTYPE(isalpha, LOWER UPPER), + CTYPE(isalnum, LOWER UPPER DIGIT), + CTYPE(is_glob_special, "*?[\\"), + CTYPE(is_regex_special, "$()*+.?[\\^{|"), + CTYPE(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~"), + CTYPE(isascii, ASCII), + CTYPE(islower, LOWER), + CTYPE(isupper, UPPER), + CTYPE(iscntrl, CNTRL), + CTYPE(ispunct, PUNCT), + CTYPE(isxdigit, DIGIT "abcdefABCDEF"), + CTYPE(isprint, LOWER UPPER DIGIT PUNCT " "), + }; /* Run all character type tests */ - TEST(test_ctype_isspace(), "isspace() works as we expect"); - TEST(test_ctype_isdigit(), "isdigit() works as we expect"); - TEST(test_ctype_isalpha(), "isalpha() works as we expect"); - TEST(test_ctype_isalnum(), "isalnum() works as we expect"); - TEST(test_ctype_is_glob_special(), "is_glob_special() works as we expect"); - TEST(test_ctype_is_regex_special(), "is_regex_special() works as we expect"); - TEST(test_ctype_is_pathspec_magic(), "is_pathspec_magic() works as we expect"); - TEST(test_ctype_isascii(), "isascii() works as we expect"); - TEST(test_ctype_islower(), "islower() works as we expect"); - TEST(test_ctype_isupper(), "isupper() works as we expect"); - TEST(test_ctype_iscntrl(), "iscntrl() works as we expect"); - TEST(test_ctype_ispunct(), "ispunct() works as we expect"); - TEST(test_ctype_isxdigit(), "isxdigit() works as we expect"); - TEST(test_ctype_isprint(), "isprint() works as we expect"); + for (int i = 0; i < ARRAY_SIZE(classes); i++) + TEST(test_ctype(&classes[i]), "%s works", classes[i].name); return test_done(); }