diff mbox series

Implement utf8 unit tests as a kunit test suite.

Message ID 20200415082826.19325-1-ricardo.canuelo@collabora.com (mailing list archive)
State New, archived
Headers show
Series Implement utf8 unit tests as a kunit test suite. | expand

Commit Message

Ricardo Cañuelo April 15, 2020, 8:28 a.m. UTC 
This translates the existing utf8 unit test module into a kunit-compliant
test suite. No functionality has been added or removed.

Some names changed to make the file name, the Kconfig option and test
suite name less specific, since this source file might hold more utf8
tests in the future.

Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
---
Tested with kunit_tool and at boot time on qemu-system-x86_64

 fs/unicode/Kconfig                          |  18 +-
 fs/unicode/Makefile                         |   2 +-
 fs/unicode/{utf8-selftest.c => utf8-test.c} | 207 ++++++++++----------
 3 files changed, 115 insertions(+), 112 deletions(-)
 rename fs/unicode/{utf8-selftest.c => utf8-test.c} (59%)

Comments

Randy Dunlap April 15, 2020, 4:19 p.m. UTC | #1 
Hi,

On 4/15/20 1:28 AM, Ricardo Cañuelo wrote:
> diff --git a/fs/unicode/Kconfig b/fs/unicode/Kconfig
> index 2c27b9a5cd6c..734c25920750 100644
> --- a/fs/unicode/Kconfig
> +++ b/fs/unicode/Kconfig
> @@ -8,7 +8,19 @@ config UNICODE
>  	  Say Y here to enable UTF-8 NFD normalization and NFD+CF casefolding
>  	  support.
>  
> -config UNICODE_NORMALIZATION_SELFTEST
> -	tristate "Test UTF-8 normalization support"
> -	depends on UNICODE
> +config UNICODE_KUNIT_TESTS
> +	bool "Kunit tests for UTF-8 support"
> +	depends on UNICODE && KUNIT
>  	default n
> +	help
> +	  This builds the ext4 KUnit tests.

			ext4??

