[PATCH v2 0/2] unicode: kunit: refactor selftest to kunit tests

[PATCH v2 0/2] unicode: kunit: refactor selftest to kunit tests

[Pedro Orlando]

Hey all,

We are making these changes as part of a KUnit Hackathon at LKCamp [1].

This patch sets out to refactor fs/unicode/utf8-selftest.c to KUnit tests.

The main benefit of this change is that we can leverage KUnit's
test suite for quickly compiling and testing the functions in utf8,
instead of compiling the kernel and loading the previous utf8-selftest
module, as well as adopting a pattern across all kernel tests.

The first commit is the refactoring itself from self test into KUnit,
which kept the original test logic intact -- maintaining the purpose
of the original tests -- with the added benefit of including these
tests into the KUnit test suite.

The second commit applies the naming style and file path conventions
defined on Documentation/dev-tools/kunit/style.rst

We appreciate any feedback and suggestions. :)

[1] https://lkcamp.dev/about/

Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>

Gabriela Bittencourt (2):
  unicode: kunit: refactor selftest to kunit tests
  unicode: kunit: change tests filename and path

 fs/unicode/Kconfig                            |   5 +-
 fs/unicode/Makefile                           |   2 +-
 fs/unicode/tests/.kunitconfig                 |   3 +
 .../{utf8-selftest.c => tests/utf8_kunit.c}   | 149 ++++++++----------
 4 files changed, 76 insertions(+), 83 deletions(-)
 create mode 100644 fs/unicode/tests/.kunitconfig
 rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (64%)

-- 
2.34.1

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

[Pedro Orlando]

From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>

Refactoring 'test' functions into kunit tests, to test utf-8 support in
unicode subsystem.

This allows the utf8 tests to be run alongside the KUnit test suite
using kunit-tool, quickly compiling and running all desired tests as
part of the KUnit test suite, instead of compiling the selftest module
and loading it.

The refactoring kept the original testing logic intact, while adopting a
testing pattern across different kernel modules and leveraging KUnit's
benefits.

Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
---

About the KUNIT readability, there are two ways of obtaing the results:
1- using `_TRUE(test, func == ret)` which may make the code more
readable, but the error message less informative. For example:
`false, but expect true`; unless we use _TRUE_MSG(test, cond, msg) to
customize the error message (which is what we've done here).
2- using `_EQ(test, func, ret)` which may be a little less readable, but the
default error message will carry more information. For example:
`64, but expected 0`

---
 fs/unicode/.kunitconfig    |   3 +
 fs/unicode/Kconfig         |   5 +-
 fs/unicode/Makefile        |   2 +-
 fs/unicode/utf8-selftest.c | 149 +++++++++++++++++--------------------
 4 files changed, 76 insertions(+), 83 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..52ab68ef2bbc 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];
@@ -170,69 +150,74 @@ static int utf8cursor(struct utf8cursor *u8c, const struct unicode_map *um,
 	return utf8ncursor(u8c, um, n, s, (unsigned int)-1);
 }
 
-static void check_utf8_nfdi(struct unicode_map *um)
+static void check_utf8_nfdi(struct kunit *test)
 {
 	int i;
 	struct utf8cursor u8c;
+	struct unicode_map *um = test->priv;
 
 	for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
 		int len = strlen(nfdi_test_data[i].str);
 		int nlen = strlen(nfdi_test_data[i].dec);
 		int j = 0;
 		unsigned char c;
+		int ret;
 
-		test((utf8len(um, UTF8_NFDI, nfdi_test_data[i].str) == nlen));
-		test((utf8nlen(um, UTF8_NFDI, nfdi_test_data[i].str, len) ==
-			nlen));
+		KUNIT_EXPECT_EQ(test, utf8len(um, UTF8_NFDI, nfdi_test_data[i].str), nlen);
+		KUNIT_EXPECT_EQ(test, utf8nlen(um, UTF8_NFDI, nfdi_test_data[i].str, len),
+				nlen);
 
-		if (utf8cursor(&u8c, um, UTF8_NFDI, nfdi_test_data[i].str) < 0)
-			pr_err("can't create cursor\n");
+
+		ret = utf8cursor(&u8c, um, UTF8_NFDI, nfdi_test_data[i].str);
+		KUNIT_EXPECT_TRUE_MSG(test, ret >= 0, "Can't create cursor\n");
 
 		while ((c = utf8byte(&u8c)) > 0) {
-			test_f((c == nfdi_test_data[i].dec[j]),
-			       "Unexpected byte 0x%x should be 0x%x\n",
-			       c, nfdi_test_data[i].dec[j]);
+			KUNIT_EXPECT_EQ_MSG(test, c, nfdi_test_data[i].dec[j],
+					    "Unexpected byte 0x%x should be 0x%x\n",
+					    c, nfdi_test_data[i].dec[j]);
 			j++;
 		}
 
-		test((j == nlen));
+		KUNIT_EXPECT_EQ(test, j, nlen);
 	}
 }
 
-static void check_utf8_nfdicf(struct unicode_map *um)
+static void check_utf8_nfdicf(struct kunit *test)
 {
 	int i;
 	struct utf8cursor u8c;
+	struct unicode_map *um = test->priv;
 
 	for (i = 0; i < ARRAY_SIZE(nfdicf_test_data); i++) {
 		int len = strlen(nfdicf_test_data[i].str);
 		int nlen = strlen(nfdicf_test_data[i].ncf);
 		int j = 0;
+		int ret;
 		unsigned char c;
 
-		test((utf8len(um, UTF8_NFDICF, nfdicf_test_data[i].str) ==
-				nlen));
-		test((utf8nlen(um, UTF8_NFDICF, nfdicf_test_data[i].str, len) ==
-				nlen));
+		KUNIT_EXPECT_EQ(test, utf8len(um, UTF8_NFDICF, nfdicf_test_data[i].str),
+				nlen);
+		KUNIT_EXPECT_EQ(test, utf8nlen(um, UTF8_NFDICF, nfdicf_test_data[i].str, len),
+				nlen);
 
-		if (utf8cursor(&u8c, um, UTF8_NFDICF,
-				nfdicf_test_data[i].str) < 0)
-			pr_err("can't create cursor\n");
+		ret = utf8cursor(&u8c, um, UTF8_NFDICF, nfdicf_test_data[i].str);
+		KUNIT_EXPECT_TRUE_MSG(test, ret >= 0, "Can't create cursor\n");
 
 		while ((c = utf8byte(&u8c)) > 0) {
-			test_f((c == nfdicf_test_data[i].ncf[j]),
-			       "Unexpected byte 0x%x should be 0x%x\n",
-			       c, nfdicf_test_data[i].ncf[j]);
+			KUNIT_EXPECT_EQ_MSG(test, c, nfdicf_test_data[i].ncf[j],
+					    "Unexpected byte 0x%x should be 0x%x\n",
+					    c, nfdicf_test_data[i].ncf[j]);
 			j++;
 		}
 
-		test((j == nlen));
+		KUNIT_EXPECT_EQ(test, j, nlen);
 	}
 }
 
