Re: [PATCH] Port helper/test-ctype.c to unit-tests/t-ctype.c
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 27, 2023, 23:48 UTC
- Message-ID
- <xmqqcyurky00.fsf@gitster.g>
- In-Reply-To
- <f743b473-40f8-423d-bf5b-d42b92e5aa1b@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 9 quoted lines
>> 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. > ... > 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.
Nice.
This (and your mempool thing) being one of the early efforts to adopt the unit-test framework outside the initial set of sample tests, it is understandable that we might find what framework offers is still lacking. But at the same time, while the macro tricks demonstrated here are all amusing to read and admire, it feels a bit too much to expect that the test writers are willing to invent something like these every time they want to test.
Being a relatively faithful conversion of the original ctype tests, with its thorough enumeration of test samples and expected output, is what makes this test program require these macro tricks, and it does not have much to do with the features (or lack thereof) of the framework, I guess.
Show 18 quoted lines
> +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);
> + }
> }Somehow, the "test_assert" does not seem to be adding much value here (i.e. we can do "res = (actual == expect)" there). Is this because we want to be able to report success, too?
... goes and looks at test_assert() ...
Ah, is it because we want to be able to "skip" (which pretends that the assert() was satisified). OK, but then the error reporting from it is redundant with our own test_msg().
Everything below this line was a fun read ;-)
Thanks.
Show 51 quoted lines
> ...
> +#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();
> }