> +
> +	  KUnit tests run during boot and output the results to the debug log
> +	  in TAP format (http://testanything.org/). Only useful for kernel devs
> +	  running KUnit test harness and are not for inclusion into a production
> +	  build.
> +
> +	  For more information on KUnit and unit tests in general please refer
> +	  to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> +	  If unsure, say N.


thanks.
Gabriel Krisman Bertazi April 15, 2020, 6:40 p.m. UTC | #2 
Ricardo Cañuelo <ricardo.canuelo@collabora.com> writes:

> This translates the existing utf8 unit test module into a kunit-compliant
> test suite. No functionality has been added or removed.
>
> Some names changed to make the file name, the Kconfig option and test
> suite name less specific, since this source file might hold more utf8
> tests in the future.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
> Tested with kunit_tool and at boot time on qemu-system-x86_64
>
>  fs/unicode/Kconfig                          |  18 +-
>  fs/unicode/Makefile                         |   2 +-
>  fs/unicode/{utf8-selftest.c => utf8-test.c} | 207 ++++++++++----------
>  3 files changed, 115 insertions(+), 112 deletions(-)
>  rename fs/unicode/{utf8-selftest.c => utf8-test.c} (59%)
>
> diff --git a/fs/unicode/Kconfig b/fs/unicode/Kconfig
> index 2c27b9a5cd6c..734c25920750 100644
> --- a/fs/unicode/Kconfig
> +++ b/fs/unicode/Kconfig
> @@ -8,7 +8,19 @@ config UNICODE
>  	  Say Y here to enable UTF-8 NFD normalization and NFD+CF casefolding
>  	  support.
>  
> -config UNICODE_NORMALIZATION_SELFTEST
> -	tristate "Test UTF-8 normalization support"
> -	depends on UNICODE
> +config UNICODE_KUNIT_TESTS
> +	bool "Kunit tests for UTF-8 support"

Kunit tests for Unicode normalization and casefolding support

> +	depends on UNICODE && KUNIT
>  	default n
> +	help
> +	  This builds the ext4 KUnit tests.

Unicode KUinit tests.

> +
> +	  KUnit tests run during boot and output the results to the debug log
> +	  in TAP format (http://testanything.org/). Only useful for kernel devs
> +	  running KUnit test harness and are not for inclusion into a production
> +	  build.
> +
> +	  For more information on KUnit and unit tests in general please refer
> +	  to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> +	  If unsure, say N.
> diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
> index b88aecc86550..0e8e2192a715 100644
> --- a/fs/unicode/Makefile
> +++ b/fs/unicode/Makefile
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  obj-$(CONFIG_UNICODE) += unicode.o
> -obj-$(CONFIG_UNICODE_NORMALIZATION_SELFTEST) += utf8-selftest.o
> +obj-$(CONFIG_UNICODE_KUNIT_TESTS) += utf8-test.o
>  
>  unicode-y := utf8-norm.o utf8-core.o
>  
> diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/utf8-test.c
> similarity index 59%
> rename from fs/unicode/utf8-selftest.c
> rename to fs/unicode/utf8-test.c
> index 6fe8af7edccb..20d12b1efc42 100644
> --- a/fs/unicode/utf8-selftest.c
> +++ b/fs/unicode/utf8-test.c
> @@ -1,39 +1,25 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Kernel module for testing utf-8 support.
> + * Kunit tests for utf-8 support.
>   *
> - * Copyright 2017 Collabora Ltd.
> + * Copyright 2020 Collabora Ltd.
>   */
>  
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/module.h>
> -#include <linux/printk.h>
> +#include <kunit/test.h>
>  #include <linux/unicode.h>
> -#include <linux/dcache.h>
> -
>  #include "utf8n.h"
>  
> -unsigned int failed_tests;
> -unsigned int total_tests;
> +#define VERSION_STR_LEN 16

Instead of this random len and the snprintf to generate the string at
runtime, why not just:

#define LATEST_VERSION_STR "12.1.0"

And use it directly, since it is constant.

>  /* Tests will be based on this version. */
>  #define latest_maj 12
>  #define latest_min 1
>  #define latest_rev 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__, "")
> +
> +/************************************************************
> + * Test data                                                *
> + ************************************************************/

Please, keep the comment style used in the rest of the file.

>  
>  static const struct {
>  	/* UTF-8 strings in this vector _must_ be NULL-terminated. */
> @@ -86,9 +72,9 @@ static const struct {
>  
>  		.dec = {0x61, 0xCC, 0xA8, 0xcc, 0x88, 0x00},
>  	},
> -
>  };
>  
> +

Some noise here

Thanks,
Ricardo Cañuelo April 16, 2020, 7:51 a.m. UTC | #3 
Hi Gabriel,

Thanks for the comments. I agree with all the style and typo fixes (sorry about
that stray "ext4" mention, I copied the help message from the ext4 kunit config
option and I didn't notice that).

On mié 15-04-2020 14:40:23, Gabriel Krisman Bertazi wrote:
> Instead of this random len and the snprintf to generate the string at
> runtime, why not just:
> 
> #define LATEST_VERSION_STR "12.1.0"
> 
> And use it directly, since it is constant.

I don't think it's a good idea to have the version specifier hardcoded twice in
the same file, one in string form (for utf8_load) and another one in integer
form (for the rest of the functions that take the version as a parameter). I
think it'd be a better option to use a macro to stringify the version number
from the integer constants and avoid the snprintf entirely:

#define str(s) #s
#define VERSION_STR(maj, min, rev) str(maj) "." str(min) "." str(rev)

...

table = utf8_load(VERSION_STR(latest_maj, latest_min, latest_rev));


This way we can define the version constant only once, in integer form, and
then the string form will be a constant generated at compile time. Are you ok
with this?

Cheers,
Ricardo
Gabriel Krisman Bertazi April 16, 2020, 2:17 p.m. UTC | #4 
Ricardo Cañuelo <ricardo.canuelo@collabora.com> writes:


> I don't think it's a good idea to have the version specifier hardcoded twice in
> the same file, one in string form (for utf8_load) and another one in integer
> form (for the rest of the functions that take the version as a parameter). I
> think it'd be a better option to use a macro to stringify the version number
> from the integer constants and avoid the snprintf entirely:
>
> #define str(s) #s
> #define VERSION_STR(maj, min, rev) str(maj) "." str(min) "." str(rev)
>
> ...
>
> table = utf8_load(VERSION_STR(latest_maj, latest_min, latest_rev));
>
>
> This way we can define the version constant only once, in integer form, and
> then the string form will be a constant generated at compile time. Are you ok
> with this?

fine with me.

Thanks,
diff mbox series

Patch

diff --git a/fs/unicode/Kconfig b/fs/unicode/Kconfig
index 2c27b9a5cd6c..734c25920750 100644
--- a/fs/unicode/Kconfig
+++ b/fs/unicode/Kconfig
@@ -8,7 +8,19 @@  config UNICODE
 	  Say Y here to enable UTF-8 NFD normalization and NFD+CF casefolding
 	  support.
 
-config UNICODE_NORMALIZATION_SELFTEST
-	tristate "Test UTF-8 normalization support"
-	depends on UNICODE
+config UNICODE_KUNIT_TESTS
+	bool "Kunit tests for UTF-8 support"
+	depends on UNICODE && KUNIT
 	default n
+	help
+	  This builds the ext4 KUnit tests.
+
+	  KUnit tests run during boot and output the results to the debug log
+	  in TAP format (http://testanything.org/). Only useful for kernel devs
+	  running KUnit test harness and are not for inclusion into a production
+	  build.
+
+	  For more information on KUnit and unit tests in general please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
diff --git a/fs/unicode/Makefile b/fs/unicode/Makefile
index b88aecc86550..0e8e2192a715 100644
--- a/fs/unicode/Makefile
+++ b/fs/unicode/Makefile
@@ -1,7 +1,7 @@ 
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_UNICODE) += unicode.o
-obj-$(CONFIG_UNICODE_NORMALIZATION_SELFTEST) += utf8-selftest.o
+obj-$(CONFIG_UNICODE_KUNIT_TESTS) += utf8-test.o
 
 unicode-y := utf8-norm.o utf8-core.o
 
diff --git a/fs/unicode/utf8-selftest.c b/fs/unicode/utf8-test.c
similarity index 59%
rename from fs/unicode/utf8-selftest.c
rename to fs/unicode/utf8-test.c
index 6fe8af7edccb..20d12b1efc42 100644
--- a/fs/unicode/utf8-selftest.c
+++ b/fs/unicode/utf8-test.c
@@ -1,39 +1,25 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Kernel module for testing utf-8 support.
+ * Kunit tests for utf-8 support.
  *
- * Copyright 2017 Collabora Ltd.
+ * Copyright 2020 Collabora Ltd.
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/module.h>
-#include <linux/printk.h>
+#include <kunit/test.h>
 #include <linux/unicode.h>
-#include <linux/dcache.h>
-
 #include "utf8n.h"
 
-unsigned int failed_tests;
-unsigned int total_tests;
+#define VERSION_STR_LEN 16
 
 /* Tests will be based on this version. */
 #define latest_maj 12
 #define latest_min 1
 #define latest_rev 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__, "")
+
+/************************************************************
+ * Test data                                                *
+ ************************************************************/
 
 static const struct {
 	/* UTF-8 strings in this vector _must_ be NULL-terminated. */
@@ -86,9 +72,9 @@  static const struct {
 
 		.dec = {0x61, 0xCC, 0xA8, 0xcc, 0x88, 0x00},
 	},
-
 };
 
+
 static const struct {
 	/* UTF-8 strings in this vector _must_ be NULL-terminated. */
 	unsigned char str[30];
@@ -160,88 +146,123 @@  static const struct {
 	}
 };
 
-static void check_utf8_nfdi(void)
+
+/************************************************************
+ * Test cases                                               *
+ ************************************************************/
+
+static void utf8_test_supported_versions(struct kunit *test)
+{
+	/* Unicode 7.0.0 should be supported. */
+	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(7, 0, 0));
+
+	/* Unicode 9.0.0 should be supported. */
+	KUNIT_EXPECT_TRUE(test, utf8version_is_supported(9, 0, 0));
+
+	/* Unicode 1x.0.0 (the latest version) should be supported. */
+	KUNIT_EXPECT_TRUE(test,
+		utf8version_is_supported(latest_maj, latest_min, latest_rev));
+
+	/* Next versions don't exist. */
+	KUNIT_EXPECT_FALSE(test,
+		utf8version_is_supported(latest_maj + 1 , 0, 0));
+
+	/* Test for invalid version values */
+	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(0, 0, 0));
+	KUNIT_EXPECT_FALSE(test, utf8version_is_supported(-1, -1, -1));
+}
+
+static void utf8_test_nfdi(struct kunit *test)
 {
 	int i;
 	struct utf8cursor u8c;
 	const struct utf8data *data;
 
 	data = utf8nfdi(UNICODE_AGE(latest_maj, latest_min, latest_rev));
-	if (!data) {
-		pr_err("%s: Unable to load utf8-%d.%d.%d. Skipping.\n",
-		       __func__, latest_maj, latest_min, latest_rev);
-		return;
-	}
+	KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, data,
+		"Unable to load utf8-%d.%d.%d. Skipping.",
+		latest_maj, latest_min, latest_rev);
 
 	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);