-static void check_utf8_comparisons(struct unicode_map *table)
+static void check_utf8_comparisons(struct kunit *test)
 {
 	int i;
+	struct unicode_map *um = test->priv;
 
 	for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
 		const struct qstr s1 = {.name = nfdi_test_data[i].str,
@@ -240,8 +225,9 @@ static void check_utf8_comparisons(struct unicode_map *table)
 		const struct qstr s2 = {.name = nfdi_test_data[i].dec,
 					.len = sizeof(nfdi_test_data[i].dec)};
 
-		test_f(!utf8_strncmp(table, &s1, &s2),
-		       "%s %s comparison mismatch\n", s1.name, s2.name);
+		/* strncmp returns 0 when strings are equal */
+		KUNIT_EXPECT_TRUE_MSG(test, utf8_strncmp(um, &s1, &s2) == 0,
+				    "%s %s comparison mismatch\n", s1.name, s2.name);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(nfdicf_test_data); i++) {
@@ -250,62 +236,65 @@ static void check_utf8_comparisons(struct unicode_map *table)
 		const struct qstr s2 = {.name = nfdicf_test_data[i].ncf,
 					.len = sizeof(nfdicf_test_data[i].ncf)};
 
-		test_f(!utf8_strncasecmp(table, &s1, &s2),
-		       "%s %s comparison mismatch\n", s1.name, s2.name);
+		/* strncasecmp returns 0 when strings are equal */
+		KUNIT_EXPECT_TRUE_MSG(test, utf8_strncasecmp(um, &s1, &s2) == 0,
+				    "%s %s comparison mismatch\n", s1.name, s2.name);
 	}
 }
 
-static void check_supported_versions(struct unicode_map *um)
+static void check_supported_versions(struct kunit *test)
 {
+	struct unicode_map *um = test->priv;
 	/* Unicode 7.0.0 should be supported. */
-	test(utf8version_is_supported(um, UNICODE_AGE(7, 0, 0)));
+	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UNICODE_AGE(7, 0, 0)));
 
 	/* Unicode 9.0.0 should be supported. */
-	test(utf8version_is_supported(um, UNICODE_AGE(9, 0, 0)));
+	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UNICODE_AGE(9, 0, 0)));
 
 	/* Unicode 1x.0.0 (the latest version) should be supported. */
-	test(utf8version_is_supported(um, UTF8_LATEST));
+	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UTF8_LATEST));
 
 	/* Next versions don't exist. */
-	test(!utf8version_is_supported(um, UNICODE_AGE(13, 0, 0)));
-	test(!utf8version_is_supported(um, UNICODE_AGE(0, 0, 0)));
-	test(!utf8version_is_supported(um, UNICODE_AGE(-1, -1, -1)));
+	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(13, 0, 0)));
+	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(0, 0, 0)));
+	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(-1, -1, -1)));
 }
 
-static int __init init_test_ucd(void)
+static struct kunit_case unicode_normalization_test_cases[] = {
+	KUNIT_CASE(check_supported_versions),
+	KUNIT_CASE(check_utf8_comparisons),
+	KUNIT_CASE(check_utf8_nfdicf),
+	KUNIT_CASE(check_utf8_nfdi),
+	{}
+};
+
+static int init_test_ucd(struct kunit *test)
 {
-	struct unicode_map *um;
+	struct unicode_map *um = utf8_load(UTF8_LATEST);
 
-	failed_tests = 0;
-	total_tests = 0;
+	test->priv = um;
 
-	um = utf8_load(UTF8_LATEST);
-	if (IS_ERR(um)) {
-		pr_err("%s: Unable to load utf8 table.\n", __func__);
-		return PTR_ERR(um);
-	}
+	KUNIT_EXPECT_EQ_MSG(test, IS_ERR(um), 0,
+			    "%s: Unable to load utf8 table.\n", __func__);
 
-	check_supported_versions(um);
-	check_utf8_nfdi(um);
-	check_utf8_nfdicf(um);
-	check_utf8_comparisons(um);
-
-	if (!failed_tests)
-		pr_info("All %u tests passed\n", total_tests);
-	else
-		pr_err("%u out of %u tests failed\n", failed_tests,
-		       total_tests);
-	utf8_unload(um);
 	return 0;
 }
 
-static void __exit exit_test_ucd(void)
+static void exit_test_ucd(struct kunit *test)
 {
+	utf8_unload(test->priv);
 }
 
-module_init(init_test_ucd);
-module_exit(exit_test_ucd);
+static struct kunit_suite unicode_normalization_test_suite = {
+	.name = "unicode_normalization",
+	.test_cases = unicode_normalization_test_cases,
+	.init = init_test_ucd,
+	.exit = exit_test_ucd,
+};
+
+kunit_test_suite(unicode_normalization_test_suite);
+
 
 MODULE_AUTHOR("Gabriel Krisman Bertazi <krisman@collabora.co.uk>");
-MODULE_DESCRIPTION("Kernel module for testing utf-8 support");
+MODULE_DESCRIPTION("KUnit tests for utf-8 support.");
 MODULE_LICENSE("GPL");
-- 
2.34.1

[PATCH v2 2/2] unicode: kunit: change tests filename and path

[Pedro Orlando]

From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>

