Re: [PATCH 1/2] unicode: kunit: refactor selftest to kunit tests

[Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>]

On 9/23/24 11:34, Gabriela Bittencourt wrote:
> Instead of creating 'test' functions, use kunit functions to test
> utf-8 support in unicode subsystem.

Can you elaborate on the reefactoring changes. This will help
others who would want to take on such refactoring in the future.

> Co-developed-by: Pedro Orlando <porlando@xxxxxxxxxx>
> Signed-off-by: Pedro Orlando <porlando@xxxxxxxxxx>
> Co-developed-by: Danilo Pereira <dpereira@xxxxxxxxxx>
> Signed-off-by: Danilo Pereira <dpereira@xxxxxxxxxx>
> Signed-off-by: Gabriela Bittencourt <gbittencourt@xxxxxxxxxx>
> ---
>   fs/unicode/.kunitconfig    |   3 +
>   fs/unicode/Kconfig         |   5 +-
>   fs/unicode/Makefile        |   2 +-
>   fs/unicode/utf8-selftest.c | 152 +++++++++++++++++--------------------
>   4 files changed, 76 insertions(+), 86 deletions(-)
>   create mode 100644 fs/unicode/.kunitconfig
>
> diff --git a/fs/unicode/.kunitconfig b/fs/unicode/.kunitconfig
> new file mode 100644
> index 000000000000..62dd5c171f9c
> --- /dev/null
> +++ b/fs/unicode/.kunitconfig
> @@ -0,0 +1,3 @@
> +CONFIG_KUNIT=y
> +CONFIG_UNICODE=y
> +CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST=y
> diff --git a/fs/unicode/Kconfig b/fs/unicode/Kconfig
> index da786a687fdc..4ad2c36550f1 100644
> --- a/fs/unicode/Kconfig
> +++ b/fs/unicode/Kconfig
> @@ -10,6 +10,7 @@ config UNICODE
>   	  be a separate loadable module that gets requested only when a file
>   	  system actually use it.
>   
> -config UNICODE_NORMALIZATION_SELFTEST
> +config UNICODE_NORMALIZATION_KUNIT_TEST
>   	tristate "Test UTF-8 normalization support"
> -	depends on UNICODE
> +	depends on UNICODE && KUNIT
> +	default KUNIT_ALL_TESTS
> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
> index e309afe2b2bb..37bbcbc628a1 100644
> --- a/fs/unicode/Makefile
> +++ b/fs/unicode/Makefile
> @@ -4,7 +4,7 @@ ifneq ($(CONFIG_UNICODE),)
>   obj-y			+= unicode.o
>   endif
>   obj-$(CONFIG_UNICODE)	+= utf8data.o
> -obj-$(CONFIG_UNICODE_NORMALIZATION_SELFTEST) += utf8-selftest.o
> +obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += utf8-selftest.o
>   
>   unicode-y := utf8-norm.o utf8-core.o
>   
> diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/utf8-selftest.c
> index 600e15efe9ed..54ded8db6b1c 100644
> --- a/fs/unicode/utf8-selftest.c
> +++ b/fs/unicode/utf8-selftest.c
> @@ -1,38 +1,18 @@
>   // SPDX-License-Identifier: GPL-2.0-only
>   /*
> - * Kernel module for testing utf-8 support.
> + * KUnit tests for utf-8 support
>    *
>    * Copyright 2017 Collabora Ltd.
>    */
>   
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/module.h>
> -#include <linux/printk.h>
>   #include <linux/unicode.h>
> -#include <linux/dcache.h>
> +#include <kunit/test.h>
>   
>   #include "utf8n.h"
>   
> -static unsigned int failed_tests;
> -static unsigned int total_tests;
> -
>   /* Tests will be based on this version. */
>   #define UTF8_LATEST	UNICODE_AGE(12, 1, 0)
>   
> -#define _test(cond, func, line, fmt, ...) do {				\
> -		total_tests++;						\
> -		if (!cond) {						\
> -			failed_tests++;					\
> -			pr_err("test %s:%d Failed: %s%s",		\
> -			       func, line, #cond, (fmt?":":"."));	\
> -			if (fmt)					\
> -				pr_err(fmt, ##__VA_ARGS__);		\
> -		}							\
> -	} while (0)
> -#define test_f(cond, fmt, ...) _test(cond, __func__, __LINE__, fmt, ##__VA_ARGS__)
> -#define test(cond) _test(cond, __func__, __LINE__, "")
> -
>   static const struct {
>   	/* UTF-8 strings in this vector _must_ be NULL-terminated. */
>   	unsigned char str[10];
> @@ -158,22 +138,22 @@ static const struct {
>   	}
>   };
>   
> -static ssize_t utf8len(const struct unicode_map *um, enum utf8_normalization n,
> -		const char *s)
> +static ssize_t utf8len(const struct unicode_map *um, enum utf8_normalization n, const char *s)

Keep "const char *s" on the second line.

>   {
>   	return utf8nlen(um, n, s, (size_t)-1);
>   }
>   

Rest looks good to me.

thanks,
-- Shuah