+		size_t len = strlen(nfdi_test_data[i].str);
+		size_t nlen = strlen(nfdi_test_data[i].dec);
 		int j = 0;
 		unsigned char c;
 
-		test((utf8len(data, nfdi_test_data[i].str) == nlen));
-		test((utf8nlen(data, nfdi_test_data[i].str, len) == nlen));
+		KUNIT_EXPECT_EQ(test,
+			utf8len(data, nfdi_test_data[i].str),
+			(ssize_t)nlen);
+		KUNIT_EXPECT_EQ(test,
+			utf8nlen(data, nfdi_test_data[i].str, len),
+			(ssize_t)nlen);
 
-		if (utf8cursor(&u8c, data, nfdi_test_data[i].str) < 0)
-			pr_err("can't create cursor\n");
+		KUNIT_ASSERT_EQ_MSG(test,
+			utf8cursor(&u8c, data, nfdi_test_data[i].str), 0,
+			"Can't create cursor");
 
 		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",
+				c, nfdi_test_data[i].dec[j]);
 			j++;
 		}
 
-		test((j == nlen));
+		KUNIT_EXPECT_EQ(test, j, (int)nlen);
 	}
 }
 
-static void check_utf8_nfdicf(void)
+static void utf8_test_nfdicf(struct kunit *test)
 {
 	int i;
 	struct utf8cursor u8c;
 	const struct utf8data *data;
 
 	data = utf8nfdicf(UNICODE_AGE(latest_maj, latest_min, latest_rev));
-	if (!data) {
-		pr_err("%s: Unable to load utf8-%d.%d.%d. Skipping.\n",
-		       __func__, latest_maj, latest_min, latest_rev);
-		return;
-	}
+	KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, data,
+		"Unable to load utf8-%d.%d.%d. Skipping.",
+		latest_maj, latest_min, latest_rev);
 
 	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);