Change utf8 kunit test filename and path to follow the style
convention on Documentation/dev-tools/kunit/style.rst

Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
---
 fs/unicode/Makefile                                | 2 +-
 fs/unicode/{ => tests}/.kunitconfig                | 0
 fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} | 0
 3 files changed, 1 insertion(+), 1 deletion(-)
 rename fs/unicode/{ => tests}/.kunitconfig (100%)
 rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (100%)

diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
index 37bbcbc628a1..d95be7fb9f6b 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_KUNIT_TEST) += utf8-selftest.o
+obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += tests/utf8_kunit.o
 
 unicode-y := utf8-norm.o utf8-core.o
 
diff --git a/fs/unicode/.kunitconfig b/fs/unicode/tests/.kunitconfig
similarity index 100%
rename from fs/unicode/.kunitconfig
rename to fs/unicode/tests/.kunitconfig
diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/tests/utf8_kunit.c
similarity index 100%
rename from fs/unicode/utf8-selftest.c
rename to fs/unicode/tests/utf8_kunit.c
-- 
2.34.1

Re: [PATCH v2 2/2] unicode: kunit: change tests filename and path

[André Almeida]

Em 28/09/2024 20:58, Pedro Orlando escreveu:
> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> 
> Change utf8 kunit test filename and path to follow the style
> convention on Documentation/dev-tools/kunit/style.rst
> 
> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>

I think you missed this in:

Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>

 From 
https://lore.kernel.org/all/7e831d5b-1bc4-4d6d-99d7-b160cd580528@linuxfoundation.org/

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

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 14207 bytes --]

