Re: [PATCH v2 4/4] t-ctype: avoid duplicating class names

[Phillip Wood <phillip.wood123@gmail.com>]

Hi René

On 03/03/2024 10:13, René Scharfe wrote:
> TEST_CTYPE_FUNC defines a function for testing a character classifier,
> TEST_CHAR_CLASS calls it, causing the class name to be mentioned twice.
> 
> Avoid the need to define a class-specific function by letting
> TEST_CHAR_CLASS do all the work.  This is done by using the internal
> functions test__run_begin() and test__run_end(), but they do exist to be
> used in test macros after all.

Those internal functions exist to implement the TEST() macro, they are 
not really intended for use outside that (which is why they are marked 
as private in the header file). If we ever want to update the 
implementation of TEST() it will be a lot harder if we're using the 
internal implementation directly in test files. Unit tests should be 
wrapping TEST() if it is appropriate but not the internal implementation 
directly.

Ideally we wouldn't need TEST_CTYPE_FUNC as there would only be a single 
function that was passed a ctype predicate, an input array and an array 
of expected results. Unfortunately I don't think that is possible due 
the the way the ctype predicates are implemented. Having separate macros 
to define the test function and to run the test is annoying but I don't 
think it is really worth exposing the internal implementation just to 
avoid it.

The other patches here look like useful improvements - thanks.

Best Wishes

Phillip

> Alternatively we could unroll the loop to provide a very long expression
> that tests all 256 characters and EOF and hand that to TEST, but that
> seems awkward and hard to read.
> 
> No change of behavior or output intended.
> 
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>   t/unit-tests/t-ctype.c | 64 ++++++++++++++++--------------------------
>   1 file changed, 24 insertions(+), 40 deletions(-)
> 
> diff --git a/t/unit-tests/t-ctype.c b/t/unit-tests/t-ctype.c
> index 02d8569aa3..d6ac1fe678 100644
> --- a/t/unit-tests/t-ctype.c
> +++ b/t/unit-tests/t-ctype.c
> @@ -1,19 +1,19 @@
>   #include "test-lib.h"
> 
> -/* Macro to test a character type */
> -#define TEST_CTYPE_FUNC(func, string) \
> -static void test_ctype_##func(void) { \
> +#define TEST_CHAR_CLASS(class, string) do { \
>   	size_t len = ARRAY_SIZE(string) - 1 + \
>   		BUILD_ASSERT_OR_ZERO(ARRAY_SIZE(string) > 0) + \
>   		BUILD_ASSERT_OR_ZERO(sizeof(string[0]) == sizeof(char)); \
> -	for (int i = 0; i < 256; i++) { \
> -		if (!check_int(func(i), ==, !!memchr(string, i, len))) \
> -			test_msg("      i: 0x%02x", i); \
> +	int skip = test__run_begin(); \
> +	if (!skip) { \
> +		for (int i = 0; i < 256; i++) { \
> +			if (!check_int(class(i), ==, !!memchr(string, i, len)))\
> +				test_msg("      i: 0x%02x", i); \
> +		} \
> +		check(!class(EOF)); \
>   	} \
> -	check(!func(EOF)); \
> -}
> -
> -#define TEST_CHAR_CLASS(class) TEST(test_ctype_##class(), #class " works")
> +	test__run_end(!skip, TEST_LOCATION(), #class " works"); \
> +} while (0)
> 
>   #define DIGIT "0123456789"
>   #define LOWER "abcdefghijklmnopqrstuvwxyz"
> @@ -33,37 +33,21 @@ 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 " ")
> -
>   int cmd_main(int argc, const char **argv) {
> -	/* Run all character type tests */
> -	TEST_CHAR_CLASS(isspace);
> -	TEST_CHAR_CLASS(isdigit);
> -	TEST_CHAR_CLASS(isalpha);
> -	TEST_CHAR_CLASS(isalnum);
> -	TEST_CHAR_CLASS(is_glob_special);
> -	TEST_CHAR_CLASS(is_regex_special);
> -	TEST_CHAR_CLASS(is_pathspec_magic);
> -	TEST_CHAR_CLASS(isascii);
> -	TEST_CHAR_CLASS(islower);
> -	TEST_CHAR_CLASS(isupper);
> -	TEST_CHAR_CLASS(iscntrl);
> -	TEST_CHAR_CLASS(ispunct);
> -	TEST_CHAR_CLASS(isxdigit);
> -	TEST_CHAR_CLASS(isprint);
> +	TEST_CHAR_CLASS(isspace, " \n\r\t");
> +	TEST_CHAR_CLASS(isdigit, DIGIT);
> +	TEST_CHAR_CLASS(isalpha, LOWER UPPER);
> +	TEST_CHAR_CLASS(isalnum, LOWER UPPER DIGIT);
> +	TEST_CHAR_CLASS(is_glob_special, "*?[\\");
> +	TEST_CHAR_CLASS(is_regex_special, "$()*+.?[\\^{|");
> +	TEST_CHAR_CLASS(is_pathspec_magic, "!\"#%&',-/:;<=>@_`~");
> +	TEST_CHAR_CLASS(isascii, ASCII);
> +	TEST_CHAR_CLASS(islower, LOWER);
> +	TEST_CHAR_CLASS(isupper, UPPER);
> +	TEST_CHAR_CLASS(iscntrl, CNTRL);
> +	TEST_CHAR_CLASS(ispunct, PUNCT);
> +	TEST_CHAR_CLASS(isxdigit, DIGIT "abcdefABCDEF");
> +	TEST_CHAR_CLASS(isprint, LOWER UPPER DIGIT PUNCT " ");
> 
>   	return test_done();
>   }
> --
> 2.44.0
>