+		size_t len = strlen(nfdicf_test_data[i].str);
+		size_t nlen = strlen(nfdicf_test_data[i].ncf);
 		int j = 0;
 		unsigned char c;
 
-		test((utf8len(data, nfdicf_test_data[i].str) == nlen));
-		test((utf8nlen(data, nfdicf_test_data[i].str, len) == nlen));
+		KUNIT_EXPECT_EQ(test,
+			utf8len(data, nfdicf_test_data[i].str),
+			(ssize_t)nlen);
+		KUNIT_EXPECT_EQ(test,
+			utf8nlen(data, nfdicf_test_data[i].str, len),
+			(ssize_t)nlen);
 
-		if (utf8cursor(&u8c, data, nfdicf_test_data[i].str) < 0)
-			pr_err("can't create cursor\n");
+		KUNIT_ASSERT_EQ_MSG(test,
+			utf8cursor(&u8c, data, nfdicf_test_data[i].str), 0,
+			"Can't create cursor");
 
 		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, (int)nlen);
 	}
 }
 
-static void check_utf8_comparisons(void)
+static void utf8_test_comparisons(struct kunit *test)
 {
 	int i;
-	struct unicode_map *table = utf8_load("12.1.0");
+	char version[VERSION_STR_LEN] = {0};
+	struct unicode_map *table;
 
-	if (IS_ERR(table)) {
-		pr_err("%s: Unable to load utf8 %d.%d.%d. Skipping.\n",
-		       __func__, latest_maj, latest_min, latest_rev);
-		return;
-	}
+	snprintf(version, VERSION_STR_LEN, "%d.%d.%d", latest_maj,
+		latest_min, latest_rev);
+
+	table = utf8_load(version);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, table,
+		"Unable to load utf8-%d.%d.%d. Skipping.\n",
+		latest_maj, latest_min, latest_rev);
 
 	for (i = 0; i < ARRAY_SIZE(nfdi_test_data); i++) {
 		const struct qstr s1 = {.name = nfdi_test_data[i].str,
@@ -249,8 +270,8 @@  static void check_utf8_comparisons(void)
 		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);
+		KUNIT_EXPECT_EQ_MSG(test, utf8_strncmp(table, &s1, &s2), 0,
+			"%s %s comparison mismatch", s1.name, s2.name);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(nfdicf_test_data); i++) {
@@ -259,54 +280,24 @@  static void check_utf8_comparisons(void)
 		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);