On Sun, 29 Sept 2024 at 08:00, Pedro Orlando <porlando@lkcamp.dev> wrote:
>
> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
>
> Refactoring 'test' functions into kunit tests, to test utf-8 support in
> unicode subsystem.
>
> This allows the utf8 tests to be run alongside the KUnit test suite
> using kunit-tool, quickly compiling and running all desired tests as
> part of the KUnit test suite, instead of compiling the selftest module
> and loading it.
>
> The refactoring kept the original testing logic intact, while adopting a
> testing pattern across different kernel modules and leveraging KUnit's
> benefits.
>
> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> ---
>
> About the KUNIT readability, there are two ways of obtaing the results:
> 1- using `_TRUE(test, func == ret)` which may make the code more
> readable, but the error message less informative. For example:
> `false, but expect true`; unless we use _TRUE_MSG(test, cond, msg) to
> customize the error message (which is what we've done here).
> 2- using `_EQ(test, func, ret)` which may be a little less readable, but the
> default error message will carry more information. For example:
> `64, but expected 0`
>
> ---


This looks good to me, and runs fine here. Thanks!

I do like the idea of keeping these as KUnit tests, even if fstests
has some unicode tests itself and the code is pretty battle-hardened,
as these are a good, quick way to sanity-check any changes during
development, and to catch regressions early.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David


>  fs/unicode/.kunitconfig    |   3 +
>  fs/unicode/Kconfig         |   5 +-
>  fs/unicode/Makefile        |   2 +-
>  fs/unicode/utf8-selftest.c | 149 +++++++++++++++++--------------------
>  4 files changed, 76 insertions(+), 83 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..52ab68ef2bbc 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];
> @@ -170,69 +150,74 @@ static int utf8cursor(struct utf8cursor *u8c, const struct unicode_map *um,
>         return utf8ncursor(u8c, um, n, s, (unsigned int)-1);
>  }
>
> -static void check_utf8_nfdi(struct unicode_map *um)
> +static void check_utf8_nfdi(struct kunit *test)
>  {
>         int i;
>         struct utf8cursor u8c;
> +       struct unicode_map *um = test->priv;
>
>         for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
>                 int len = strlen(nfdi_test_data[i].str);
>                 int nlen = strlen(nfdi_test_data[i].dec);
>                 int j = 0;
>                 unsigned char c;
> +               int ret;
>
> -               test((utf8len(um, UTF8_NFDI, nfdi_test_data[i].str) == nlen));
> -               test((utf8nlen(um, UTF8_NFDI, nfdi_test_data[i].str, len) ==
> -                       nlen));
> +               KUNIT_EXPECT_EQ(test, utf8len(um, UTF8_NFDI, nfdi_test_data[i].str), nlen);
> +               KUNIT_EXPECT_EQ(test, utf8nlen(um, UTF8_NFDI, nfdi_test_data[i].str, len),
> +                               nlen);
>
> -               if (utf8cursor(&u8c, um, UTF8_NFDI, nfdi_test_data[i].str) < 0)
> -                       pr_err("can't create cursor\n");
> +
> +               ret = utf8cursor(&u8c, um, UTF8_NFDI, nfdi_test_data[i].str);
> +               KUNIT_EXPECT_TRUE_MSG(test, ret >= 0, "Can't create cursor\n");
>
>                 while ((c = utf8byte(&u8c)) > 0) {
> -                       test_f((c == nfdi_test_data[i].dec[j]),
> -                              "Unexpected byte 0x%x should be 0x%x\n",
> -                              c, nfdi_test_data[i].dec[j]);
> +                       KUNIT_EXPECT_EQ_MSG(test, c, nfdi_test_data[i].dec[j],
> +                                           "Unexpected byte 0x%x should be 0x%x\n",
> +                                           c, nfdi_test_data[i].dec[j]);
>                         j++;
>                 }
>
> -               test((j == nlen));
> +               KUNIT_EXPECT_EQ(test, j, nlen);
>         }
>  }
>
> -static void check_utf8_nfdicf(struct unicode_map *um)
> +static void check_utf8_nfdicf(struct kunit *test)
>  {
>         int i;
>         struct utf8cursor u8c;
> +       struct unicode_map *um = test->priv;
>
>         for (i = 0; i < ARRAY_SIZE(nfdicf_test_data); i++) {
>                 int len = strlen(nfdicf_test_data[i].str);
>                 int nlen = strlen(nfdicf_test_data[i].ncf);
>                 int j = 0;
> +               int ret;
>                 unsigned char c;
>
> -               test((utf8len(um, UTF8_NFDICF, nfdicf_test_data[i].str) ==
> -                               nlen));
> -               test((utf8nlen(um, UTF8_NFDICF, nfdicf_test_data[i].str, len) ==
> -                               nlen));
> +               KUNIT_EXPECT_EQ(test, utf8len(um, UTF8_NFDICF, nfdicf_test_data[i].str),
> +                               nlen);
> +               KUNIT_EXPECT_EQ(test, utf8nlen(um, UTF8_NFDICF, nfdicf_test_data[i].str, len),
> +                               nlen);
>
> -               if (utf8cursor(&u8c, um, UTF8_NFDICF,
> -                               nfdicf_test_data[i].str) < 0)
> -                       pr_err("can't create cursor\n");
> +               ret = utf8cursor(&u8c, um, UTF8_NFDICF, nfdicf_test_data[i].str);
> +               KUNIT_EXPECT_TRUE_MSG(test, ret >= 0, "Can't create cursor\n");
>
>                 while ((c = utf8byte(&u8c)) > 0) {
> -                       test_f((c == nfdicf_test_data[i].ncf[j]),
> -                              "Unexpected byte 0x%x should be 0x%x\n",
> -                              c, nfdicf_test_data[i].ncf[j]);
> +                       KUNIT_EXPECT_EQ_MSG(test, c, nfdicf_test_data[i].ncf[j],
> +                                           "Unexpected byte 0x%x should be 0x%x\n",
> +                                           c, nfdicf_test_data[i].ncf[j]);
>                         j++;
>                 }
>
> -               test((j == nlen));
> +               KUNIT_EXPECT_EQ(test, j, nlen);
>         }
>  }
>
> -static void check_utf8_comparisons(struct unicode_map *table)
> +static void check_utf8_comparisons(struct kunit *test)
>  {
>         int i;
> +       struct unicode_map *um = test->priv;
>
>         for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
>                 const struct qstr s1 = {.name = nfdi_test_data[i].str,
> @@ -240,8 +225,9 @@ static void check_utf8_comparisons(struct unicode_map *table)
>                 const struct qstr s2 = {.name = nfdi_test_data[i].dec,
>                                         .len = sizeof(nfdi_test_data[i].dec)};
>
> -               test_f(!utf8_strncmp(table, &s1, &s2),
> -                      "%s %s comparison mismatch\n", s1.name, s2.name);
> +               /* strncmp returns 0 when strings are equal */
> +               KUNIT_EXPECT_TRUE_MSG(test, utf8_strncmp(um, &s1, &s2) == 0,
> +                                   "%s %s comparison mismatch\n", s1.name, s2.name);
>         }
>
>         for (i = 0; i < ARRAY_SIZE(nfdicf_test_data); i++) {
> @@ -250,62 +236,65 @@ static void check_utf8_comparisons(struct unicode_map *table)
>                 const struct qstr s2 = {.name = nfdicf_test_data[i].ncf,
>                                         .len = sizeof(nfdicf_test_data[i].ncf)};
>
> -               test_f(!utf8_strncasecmp(table, &s1, &s2),
> -                      "%s %s comparison mismatch\n", s1.name, s2.name);
> +               /* strncasecmp returns 0 when strings are equal */
> +               KUNIT_EXPECT_TRUE_MSG(test, utf8_strncasecmp(um, &s1, &s2) == 0,
> +                                   "%s %s comparison mismatch\n", s1.name, s2.name);
>         }
>  }
>
> -static void check_supported_versions(struct unicode_map *um)
> +static void check_supported_versions(struct kunit *test)
>  {
> +       struct unicode_map *um = test->priv;
>         /* Unicode 7.0.0 should be supported. */
> -       test(utf8version_is_supported(um, UNICODE_AGE(7, 0, 0)));
> +       KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UNICODE_AGE(7, 0, 0)));
>
>         /* Unicode 9.0.0 should be supported. */
> -       test(utf8version_is_supported(um, UNICODE_AGE(9, 0, 0)));
> +       KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UNICODE_AGE(9, 0, 0)));
>
>         /* Unicode 1x.0.0 (the latest version) should be supported. */
> -       test(utf8version_is_supported(um, UTF8_LATEST));
> +       KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UTF8_LATEST));
>
>         /* Next versions don't exist. */
> -       test(!utf8version_is_supported(um, UNICODE_AGE(13, 0, 0)));
> -       test(!utf8version_is_supported(um, UNICODE_AGE(0, 0, 0)));
> -       test(!utf8version_is_supported(um, UNICODE_AGE(-1, -1, -1)));
> +       KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(13, 0, 0)));
> +       KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(0, 0, 0)));
> +       KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(-1, -1, -1)));
>  }
>
> -static int __init init_test_ucd(void)
> +static struct kunit_case unicode_normalization_test_cases[] = {
> +       KUNIT_CASE(check_supported_versions),
> +       KUNIT_CASE(check_utf8_comparisons),
> +       KUNIT_CASE(check_utf8_nfdicf),
> +       KUNIT_CASE(check_utf8_nfdi),
> +       {}
> +};
> +
> +static int init_test_ucd(struct kunit *test)
>  {
> -       struct unicode_map *um;
> +       struct unicode_map *um = utf8_load(UTF8_LATEST);
>
> -       failed_tests = 0;
> -       total_tests = 0;
> +       test->priv = um;
>
> -       um = utf8_load(UTF8_LATEST);
> -       if (IS_ERR(um)) {
> -               pr_err("%s: Unable to load utf8 table.\n", __func__);
> -               return PTR_ERR(um);
> -       }
> +       KUNIT_EXPECT_EQ_MSG(test, IS_ERR(um), 0,
> +                           "%s: Unable to load utf8 table.\n", __func__);
>
> -       check_supported_versions(um);
> -       check_utf8_nfdi(um);
> -       check_utf8_nfdicf(um);
> -       check_utf8_comparisons(um);
> -
> -       if (!failed_tests)
> -               pr_info("All %u tests passed\n", total_tests);
> -       else
> -               pr_err("%u out of %u tests failed\n", failed_tests,
> -                      total_tests);
> -       utf8_unload(um);
>         return 0;
>  }
>
> -static void __exit exit_test_ucd(void)
> +static void exit_test_ucd(struct kunit *test)
>  {
> +       utf8_unload(test->priv);
>  }
>
> -module_init(init_test_ucd);
> -module_exit(exit_test_ucd);
> +static struct kunit_suite unicode_normalization_test_suite = {
> +       .name = "unicode_normalization",
> +       .test_cases = unicode_normalization_test_cases,
> +       .init = init_test_ucd,
> +       .exit = exit_test_ucd,
> +};
> +
> +kunit_test_suite(unicode_normalization_test_suite);
> +
>
>  MODULE_AUTHOR("Gabriel Krisman Bertazi <krisman@collabora.co.uk>");
> -MODULE_DESCRIPTION("Kernel module for testing utf-8 support");
> +MODULE_DESCRIPTION("KUnit tests for utf-8 support.");
>  MODULE_LICENSE("GPL");
> --
> 2.34.1
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]