+		KUNIT_EXPECT_EQ_MSG(test, utf8_strncasecmp(table, &s1, &s2), 0,
+			"%s %s comparison mismatch", s1.name, s2.name);
 	}
 
 	utf8_unload(table);
 }
 
-static void check_supported_versions(void)
-{
-	/* Unicode 7.0.0 should be supported. */
-	test(utf8version_is_supported(7, 0, 0));
-
-	/* Unicode 9.0.0 should be supported. */
-	test(utf8version_is_supported(9, 0, 0));
-
-	/* Unicode 1x.0.0 (the latest version) should be supported. */
-	test(utf8version_is_supported(latest_maj, latest_min, latest_rev));
-
-	/* Next versions don't exist. */
-	test(!utf8version_is_supported(13, 0, 0));
-	test(!utf8version_is_supported(0, 0, 0));
-	test(!utf8version_is_supported(-1, -1, -1));
-}
-
-static int __init init_test_ucd(void)
-{
-	failed_tests = 0;
-	total_tests = 0;
-
-	check_supported_versions();
-	check_utf8_nfdi();
-	check_utf8_nfdicf();
-	check_utf8_comparisons();
-
-	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);
-	return 0;
-}
-
-static void __exit exit_test_ucd(void)
-{
-}
+static struct kunit_case utf8_test_cases[] = {
+	KUNIT_CASE(utf8_test_supported_versions),
+	KUNIT_CASE(utf8_test_nfdi),
+	KUNIT_CASE(utf8_test_nfdicf),
+	KUNIT_CASE(utf8_test_comparisons),
+	{}
+};
 
-module_init(init_test_ucd);
-module_exit(exit_test_ucd);
+static struct kunit_suite utf8_test_suite = {
+	.name = "utf8-unit-test",
+	.test_cases = utf8_test_cases,
+};
 
-MODULE_AUTHOR("Gabriel Krisman Bertazi <krisman@collabora.co.uk>");
-MODULE_LICENSE("GPL");
+kunit_test_suite(utf8_test_suite);