Re: [PATCH v2 2/2] unicode: kunit: change tests filename and path

[David Gow]

On Sun, 29 Sept 2024 at 08:00, Pedro Orlando <porlando@lkcamp.dev> wrote:
>
> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
>
> Change utf8 kunit test filename and path to follow the style
> convention on Documentation/dev-tools/kunit/style.rst
>
> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> ---

This looks good to me. The only thing I'm not 100% sure about is
whether or not we want to move the .kunitconfig file into the tests/
directory. I'm leaning towards "yes", but we may want to update
kunit.py at some point to support automatically adding tests/ to the
search path for a .kunitconfig if this becomes a common enough
pattern.

Regardless, let's take this as-is.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David


>  fs/unicode/Makefile                                | 2 +-
>  fs/unicode/{ => tests}/.kunitconfig                | 0
>  fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} | 0
>  3 files changed, 1 insertion(+), 1 deletion(-)
>  rename fs/unicode/{ => tests}/.kunitconfig (100%)
>  rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (100%)
>
> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
> index 37bbcbc628a1..d95be7fb9f6b 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_KUNIT_TEST) += utf8-selftest.o
> +obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += tests/utf8_kunit.o
>
>  unicode-y := utf8-norm.o utf8-core.o
>
> diff --git a/fs/unicode/.kunitconfig b/fs/unicode/tests/.kunitconfig
> similarity index 100%
> rename from fs/unicode/.kunitconfig
> rename to fs/unicode/tests/.kunitconfig
> diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/tests/utf8_kunit.c
> similarity index 100%
> rename from fs/unicode/utf8-selftest.c
> rename to fs/unicode/tests/utf8_kunit.c
> --
> 2.34.1
>

Re: [PATCH v2 2/2] unicode: kunit: change tests filename and path

[Gabriel Krisman Bertazi]

Pedro Orlando <porlando@lkcamp.dev> writes:

> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
>
> Change utf8 kunit test filename and path to follow the style
> convention on Documentation/dev-tools/kunit/style.rst
>
> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> ---
>  fs/unicode/Makefile                                | 2 +-
>  fs/unicode/{ => tests}/.kunitconfig                | 0
>  fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} | 0
>  3 files changed, 1 insertion(+), 1 deletion(-)
>  rename fs/unicode/{ => tests}/.kunitconfig (100%)
>  rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (100%)
>
> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
> index 37bbcbc628a1..d95be7fb9f6b 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_KUNIT_TEST) += utf8-selftest.o
> +obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += tests/utf8_kunit.o

This breaks compilation for me.

fs/unicode/tests/utf8_kunit.c:11:10: fatal error: utf8n.h: No such file or directory
   11 | #include "utf8n.h"
      |          ^~~~~~~~~

After this patch that local header is now in the parent directory.

I'm building with:

CONFIG_UNICODE=m
CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST=m

>  unicode-y := utf8-norm.o utf8-core.o
>  
> diff --git a/fs/unicode/.kunitconfig b/fs/unicode/tests/.kunitconfig
> similarity index 100%
> rename from fs/unicode/.kunitconfig
> rename to fs/unicode/tests/.kunitconfig
> diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/tests/utf8_kunit.c
> similarity index 100%
> rename from fs/unicode/utf8-selftest.c
> rename to fs/unicode/tests/utf8_kunit.c

-- 
Gabriel Krisman Bertazi

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

[Gabriel Krisman Bertazi]

Pedro Orlando <porlando@lkcamp.dev> writes:

> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
>
> Refactoring 'test' functions into kunit tests, to test utf-8 support in
> unicode subsystem.
>
> This allows the utf8 tests to be run alongside the KUnit test suite
> using kunit-tool, quickly compiling and running all desired tests as
> part of the KUnit test suite, instead of compiling the selftest module
> and loading it.
>
> The refactoring kept the original testing logic intact, while adopting a
> testing pattern across different kernel modules and leveraging KUnit's
> benefits.
>
> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> ---
>
> About the KUNIT readability, there are two ways of obtaing the results:
> 1- using `_TRUE(test, func == ret)` which may make the code more
> readable, but the error message less informative. For example:
> `false, but expect true`; unless we use _TRUE_MSG(test, cond, msg) to
> customize the error message (which is what we've done here).
> 2- using `_EQ(test, func, ret)` which may be a little less readable, but the
> default error message will carry more information. For example:
> `64, but expected 0`
>
> ---
>  fs/unicode/.kunitconfig    |   3 +
>  fs/unicode/Kconfig         |   5 +-
>  fs/unicode/Makefile        |   2 +-
>  fs/unicode/utf8-selftest.c | 149 +++++++++++++++++--------------------
>  4 files changed, 76 insertions(+), 83 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

This rename is missing a spot at the bottom of utf8norm.c:

#ifdef CONFIG_UNICODE_NORMALIZATION_SELFTEST_MODULE
EXPORT_SYMBOL_GPL(utf8version_is_supported);
EXPORT_SYMBOL_GPL(utf8nlen);
EXPORT_SYMBOL_GPL(utf8ncursor);
EXPORT_SYMBOL_GPL(utf8byte);
#endif

Which is needed when UNICODE_NORMALIZATION_KUNIT_TEST=m.

>  	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..52ab68ef2bbc 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];
> @@ -170,69 +150,74 @@ static int utf8cursor(struct utf8cursor *u8c, const struct unicode_map *um,
>  	return utf8ncursor(u8c, um, n, s, (unsigned int)-1);
>  }
>  
> -static void check_utf8_nfdi(struct unicode_map *um)
> +static void check_utf8_nfdi(struct kunit *test)
>  {
>  	int i;
>  	struct utf8cursor u8c;
> +	struct unicode_map *um = test->priv;
>  
>  	for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
>  		int len = strlen(nfdi_test_data[i].str);
>  		int nlen = strlen(nfdi_test_data[i].dec);
>  		int j = 0;
>  		unsigned char c;
> +		int ret;
>  
> -		test((utf8len(um, UTF8_NFDI, nfdi_test_data[i].str) == nlen));
> -		test((utf8nlen(um, UTF8_NFDI, nfdi_test_data[i].str, len) ==
> -			nlen));
> +		KUNIT_EXPECT_EQ(test, utf8len(um, UTF8_NFDI, nfdi_test_data[i].str), nlen);
> +		KUNIT_EXPECT_EQ(test, utf8nlen(um, UTF8_NFDI, nfdi_test_data[i].str, len),
> +				nlen);
>  
> -		if (utf8cursor(&u8c, um, UTF8_NFDI, nfdi_test_data[i].str) < 0)
> -			pr_err("can't create cursor\n");
> +
> +		ret = utf8cursor(&u8c, um, UTF8_NFDI, nfdi_test_data[i].str);
> +		KUNIT_EXPECT_TRUE_MSG(test, ret >= 0, "Can't create cursor\n");
>  
>  		while ((c = utf8byte(&u8c)) > 0) {
> -			test_f((c == nfdi_test_data[i].dec[j]),
> -			       "Unexpected byte 0x%x should be 0x%x\n",
> -			       c, nfdi_test_data[i].dec[j]);
> +			KUNIT_EXPECT_EQ_MSG(test, c, nfdi_test_data[i].dec[j],
> +					    "Unexpected byte 0x%x should be 0x%x\n",
> +					    c, nfdi_test_data[i].dec[j]);
>  			j++;
>  		}
>  
> -		test((j == nlen));
> +		KUNIT_EXPECT_EQ(test, j, nlen);
>  	}
>  }
>  
> -static void check_utf8_nfdicf(struct unicode_map *um)
> +static void check_utf8_nfdicf(struct kunit *test)
>  {
>  	int i;
>  	struct utf8cursor u8c;
> +	struct unicode_map *um = test->priv;
>  
>  	for (i = 0; i < ARRAY_SIZE(nfdicf_test_data); i++) {
>  		int len = strlen(nfdicf_test_data[i].str);
>  		int nlen = strlen(nfdicf_test_data[i].ncf);
>  		int j = 0;
> +		int ret;
>  		unsigned char c;
>  
> -		test((utf8len(um, UTF8_NFDICF, nfdicf_test_data[i].str) ==
> -				nlen));
> -		test((utf8nlen(um, UTF8_NFDICF, nfdicf_test_data[i].str, len) ==
> -				nlen));
> +		KUNIT_EXPECT_EQ(test, utf8len(um, UTF8_NFDICF, nfdicf_test_data[i].str),
> +				nlen);
> +		KUNIT_EXPECT_EQ(test, utf8nlen(um, UTF8_NFDICF, nfdicf_test_data[i].str, len),
> +				nlen);
>  
> -		if (utf8cursor(&u8c, um, UTF8_NFDICF,
> -				nfdicf_test_data[i].str) < 0)
> -			pr_err("can't create cursor\n");
> +		ret = utf8cursor(&u8c, um, UTF8_NFDICF, nfdicf_test_data[i].str);
> +		KUNIT_EXPECT_TRUE_MSG(test, ret >= 0, "Can't create cursor\n");
>  
>  		while ((c = utf8byte(&u8c)) > 0) {
> -			test_f((c == nfdicf_test_data[i].ncf[j]),
> -			       "Unexpected byte 0x%x should be 0x%x\n",
> -			       c, nfdicf_test_data[i].ncf[j]);
> +			KUNIT_EXPECT_EQ_MSG(test, c, nfdicf_test_data[i].ncf[j],
> +					    "Unexpected byte 0x%x should be 0x%x\n",
> +					    c, nfdicf_test_data[i].ncf[j]);
>  			j++;
>  		}
>  
> -		test((j == nlen));
> +		KUNIT_EXPECT_EQ(test, j, nlen);
>  	}
>  }
>  
> -static void check_utf8_comparisons(struct unicode_map *table)
> +static void check_utf8_comparisons(struct kunit *test)
>  {
>  	int i;
> +	struct unicode_map *um = test->priv;
>  
>  	for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
>  		const struct qstr s1 = {.name = nfdi_test_data[i].str,
> @@ -240,8 +225,9 @@ static void check_utf8_comparisons(struct unicode_map *table)
>  		const struct qstr s2 = {.name = nfdi_test_data[i].dec,
>  					.len = sizeof(nfdi_test_data[i].dec)};
>  
> -		test_f(!utf8_strncmp(table, &s1, &s2),
> -		       "%s %s comparison mismatch\n", s1.name, s2.name);
> +		/* strncmp returns 0 when strings are equal */
> +		KUNIT_EXPECT_TRUE_MSG(test, utf8_strncmp(um, &s1, &s2) == 0,
> +				    "%s %s comparison mismatch\n", s1.name, s2.name);
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(nfdicf_test_data); i++) {
> @@ -250,62 +236,65 @@ static void check_utf8_comparisons(struct unicode_map *table)
>  		const struct qstr s2 = {.name = nfdicf_test_data[i].ncf,
>  					.len = sizeof(nfdicf_test_data[i].ncf)};
>  
> -		test_f(!utf8_strncasecmp(table, &s1, &s2),
> -		       "%s %s comparison mismatch\n", s1.name, s2.name);
> +		/* strncasecmp returns 0 when strings are equal */
> +		KUNIT_EXPECT_TRUE_MSG(test, utf8_strncasecmp(um, &s1, &s2) == 0,
> +				    "%s %s comparison mismatch\n", s1.name, s2.name);
>  	}
>  }
>  
> -static void check_supported_versions(struct unicode_map *um)
> +static void check_supported_versions(struct kunit *test)
>  {
> +	struct unicode_map *um = test->priv;
>  	/* Unicode 7.0.0 should be supported. */
> -	test(utf8version_is_supported(um, UNICODE_AGE(7, 0, 0)));
> +	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UNICODE_AGE(7, 0, 0)));
>  
>  	/* Unicode 9.0.0 should be supported. */
> -	test(utf8version_is_supported(um, UNICODE_AGE(9, 0, 0)));
> +	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UNICODE_AGE(9, 0, 0)));
>  
>  	/* Unicode 1x.0.0 (the latest version) should be supported. */
> -	test(utf8version_is_supported(um, UTF8_LATEST));
> +	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(um, UTF8_LATEST));
>  
>  	/* Next versions don't exist. */
> -	test(!utf8version_is_supported(um, UNICODE_AGE(13, 0, 0)));
> -	test(!utf8version_is_supported(um, UNICODE_AGE(0, 0, 0)));
> -	test(!utf8version_is_supported(um, UNICODE_AGE(-1, -1, -1)));
> +	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(13, 0, 0)));
> +	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(0, 0, 0)));
> +	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(um, UNICODE_AGE(-1, -1, -1)));
>  }
>  
> -static int __init init_test_ucd(void)
> +static struct kunit_case unicode_normalization_test_cases[] = {
> +	KUNIT_CASE(check_supported_versions),
> +	KUNIT_CASE(check_utf8_comparisons),
> +	KUNIT_CASE(check_utf8_nfdicf),
> +	KUNIT_CASE(check_utf8_nfdi),
> +	{}
> +};
> +
> +static int init_test_ucd(struct kunit *test)
>  {
> -	struct unicode_map *um;
> +	struct unicode_map *um = utf8_load(UTF8_LATEST);
>  
> -	failed_tests = 0;
> -	total_tests = 0;
> +	test->priv = um;
>  
> -	um = utf8_load(UTF8_LATEST);
> -	if (IS_ERR(um)) {
> -		pr_err("%s: Unable to load utf8 table.\n", __func__);
> -		return PTR_ERR(um);
> -	}
> +	KUNIT_EXPECT_EQ_MSG(test, IS_ERR(um), 0,
> +			    "%s: Unable to load utf8 table.\n", __func__);
>  
> -	check_supported_versions(um);
> -	check_utf8_nfdi(um);
> -	check_utf8_nfdicf(um);
> -	check_utf8_comparisons(um);
> -
> -	if (!failed_tests)
> -		pr_info("All %u tests passed\n", total_tests);
> -	else
> -		pr_err("%u out of %u tests failed\n", failed_tests,
> -		       total_tests);
> -	utf8_unload(um);
>  	return 0;
>  }
>  
> -static void __exit exit_test_ucd(void)
> +static void exit_test_ucd(struct kunit *test)
>  {
> +	utf8_unload(test->priv);
>  }
>  
> -module_init(init_test_ucd);
> -module_exit(exit_test_ucd);
> +static struct kunit_suite unicode_normalization_test_suite = {
> +	.name = "unicode_normalization",
> +	.test_cases = unicode_normalization_test_cases,
> +	.init = init_test_ucd,
> +	.exit = exit_test_ucd,
> +};
> +
> +kunit_test_suite(unicode_normalization_test_suite);
> +
>  
>  MODULE_AUTHOR("Gabriel Krisman Bertazi <krisman@collabora.co.uk>");
> -MODULE_DESCRIPTION("Kernel module for testing utf-8 support");
> +MODULE_DESCRIPTION("KUnit tests for utf-8 support.");
>  MODULE_LICENSE("GPL");

-- 
Gabriel Krisman Bertazi

Re: [PATCH v2 2/2] unicode: kunit: change tests filename and path

[Thorsten Leemhuis]

On 04.10.24 21:00, Gabriel Krisman Bertazi wrote:
> Pedro Orlando <porlando@lkcamp.dev> writes:
>> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
>>
>> Change utf8 kunit test filename and path to follow the style
>> convention on Documentation/dev-tools/kunit/style.rst
>>
>> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
>> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
>> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
>> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
>> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
>> ---
>>  fs/unicode/Makefile                                | 2 +-
>>  fs/unicode/{ => tests}/.kunitconfig                | 0
>>  fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} | 0
>>  3 files changed, 1 insertion(+), 1 deletion(-)
>>  rename fs/unicode/{ => tests}/.kunitconfig (100%)
>>  rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (100%)
>>
>> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
>> index 37bbcbc628a1..d95be7fb9f6b 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_KUNIT_TEST) += utf8-selftest.o
>> +obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += tests/utf8_kunit.o
> 
> This breaks compilation for me.
> 
> fs/unicode/tests/utf8_kunit.c:11:10: fatal error: utf8n.h: No such file or directory
>    11 | #include "utf8n.h"
>       |          ^~~~~~~~~

I encountered the same error when building -next using the Fedora
rawhide config today. Given that this patch landed in -next today I
suspect it might be due to this change, but I'm on the road and unable
to verify that right now.

Log:
https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/08642966-next-next-all/builder-live.log.gz

Cioa, Thorsten

> After this patch that local header is now in the parent directory.
> 
> I'm building with:
> 
> CONFIG_UNICODE=m
> CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST=m
> 
>>  unicode-y := utf8-norm.o utf8-core.o
>>  
>> diff --git a/fs/unicode/.kunitconfig b/fs/unicode/tests/.kunitconfig
>> similarity index 100%
>> rename from fs/unicode/.kunitconfig
>> rename to fs/unicode/tests/.kunitconfig
>> diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/tests/utf8_kunit.c
>> similarity index 100%
>> rename from fs/unicode/utf8-selftest.c
>> rename to fs/unicode/tests/utf8_kunit.c
> 

Re: [PATCH v2 2/2] unicode: kunit: change tests filename and path

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

On Wed, 12 Feb 2025 at 14:45, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> On 04.10.24 21:00, Gabriel Krisman Bertazi wrote:
> > Pedro Orlando <porlando@lkcamp.dev> writes:
> >> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> >>
> >> Change utf8 kunit test filename and path to follow the style
> >> convention on Documentation/dev-tools/kunit/style.rst
> >>
> >> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
> >> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
> >> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
> >> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
> >> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> >> ---
> >>  fs/unicode/Makefile                                | 2 +-
> >>  fs/unicode/{ => tests}/.kunitconfig                | 0
> >>  fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} | 0
> >>  3 files changed, 1 insertion(+), 1 deletion(-)
> >>  rename fs/unicode/{ => tests}/.kunitconfig (100%)
> >>  rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (100%)
> >>
> >> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
> >> index 37bbcbc628a1..d95be7fb9f6b 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_KUNIT_TEST) += utf8-selftest.o
> >> +obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += tests/utf8_kunit.o
> >
> > This breaks compilation for me.
> >
> > fs/unicode/tests/utf8_kunit.c:11:10: fatal error: utf8n.h: No such file or directory
> >    11 | #include "utf8n.h"
> >       |          ^~~~~~~~~
>
> I encountered the same error when building -next using the Fedora
> rawhide config today. Given that this patch landed in -next today I
> suspect it might be due to this change, but I'm on the road and unable
> to verify that right now.
>
> Log:
> https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/08642966-next-next-all/builder-live.log.gz
>
> Cioa, Thorsten
>

Hmm... this definitely seems like a problem, but I haven't been able
to reproduce it here (either under x86_64 or UML, both as a module and
built-in). The suggested fix of changing the path to "../utf8n.h"
doesn't seem to have broken it, though.

Cheers,
-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]

Re: [PATCH v2 2/2] unicode: kunit: change tests filename and path

[Thorsten Leemhuis]

On 12.02.25 07:45, Thorsten Leemhuis wrote:
> On 04.10.24 21:00, Gabriel Krisman Bertazi wrote:
>> Pedro Orlando <porlando@lkcamp.dev> writes:
> [...]
>> This breaks compilation for me.
>>
>> fs/unicode/tests/utf8_kunit.c:11:10: fatal error: utf8n.h: No such file or directory
>>    11 | #include "utf8n.h"
>>       |          ^~~~~~~~~
> 
> I encountered the same error when building -next using the Fedora
> rawhide config today. Given that this patch landed in -next today I
> suspect it might be due to this change, but I'm on the road and unable
> to verify that right now.

Did that now and this patch was indeed the culprit, as reverting
be6f498e7391 ("unicode: kunit: change tests filename and path") from
-next fixed the build error for me.

Ciao, Thorsten

> Log:
> https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/08642966-next-next-all/builder-live.log.gz
>
>> After this patch that local header is now in the parent directory.
>>
>> I'm building with:
>>
>> CONFIG_UNICODE=m
>> CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST=m
>>
>>>  unicode-y := utf8-norm.o utf8-core.o
>>>  
>>> diff --git a/fs/unicode/.kunitconfig b/fs/unicode/tests/.kunitconfig
>>> similarity index 100%
>>> rename from fs/unicode/.kunitconfig
>>> rename to fs/unicode/tests/.kunitconfig
>>> diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/tests/utf8_kunit.c
>>> similarity index 100%
>>> rename from fs/unicode/utf8-selftest.c
>>> rename to fs/unicode/tests/utf8_kunit.c
>>
> 

Re: [PATCH v2 2/2] unicode: kunit: change tests filename and path

[Kees Cook]

On Wed, Feb 12, 2025 at 05:31:59PM +0800, David Gow wrote:
> On Wed, 12 Feb 2025 at 14:45, Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >
> > On 04.10.24 21:00, Gabriel Krisman Bertazi wrote:
> > > Pedro Orlando <porlando@lkcamp.dev> writes:
> > >> From: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> > >>
> > >> Change utf8 kunit test filename and path to follow the style
> > >> convention on Documentation/dev-tools/kunit/style.rst
> > >>
> > >> Co-developed-by: Pedro Orlando <porlando@lkcamp.dev>
> > >> Signed-off-by: Pedro Orlando <porlando@lkcamp.dev>
> > >> Co-developed-by: Danilo Pereira <dpereira@lkcamp.dev>
> > >> Signed-off-by: Danilo Pereira <dpereira@lkcamp.dev>
> > >> Signed-off-by: Gabriela Bittencourt <gbittencourt@lkcamp.dev>
> > >> ---
> > >>  fs/unicode/Makefile                                | 2 +-
> > >>  fs/unicode/{ => tests}/.kunitconfig                | 0
> > >>  fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} | 0
> > >>  3 files changed, 1 insertion(+), 1 deletion(-)
> > >>  rename fs/unicode/{ => tests}/.kunitconfig (100%)
> > >>  rename fs/unicode/{utf8-selftest.c => tests/utf8_kunit.c} (100%)
> > >>
> > >> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
> > >> index 37bbcbc628a1..d95be7fb9f6b 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_KUNIT_TEST) += utf8-selftest.o
> > >> +obj-$(CONFIG_UNICODE_NORMALIZATION_KUNIT_TEST) += tests/utf8_kunit.o
> > >
> > > This breaks compilation for me.
> > >
> > > fs/unicode/tests/utf8_kunit.c:11:10: fatal error: utf8n.h: No such file or directory
> > >    11 | #include "utf8n.h"
> > >       |          ^~~~~~~~~
> >
> > I encountered the same error when building -next using the Fedora
> > rawhide config today. Given that this patch landed in -next today I
> > suspect it might be due to this change, but I'm on the road and unable
> > to verify that right now.
> >
> > Log:
> > https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/08642966-next-next-all/builder-live.log.gz
> >
> > Cioa, Thorsten
> >
> 
> Hmm... this definitely seems like a problem, but I haven't been able
> to reproduce it here (either under x86_64 or UML, both as a module and
> built-in). The suggested fix of changing the path to "../utf8n.h"
> doesn't seem to have broken it, though.

Thanks for the reports! I've squashed this path correction into my tree
and it should be fix in the next -next. :)

-Kees

-- 
Kees Cook