linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/1] lib: Convert UUID runtime test to KUnit
@ 2021-06-21 13:31 André Almeida
  2021-06-21 13:31 ` [PATCH v4 1/1] " André Almeida
  0 siblings, 1 reply; 4+ messages in thread
From: André Almeida @ 2021-06-21 13:31 UTC (permalink / raw)
  To: Christoph Hellwig, Andy Shevchenko, linux-kernel
  Cc: Brendan Higgins, linux-kselftest, kunit-dev, Shuah Khan,
	~lkcamp/patches, nfraprado, leandro.ribeiro, Vitor Massaru Iha,
	lucmaga, David Gow, Daniel Latypov, tales.aparecida,
	André Almeida

Hi,

This patch converts existing UUID runtime test to use KUnit framework.

Below, there's a comparison between the old output format and the new
one. Keep in mind that even if KUnit seems very verbose, this is the
corner case where _every_ test has failed.

* This is how the current output looks like in success:

  test_uuid: all 18 tests passed

* And when it fails:

  test_uuid: conversion test #1 failed on LE data: 'c33f4995-3701-450e-9fbf-206a2e98e576'
  test_uuid: cmp test #2 failed on LE data: 'c33f4995-3701-450e-9fbf-206a2e98e576'
  test_uuid: cmp test #2 actual data: 'c33f4995-3701-450e-9fbf-206a2e98e576'
  test_uuid: conversion test #3 failed on BE data: 'c33f4995-3701-450e-9fbf-206a2e98e576'
  test_uuid: cmp test #4 failed on BE data: 'c33f4995-3701-450e-9fbf-206a2e98e576'
  test_uuid: cmp test #4 actual data: 'c33f4995-3701-450e-9fbf-206a2e98e576'
  test_uuid: conversion test #5 failed on LE data: '64b4371c-77c1-48f9-8221-29f054fc023b'
  test_uuid: cmp test #6 failed on LE data: '64b4371c-77c1-48f9-8221-29f054fc023b'
  test_uuid: cmp test #6 actual data: '64b4371c-77c1-48f9-8221-29f054fc023b'
  test_uuid: conversion test #7 failed on BE data: '64b4371c-77c1-48f9-8221-29f054fc023b'
  test_uuid: cmp test #8 failed on BE data: '64b4371c-77c1-48f9-8221-29f054fc023b'
  test_uuid: cmp test #8 actual data: '64b4371c-77c1-48f9-8221-29f054fc023b'
  test_uuid: conversion test #9 failed on LE data: '0cb4ddff-a545-4401-9d06-688af53e7f84'
  test_uuid: cmp test #10 failed on LE data: '0cb4ddff-a545-4401-9d06-688af53e7f84'
  test_uuid: cmp test #10 actual data: '0cb4ddff-a545-4401-9d06-688af53e7f84'
  test_uuid: conversion test #11 failed on BE data: '0cb4ddff-a545-4401-9d06-688af53e7f84'
  test_uuid: cmp test #12 failed on BE data: '0cb4ddff-a545-4401-9d06-688af53e7f84'
  test_uuid: cmp test #12 actual data: '0cb4ddff-a545-4401-9d06-688af53e7f84'
  test_uuid: negative test #13 passed on wrong LE data: 'c33f4995-3701-450e-9fbf206a2e98e576 '
  test_uuid: negative test #14 passed on wrong BE data: 'c33f4995-3701-450e-9fbf206a2e98e576 '
  test_uuid: negative test #15 passed on wrong LE data: '64b4371c-77c1-48f9-8221-29f054XX023b'
  test_uuid: negative test #16 passed on wrong BE data: '64b4371c-77c1-48f9-8221-29f054XX023b'
  test_uuid: negative test #17 passed on wrong LE data: '0cb4ddff-a545-4401-9d06-688af53e'
  test_uuid: negative test #18 passed on wrong BE data: '0cb4ddff-a545-4401-9d06-688af53e'
  test_uuid: failed 18 out of 18 tests


* Now, here's how it looks like with KUnit:

  ======== [PASSED] uuid ========
  [PASSED] uuid_correct_be
  [PASSED] uuid_correct_le
  [PASSED] uuid_wrong_be
  [PASSED] uuid_wrong_le

* And if every test fail with KUnit:

  ======== [FAILED] uuid ========
  [FAILED] uuid_correct_be
      # uuid_correct_be: ASSERTION FAILED at lib/test_uuid.c:57
      Expected uuid_parse(data->uuid, &be) == 1, but
          uuid_parse(data->uuid, &be) == 0
  
  failed to parse 'c33f4995-3701-450e-9fbf-206a2e98e576'
      # uuid_correct_be: not ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
      # uuid_correct_be: ASSERTION FAILED at lib/test_uuid.c:57
      Expected uuid_parse(data->uuid, &be) == 1, but
          uuid_parse(data->uuid, &be) == 0
  
  failed to parse '64b4371c-77c1-48f9-8221-29f054fc023b'
      # uuid_correct_be: not ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
      # uuid_correct_be: ASSERTION FAILED at lib/test_uuid.c:57
      Expected uuid_parse(data->uuid, &be) == 1, but
          uuid_parse(data->uuid, &be) == 0
  
  failed to parse '0cb4ddff-a545-4401-9d06-688af53e7f84'
      # uuid_correct_be: not ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
      not ok 1 - uuid_correct_be
  
  [FAILED] uuid_correct_le
      # uuid_correct_le: ASSERTION FAILED at lib/test_uuid.c:46
      Expected guid_parse(data->uuid, &le) == 1, but
          guid_parse(data->uuid, &le) == 0
  
  failed to parse 'c33f4995-3701-450e-9fbf-206a2e98e576'
      # uuid_correct_le: not ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
      # uuid_correct_le: ASSERTION FAILED at lib/test_uuid.c:46
      Expected guid_parse(data->uuid, &le) == 1, but
          guid_parse(data->uuid, &le) == 0
  
  failed to parse '64b4371c-77c1-48f9-8221-29f054fc023b'
      # uuid_correct_le: not ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
      # uuid_correct_le: ASSERTION FAILED at lib/test_uuid.c:46
      Expected guid_parse(data->uuid, &le) == 1, but
          guid_parse(data->uuid, &le) == 0
  
  failed to parse '0cb4ddff-a545-4401-9d06-688af53e7f84'
      # uuid_correct_le: not ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
      not ok 2 - uuid_correct_le
  
  [FAILED] uuid_wrong_be
      # uuid_wrong_be: ASSERTION FAILED at lib/test_uuid.c:77
      Expected uuid_parse(*data, &be) == 0, but
          uuid_parse(*data, &be) == -22
  
  parsing of 'c33f4995-3701-450e-9fbf206a2e98e576 ' should've failed
      # uuid_wrong_be: not ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
      # uuid_wrong_be: ASSERTION FAILED at lib/test_uuid.c:77
      Expected uuid_parse(*data, &be) == 0, but
          uuid_parse(*data, &be) == -22
  
  parsing of '64b4371c-77c1-48f9-8221-29f054XX023b' should've failed
      # uuid_wrong_be: not ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
      # uuid_wrong_be: ASSERTION FAILED at lib/test_uuid.c:77
      Expected uuid_parse(*data, &be) == 0, but
          uuid_parse(*data, &be) == -22
  
  parsing of '0cb4ddff-a545-4401-9d06-688af53e' should've failed
      # uuid_wrong_be: not ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
      not ok 3 - uuid_wrong_be
  
  [FAILED] uuid_wrong_le
      # uuid_wrong_le: ASSERTION FAILED at lib/test_uuid.c:68
      Expected guid_parse(*data, &le) == 0, but
          guid_parse(*data, &le) == -22
  
  parsing of 'c33f4995-3701-450e-9fbf206a2e98e576 ' should've failed
      # uuid_wrong_le: not ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
      # uuid_wrong_le: ASSERTION FAILED at lib/test_uuid.c:68
      Expected guid_parse(*data, &le) == 0, but
          guid_parse(*data, &le) == -22
  
  parsing of '64b4371c-77c1-48f9-8221-29f054XX023b' should've failed
      # uuid_wrong_le: not ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
      # uuid_wrong_le: ASSERTION FAILED at lib/test_uuid.c:68
      Expected guid_parse(*data, &le) == 0, but
          guid_parse(*data, &le) == -22
  
  parsing of '0cb4ddff-a545-4401-9d06-688af53e' should've failed
      # uuid_wrong_le: not ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
      not ok 4 - uuid_wrong_le

Changes from v3:
 - Drop unnecessary casts and braces.
 - Simplify Kconfig entry
v3: https://lore.kernel.org/lkml/20210610163959.71634-1-andrealmeid@collabora.com/

Changes from v2:
 - Clarify in commit message the new test cases setup
v2: https://lore.kernel.org/lkml/20210609233730.164082-1-andrealmeid@collabora.com/

Changes from v1:
 - Test suite name: uuid_test -> uuid
 - Config name: TEST_UUID -> UUID_KUNIT_TEST
 - Config entry in the Kconfig file left where it is
 - Converted tests to use _MSG variant
v1: https://lore.kernel.org/lkml/20210605215215.171165-1-andrealmeid@collabora.com/

André Almeida (1):
  lib: Convert UUID runtime test to KUnit

 lib/Kconfig.debug |   8 ++-
 lib/Makefile      |   2 +-
 lib/test_uuid.c   | 137 +++++++++++++++++++---------------------------
 3 files changed, 64 insertions(+), 83 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 1/1] lib: Convert UUID runtime test to KUnit
  2021-06-21 13:31 [PATCH v4 0/1] lib: Convert UUID runtime test to KUnit André Almeida
@ 2021-06-21 13:31 ` André Almeida
  2021-06-21 21:29   ` Daniel Latypov
  0 siblings, 1 reply; 4+ messages in thread
From: André Almeida @ 2021-06-21 13:31 UTC (permalink / raw)
  To: Christoph Hellwig, Andy Shevchenko, linux-kernel
  Cc: Brendan Higgins, linux-kselftest, kunit-dev, Shuah Khan,
	~lkcamp/patches, nfraprado, leandro.ribeiro, Vitor Massaru Iha,
	lucmaga, David Gow, Daniel Latypov, tales.aparecida,
	André Almeida

Remove custom functions for testing and use KUnit framework. Keep the
tested functions and test data the same.

Current test threat (g/u)uid_parse and (g/u)uid_equal as different test
cases. Make both functions being part of the same test case, given the
dependency regarding their results. This reduces the tests cases from 6
cases to 4, while keeping the test coverage the same. Given that we have
3 strings for each test case, current test output notifies 18 tests
results, and the KUnit output announces 12 results.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 lib/Kconfig.debug |   8 ++-
 lib/Makefile      |   2 +-
 lib/test_uuid.c   | 137 +++++++++++++++++++---------------------------
 3 files changed, 64 insertions(+), 83 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 678c13967580..606ec5e2586d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2188,8 +2188,12 @@ config TEST_BITMAP
 
 	  If unsure, say N.
 
-config TEST_UUID
-	tristate "Test functions located in the uuid module at runtime"
+config UUID_KUNIT_TEST
+	tristate "Unit test for UUID" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Tests parsing functions for UUID/GUID strings.
 
 config TEST_XARRAY
 	tristate "Test the XArray code at runtime"
diff --git a/lib/Makefile b/lib/Makefile
index 2cc359ec1fdd..cc19048961c0 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -85,7 +85,6 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
 obj-$(CONFIG_TEST_PRINTF) += test_printf.o
 obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
 obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
-obj-$(CONFIG_TEST_UUID) += test_uuid.o
 obj-$(CONFIG_TEST_XARRAY) += test_xarray.o
 obj-$(CONFIG_TEST_PARMAN) += test_parman.o
 obj-$(CONFIG_TEST_KMOD) += test_kmod.o
@@ -354,5 +353,6 @@ obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
 obj-$(CONFIG_BITS_TEST) += test_bits.o
 obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
+obj-$(CONFIG_UUID_KUNIT_TEST) += test_uuid.o
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
diff --git a/lib/test_uuid.c b/lib/test_uuid.c
index cd819c397dc7..30f350301e6d 100644
--- a/lib/test_uuid.c
+++ b/lib/test_uuid.c
@@ -1,21 +1,20 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
 /*
- * Test cases for lib/uuid.c module.
+ * Unit tests for lib/uuid.c module.
+ *
+ * Copyright 2016 Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ * Copyright 2021 André Almeida <andrealmeid@riseup.net>
  */
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/init.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/string.h>
+#include <kunit/test.h>
 #include <linux/uuid.h>
 
-struct test_uuid_data {
+struct test_data {
 	const char *uuid;
 	guid_t le;
 	uuid_t be;
 };
 
-static const struct test_uuid_data test_uuid_test_data[] = {
+static const struct test_data correct_data[] = {
 	{
 		.uuid = "c33f4995-3701-450e-9fbf-206a2e98e576",
 		.le = GUID_INIT(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
@@ -33,101 +32,79 @@ static const struct test_uuid_data test_uuid_test_data[] = {
 	},
 };
 
-static const char * const test_uuid_wrong_data[] = {
+static const char * const wrong_data[] = {
 	"c33f4995-3701-450e-9fbf206a2e98e576 ",	/* no hyphen(s) */
 	"64b4371c-77c1-48f9-8221-29f054XX023b",	/* invalid character(s) */
 	"0cb4ddff-a545-4401-9d06-688af53e",	/* not enough data */
 };
 
-static unsigned total_tests __initdata;
-static unsigned failed_tests __initdata;
-
-static void __init test_uuid_failed(const char *prefix, bool wrong, bool be,
-				    const char *data, const char *actual)
+static void uuid_correct_le(struct kunit *test)
 {
-	pr_err("%s test #%u %s %s data: '%s'\n",
-	       prefix,
-	       total_tests,
-	       wrong ? "passed on wrong" : "failed on",
-	       be ? "BE" : "LE",
-	       data);
-	if (actual && *actual)
-		pr_err("%s test #%u actual data: '%s'\n",
-		       prefix,
-		       total_tests,
-		       actual);
-	failed_tests++;
+	guid_t le;
+	const struct test_data *data = test->param_value;
+
+	KUNIT_ASSERT_EQ_MSG(test, guid_parse(data->uuid, &le), 0,
+			    "failed to parse '%s'", data->uuid);
+	KUNIT_EXPECT_TRUE_MSG(test, guid_equal(&data->le, &le),
+			      "'%s' should be equal to %pUl", data->uuid, &le);
 }
 
-static void __init test_uuid_test(const struct test_uuid_data *data)
+static void uuid_correct_be(struct kunit *test)
 {
-	guid_t le;
 	uuid_t be;
-	char buf[48];
-
-	/* LE */
-	total_tests++;
-	if (guid_parse(data->uuid, &le))
-		test_uuid_failed("conversion", false, false, data->uuid, NULL);
-
-	total_tests++;
-	if (!guid_equal(&data->le, &le)) {
-		sprintf(buf, "%pUl", &le);
-		test_uuid_failed("cmp", false, false, data->uuid, buf);
-	}
-
-	/* BE */
-	total_tests++;
-	if (uuid_parse(data->uuid, &be))
-		test_uuid_failed("conversion", false, true, data->uuid, NULL);
-
-	total_tests++;
-	if (!uuid_equal(&data->be, &be)) {
-		sprintf(buf, "%pUb", &be);
-		test_uuid_failed("cmp", false, true, data->uuid, buf);
-	}
+	const struct test_data *data = test->param_value;
+
+	KUNIT_ASSERT_EQ_MSG(test, uuid_parse(data->uuid, &be), 0,
+			    "failed to parse '%s'", data->uuid);
+	KUNIT_EXPECT_TRUE_MSG(test, uuid_equal(&data->be, &be),
+			      "'%s' should be equal to %pUl", data->uuid, &be);
 }
 
-static void __init test_uuid_wrong(const char *data)
+static void uuid_wrong_le(struct kunit *test)
 {
 	guid_t le;
-	uuid_t be;
-
-	/* LE */
-	total_tests++;
-	if (!guid_parse(data, &le))
-		test_uuid_failed("negative", true, false, data, NULL);
+	const char * const *data = test->param_value;
 
-	/* BE */
-	total_tests++;
-	if (!uuid_parse(data, &be))
-		test_uuid_failed("negative", true, true, data, NULL);
+	KUNIT_ASSERT_NE_MSG(test, guid_parse(*data, &le), 0,
+			    "parsing of '%s' should've failed", *data);
 }
 
-static int __init test_uuid_init(void)
+static void uuid_wrong_be(struct kunit *test)
 {
-	unsigned int i;
-
-	for (i = 0; i < ARRAY_SIZE(test_uuid_test_data); i++)
-		test_uuid_test(&test_uuid_test_data[i]);
-
-	for (i = 0; i < ARRAY_SIZE(test_uuid_wrong_data); i++)
-		test_uuid_wrong(test_uuid_wrong_data[i]);
+	uuid_t be;
+	const char * const *data = test->param_value;
 
-	if (failed_tests == 0)
-		pr_info("all %u tests passed\n", total_tests);
-	else
-		pr_err("failed %u out of %u tests\n", failed_tests, total_tests);
+	KUNIT_ASSERT_NE_MSG(test, uuid_parse(*data, &be), 0,
+			    "parsing of '%s' should've failed", *data);
+}
 
-	return failed_tests ? -EINVAL : 0;
+static void case_to_desc_correct(const struct test_data *t, char *desc)
+{
+	strcpy(desc, t->uuid);
 }
-module_init(test_uuid_init);
 
-static void __exit test_uuid_exit(void)
+KUNIT_ARRAY_PARAM(correct, correct_data, case_to_desc_correct);
+
+static void case_to_desc_wrong(const char * const *s, char *desc)
 {
-	/* do nothing */
+	strcpy(desc, *s);
 }
-module_exit(test_uuid_exit);
+
+KUNIT_ARRAY_PARAM(wrong, wrong_data, case_to_desc_wrong);
+
+static struct kunit_case uuid_test_cases[] = {
+	KUNIT_CASE_PARAM(uuid_correct_be, correct_gen_params),
+	KUNIT_CASE_PARAM(uuid_correct_le, correct_gen_params),
+	KUNIT_CASE_PARAM(uuid_wrong_be, wrong_gen_params),
+	KUNIT_CASE_PARAM(uuid_wrong_le, wrong_gen_params),
+	{}
+};
+
+static struct kunit_suite uuid_test_suite = {
+	.name = "uuid",
+	.test_cases = uuid_test_cases,
+};
+kunit_test_suite(uuid_test_suite);
 
 MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
 MODULE_LICENSE("Dual BSD/GPL");
-- 
2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 1/1] lib: Convert UUID runtime test to KUnit
  2021-06-21 13:31 ` [PATCH v4 1/1] " André Almeida
@ 2021-06-21 21:29   ` Daniel Latypov
  2021-06-21 21:56     ` André Almeida
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Latypov @ 2021-06-21 21:29 UTC (permalink / raw)
  To: André Almeida
  Cc: Christoph Hellwig, Andy Shevchenko, linux-kernel,
	Brendan Higgins, linux-kselftest, kunit-dev, Shuah Khan,
	~lkcamp/patches, nfraprado, leandro.ribeiro, Vitor Massaru Iha,
	lucmaga, David Gow, tales.aparecida

On Mon, Jun 21, 2021 at 6:32 AM André Almeida <andrealmeid@collabora.com> wrote:
>
> Remove custom functions for testing and use KUnit framework. Keep the
> tested functions and test data the same.
>
> Current test threat (g/u)uid_parse and (g/u)uid_equal as different test
> cases. Make both functions being part of the same test case, given the
> dependency regarding their results. This reduces the tests cases from 6
> cases to 4, while keeping the test coverage the same. Given that we have
> 3 strings for each test case, current test output notifies 18 tests
> results, and the KUnit output announces 12 results.
>
> Signed-off-by: André Almeida <andrealmeid@collabora.com>

Reviewed-by: Daniel Latypov <dlatypov@google.com>

Looks good to me.

I haven't been keeping up, but this won't get picked up until KUnit
prints test statistics?
But even if that's the case or not, this patch would be fine as-is.
It'd be strictly an KUnit-internal thing to print test stats.

> ---
>  lib/Kconfig.debug |   8 ++-
>  lib/Makefile      |   2 +-
>  lib/test_uuid.c   | 137 +++++++++++++++++++---------------------------
>  3 files changed, 64 insertions(+), 83 deletions(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 678c13967580..606ec5e2586d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2188,8 +2188,12 @@ config TEST_BITMAP
>
>           If unsure, say N.
>
> -config TEST_UUID
> -       tristate "Test functions located in the uuid module at runtime"
> +config UUID_KUNIT_TEST
> +       tristate "Unit test for UUID" if !KUNIT_ALL_TESTS
> +       depends on KUNIT
> +       default KUNIT_ALL_TESTS
> +       help
> +         Tests parsing functions for UUID/GUID strings.
>
>  config TEST_XARRAY
>         tristate "Test the XArray code at runtime"
> diff --git a/lib/Makefile b/lib/Makefile
> index 2cc359ec1fdd..cc19048961c0 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -85,7 +85,6 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
>  obj-$(CONFIG_TEST_PRINTF) += test_printf.o
>  obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
>  obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
> -obj-$(CONFIG_TEST_UUID) += test_uuid.o
>  obj-$(CONFIG_TEST_XARRAY) += test_xarray.o
>  obj-$(CONFIG_TEST_PARMAN) += test_parman.o
>  obj-$(CONFIG_TEST_KMOD) += test_kmod.o
> @@ -354,5 +353,6 @@ obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
>  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
>  obj-$(CONFIG_BITS_TEST) += test_bits.o
>  obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
> +obj-$(CONFIG_UUID_KUNIT_TEST) += test_uuid.o
>
>  obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
> diff --git a/lib/test_uuid.c b/lib/test_uuid.c
> index cd819c397dc7..30f350301e6d 100644
> --- a/lib/test_uuid.c
> +++ b/lib/test_uuid.c
> @@ -1,21 +1,20 @@
> +// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
>  /*
> - * Test cases for lib/uuid.c module.
> + * Unit tests for lib/uuid.c module.
> + *
> + * Copyright 2016 Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + * Copyright 2021 André Almeida <andrealmeid@riseup.net>
>   */
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/init.h>
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/string.h>
> +#include <kunit/test.h>
>  #include <linux/uuid.h>
>
> -struct test_uuid_data {
> +struct test_data {
>         const char *uuid;
>         guid_t le;
>         uuid_t be;
>  };
>
> -static const struct test_uuid_data test_uuid_test_data[] = {
> +static const struct test_data correct_data[] = {
>         {
>                 .uuid = "c33f4995-3701-450e-9fbf-206a2e98e576",
>                 .le = GUID_INIT(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
> @@ -33,101 +32,79 @@ static const struct test_uuid_data test_uuid_test_data[] = {
>         },
>  };
>
> -static const char * const test_uuid_wrong_data[] = {
> +static const char * const wrong_data[] = {
>         "c33f4995-3701-450e-9fbf206a2e98e576 ", /* no hyphen(s) */
>         "64b4371c-77c1-48f9-8221-29f054XX023b", /* invalid character(s) */
>         "0cb4ddff-a545-4401-9d06-688af53e",     /* not enough data */
>  };
>
> -static unsigned total_tests __initdata;
> -static unsigned failed_tests __initdata;
> -
> -static void __init test_uuid_failed(const char *prefix, bool wrong, bool be,
> -                                   const char *data, const char *actual)
> +static void uuid_correct_le(struct kunit *test)
>  {
> -       pr_err("%s test #%u %s %s data: '%s'\n",
> -              prefix,
> -              total_tests,
> -              wrong ? "passed on wrong" : "failed on",
> -              be ? "BE" : "LE",
> -              data);
> -       if (actual && *actual)
> -               pr_err("%s test #%u actual data: '%s'\n",
> -                      prefix,
> -                      total_tests,
> -                      actual);
> -       failed_tests++;
> +       guid_t le;
> +       const struct test_data *data = test->param_value;
> +
> +       KUNIT_ASSERT_EQ_MSG(test, guid_parse(data->uuid, &le), 0,
> +                           "failed to parse '%s'", data->uuid);
> +       KUNIT_EXPECT_TRUE_MSG(test, guid_equal(&data->le, &le),
> +                             "'%s' should be equal to %pUl", data->uuid, &le);
>  }
>
> -static void __init test_uuid_test(const struct test_uuid_data *data)
> +static void uuid_correct_be(struct kunit *test)
>  {
> -       guid_t le;
>         uuid_t be;
> -       char buf[48];
> -
> -       /* LE */
> -       total_tests++;
> -       if (guid_parse(data->uuid, &le))
> -               test_uuid_failed("conversion", false, false, data->uuid, NULL);
> -
> -       total_tests++;
> -       if (!guid_equal(&data->le, &le)) {
> -               sprintf(buf, "%pUl", &le);
> -               test_uuid_failed("cmp", false, false, data->uuid, buf);
> -       }
> -
> -       /* BE */
> -       total_tests++;
> -       if (uuid_parse(data->uuid, &be))
> -               test_uuid_failed("conversion", false, true, data->uuid, NULL);
> -
> -       total_tests++;
> -       if (!uuid_equal(&data->be, &be)) {
> -               sprintf(buf, "%pUb", &be);
> -               test_uuid_failed("cmp", false, true, data->uuid, buf);
> -       }
> +       const struct test_data *data = test->param_value;
> +
> +       KUNIT_ASSERT_EQ_MSG(test, uuid_parse(data->uuid, &be), 0,
> +                           "failed to parse '%s'", data->uuid);
> +       KUNIT_EXPECT_TRUE_MSG(test, uuid_equal(&data->be, &be),
> +                             "'%s' should be equal to %pUl", data->uuid, &be);
>  }
>
> -static void __init test_uuid_wrong(const char *data)
> +static void uuid_wrong_le(struct kunit *test)
>  {
>         guid_t le;
> -       uuid_t be;
> -
> -       /* LE */
> -       total_tests++;
> -       if (!guid_parse(data, &le))
> -               test_uuid_failed("negative", true, false, data, NULL);
> +       const char * const *data = test->param_value;
>
> -       /* BE */
> -       total_tests++;
> -       if (!uuid_parse(data, &be))
> -               test_uuid_failed("negative", true, true, data, NULL);
> +       KUNIT_ASSERT_NE_MSG(test, guid_parse(*data, &le), 0,
> +                           "parsing of '%s' should've failed", *data);
>  }
>
> -static int __init test_uuid_init(void)
> +static void uuid_wrong_be(struct kunit *test)
>  {
> -       unsigned int i;
> -
> -       for (i = 0; i < ARRAY_SIZE(test_uuid_test_data); i++)
> -               test_uuid_test(&test_uuid_test_data[i]);
> -
> -       for (i = 0; i < ARRAY_SIZE(test_uuid_wrong_data); i++)
> -               test_uuid_wrong(test_uuid_wrong_data[i]);
> +       uuid_t be;
> +       const char * const *data = test->param_value;
>
> -       if (failed_tests == 0)
> -               pr_info("all %u tests passed\n", total_tests);
> -       else
> -               pr_err("failed %u out of %u tests\n", failed_tests, total_tests);
> +       KUNIT_ASSERT_NE_MSG(test, uuid_parse(*data, &be), 0,
> +                           "parsing of '%s' should've failed", *data);
> +}
>
> -       return failed_tests ? -EINVAL : 0;
> +static void case_to_desc_correct(const struct test_data *t, char *desc)
> +{
> +       strcpy(desc, t->uuid);
>  }
> -module_init(test_uuid_init);
>
> -static void __exit test_uuid_exit(void)
> +KUNIT_ARRAY_PARAM(correct, correct_data, case_to_desc_correct);
> +
> +static void case_to_desc_wrong(const char * const *s, char *desc)
>  {
> -       /* do nothing */
> +       strcpy(desc, *s);
>  }
> -module_exit(test_uuid_exit);
> +
> +KUNIT_ARRAY_PARAM(wrong, wrong_data, case_to_desc_wrong);
> +
> +static struct kunit_case uuid_test_cases[] = {
> +       KUNIT_CASE_PARAM(uuid_correct_be, correct_gen_params),
> +       KUNIT_CASE_PARAM(uuid_correct_le, correct_gen_params),
> +       KUNIT_CASE_PARAM(uuid_wrong_be, wrong_gen_params),
> +       KUNIT_CASE_PARAM(uuid_wrong_le, wrong_gen_params),
> +       {}
> +};
> +
> +static struct kunit_suite uuid_test_suite = {
> +       .name = "uuid",
> +       .test_cases = uuid_test_cases,
> +};
> +kunit_test_suite(uuid_test_suite);
>
>  MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
>  MODULE_LICENSE("Dual BSD/GPL");
> --
> 2.32.0
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4 1/1] lib: Convert UUID runtime test to KUnit
  2021-06-21 21:29   ` Daniel Latypov
@ 2021-06-21 21:56     ` André Almeida
  0 siblings, 0 replies; 4+ messages in thread
From: André Almeida @ 2021-06-21 21:56 UTC (permalink / raw)
  To: Daniel Latypov, Andy Shevchenko
  Cc: Christoph Hellwig, linux-kernel, Brendan Higgins,
	linux-kselftest, kunit-dev, Shuah Khan, ~lkcamp/patches,
	nfraprado, leandro.ribeiro, Vitor Massaru Iha, lucmaga,
	David Gow, tales.aparecida

Às 18:29 de 21/06/21, Daniel Latypov escreveu:
> On Mon, Jun 21, 2021 at 6:32 AM André Almeida <andrealmeid@collabora.com> wrote:
>>
>> Remove custom functions for testing and use KUnit framework. Keep the
>> tested functions and test data the same.
>>
>> Current test threat (g/u)uid_parse and (g/u)uid_equal as different test
>> cases. Make both functions being part of the same test case, given the
>> dependency regarding their results. This reduces the tests cases from 6
>> cases to 4, while keeping the test coverage the same. Given that we have
>> 3 strings for each test case, current test output notifies 18 tests
>> results, and the KUnit output announces 12 results.
>>
>> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> 
> Reviewed-by: Daniel Latypov <dlatypov@google.com>
> 
> Looks good to me.
> 
> I haven't been keeping up, but this won't get picked up until KUnit
> prints test statistics?

Good question, I don't think this was decided.

Andy, what do you think?

> But even if that's the case or not, this patch would be fine as-is.
> It'd be strictly an KUnit-internal thing to print test stats.
> 
>> ---
>>  lib/Kconfig.debug |   8 ++-
>>  lib/Makefile      |   2 +-
>>  lib/test_uuid.c   | 137 +++++++++++++++++++---------------------------
>>  3 files changed, 64 insertions(+), 83 deletions(-)
>>
>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>> index 678c13967580..606ec5e2586d 100644
>> --- a/lib/Kconfig.debug
>> +++ b/lib/Kconfig.debug
>> @@ -2188,8 +2188,12 @@ config TEST_BITMAP
>>
>>           If unsure, say N.
>>
>> -config TEST_UUID
>> -       tristate "Test functions located in the uuid module at runtime"
>> +config UUID_KUNIT_TEST
>> +       tristate "Unit test for UUID" if !KUNIT_ALL_TESTS
>> +       depends on KUNIT
>> +       default KUNIT_ALL_TESTS
>> +       help
>> +         Tests parsing functions for UUID/GUID strings.
>>
>>  config TEST_XARRAY
>>         tristate "Test the XArray code at runtime"
>> diff --git a/lib/Makefile b/lib/Makefile
>> index 2cc359ec1fdd..cc19048961c0 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -85,7 +85,6 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
>>  obj-$(CONFIG_TEST_PRINTF) += test_printf.o
>>  obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
>>  obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
>> -obj-$(CONFIG_TEST_UUID) += test_uuid.o
>>  obj-$(CONFIG_TEST_XARRAY) += test_xarray.o
>>  obj-$(CONFIG_TEST_PARMAN) += test_parman.o
>>  obj-$(CONFIG_TEST_KMOD) += test_kmod.o
>> @@ -354,5 +353,6 @@ obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
>>  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
>>  obj-$(CONFIG_BITS_TEST) += test_bits.o
>>  obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
>> +obj-$(CONFIG_UUID_KUNIT_TEST) += test_uuid.o
>>
>>  obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
>> diff --git a/lib/test_uuid.c b/lib/test_uuid.c
>> index cd819c397dc7..30f350301e6d 100644
>> --- a/lib/test_uuid.c
>> +++ b/lib/test_uuid.c
>> @@ -1,21 +1,20 @@
>> +// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
>>  /*
>> - * Test cases for lib/uuid.c module.
>> + * Unit tests for lib/uuid.c module.
>> + *
>> + * Copyright 2016 Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> + * Copyright 2021 André Almeida <andrealmeid@riseup.net>
>>   */
>> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> -
>> -#include <linux/init.h>
>> -#include <linux/kernel.h>
>> -#include <linux/module.h>
>> -#include <linux/string.h>
>> +#include <kunit/test.h>
>>  #include <linux/uuid.h>
>>
>> -struct test_uuid_data {
>> +struct test_data {
>>         const char *uuid;
>>         guid_t le;
>>         uuid_t be;
>>  };
>>
>> -static const struct test_uuid_data test_uuid_test_data[] = {
>> +static const struct test_data correct_data[] = {
>>         {
>>                 .uuid = "c33f4995-3701-450e-9fbf-206a2e98e576",
>>                 .le = GUID_INIT(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
>> @@ -33,101 +32,79 @@ static const struct test_uuid_data test_uuid_test_data[] = {
>>         },
>>  };
>>
>> -static const char * const test_uuid_wrong_data[] = {
>> +static const char * const wrong_data[] = {
>>         "c33f4995-3701-450e-9fbf206a2e98e576 ", /* no hyphen(s) */
>>         "64b4371c-77c1-48f9-8221-29f054XX023b", /* invalid character(s) */
>>         "0cb4ddff-a545-4401-9d06-688af53e",     /* not enough data */
>>  };
>>
>> -static unsigned total_tests __initdata;
>> -static unsigned failed_tests __initdata;
>> -
>> -static void __init test_uuid_failed(const char *prefix, bool wrong, bool be,
>> -                                   const char *data, const char *actual)
>> +static void uuid_correct_le(struct kunit *test)
>>  {
>> -       pr_err("%s test #%u %s %s data: '%s'\n",
>> -              prefix,
>> -              total_tests,
>> -              wrong ? "passed on wrong" : "failed on",
>> -              be ? "BE" : "LE",
>> -              data);
>> -       if (actual && *actual)
>> -               pr_err("%s test #%u actual data: '%s'\n",
>> -                      prefix,
>> -                      total_tests,
>> -                      actual);
>> -       failed_tests++;
>> +       guid_t le;
>> +       const struct test_data *data = test->param_value;
>> +
>> +       KUNIT_ASSERT_EQ_MSG(test, guid_parse(data->uuid, &le), 0,
>> +                           "failed to parse '%s'", data->uuid);
>> +       KUNIT_EXPECT_TRUE_MSG(test, guid_equal(&data->le, &le),
>> +                             "'%s' should be equal to %pUl", data->uuid, &le);
>>  }
>>
>> -static void __init test_uuid_test(const struct test_uuid_data *data)
>> +static void uuid_correct_be(struct kunit *test)
>>  {
>> -       guid_t le;
>>         uuid_t be;
>> -       char buf[48];
>> -
>> -       /* LE */
>> -       total_tests++;
>> -       if (guid_parse(data->uuid, &le))
>> -               test_uuid_failed("conversion", false, false, data->uuid, NULL);
>> -
>> -       total_tests++;
>> -       if (!guid_equal(&data->le, &le)) {
>> -               sprintf(buf, "%pUl", &le);
>> -               test_uuid_failed("cmp", false, false, data->uuid, buf);
>> -       }
>> -
>> -       /* BE */
>> -       total_tests++;
>> -       if (uuid_parse(data->uuid, &be))
>> -               test_uuid_failed("conversion", false, true, data->uuid, NULL);
>> -
>> -       total_tests++;
>> -       if (!uuid_equal(&data->be, &be)) {
>> -               sprintf(buf, "%pUb", &be);
>> -               test_uuid_failed("cmp", false, true, data->uuid, buf);
>> -       }
>> +       const struct test_data *data = test->param_value;
>> +
>> +       KUNIT_ASSERT_EQ_MSG(test, uuid_parse(data->uuid, &be), 0,
>> +                           "failed to parse '%s'", data->uuid);
>> +       KUNIT_EXPECT_TRUE_MSG(test, uuid_equal(&data->be, &be),
>> +                             "'%s' should be equal to %pUl", data->uuid, &be);
>>  }
>>
>> -static void __init test_uuid_wrong(const char *data)
>> +static void uuid_wrong_le(struct kunit *test)
>>  {
>>         guid_t le;
>> -       uuid_t be;
>> -
>> -       /* LE */
>> -       total_tests++;
>> -       if (!guid_parse(data, &le))
>> -               test_uuid_failed("negative", true, false, data, NULL);
>> +       const char * const *data = test->param_value;
>>
>> -       /* BE */
>> -       total_tests++;
>> -       if (!uuid_parse(data, &be))
>> -               test_uuid_failed("negative", true, true, data, NULL);
>> +       KUNIT_ASSERT_NE_MSG(test, guid_parse(*data, &le), 0,
>> +                           "parsing of '%s' should've failed", *data);
>>  }
>>
>> -static int __init test_uuid_init(void)
>> +static void uuid_wrong_be(struct kunit *test)
>>  {
>> -       unsigned int i;
>> -
>> -       for (i = 0; i < ARRAY_SIZE(test_uuid_test_data); i++)
>> -               test_uuid_test(&test_uuid_test_data[i]);
>> -
>> -       for (i = 0; i < ARRAY_SIZE(test_uuid_wrong_data); i++)
>> -               test_uuid_wrong(test_uuid_wrong_data[i]);
>> +       uuid_t be;
>> +       const char * const *data = test->param_value;
>>
>> -       if (failed_tests == 0)
>> -               pr_info("all %u tests passed\n", total_tests);
>> -       else
>> -               pr_err("failed %u out of %u tests\n", failed_tests, total_tests);
>> +       KUNIT_ASSERT_NE_MSG(test, uuid_parse(*data, &be), 0,
>> +                           "parsing of '%s' should've failed", *data);
>> +}
>>
>> -       return failed_tests ? -EINVAL : 0;
>> +static void case_to_desc_correct(const struct test_data *t, char *desc)
>> +{
>> +       strcpy(desc, t->uuid);
>>  }
>> -module_init(test_uuid_init);
>>
>> -static void __exit test_uuid_exit(void)
>> +KUNIT_ARRAY_PARAM(correct, correct_data, case_to_desc_correct);
>> +
>> +static void case_to_desc_wrong(const char * const *s, char *desc)
>>  {
>> -       /* do nothing */
>> +       strcpy(desc, *s);
>>  }
>> -module_exit(test_uuid_exit);
>> +
>> +KUNIT_ARRAY_PARAM(wrong, wrong_data, case_to_desc_wrong);
>> +
>> +static struct kunit_case uuid_test_cases[] = {
>> +       KUNIT_CASE_PARAM(uuid_correct_be, correct_gen_params),
>> +       KUNIT_CASE_PARAM(uuid_correct_le, correct_gen_params),
>> +       KUNIT_CASE_PARAM(uuid_wrong_be, wrong_gen_params),
>> +       KUNIT_CASE_PARAM(uuid_wrong_le, wrong_gen_params),
>> +       {}
>> +};
>> +
>> +static struct kunit_suite uuid_test_suite = {
>> +       .name = "uuid",
>> +       .test_cases = uuid_test_cases,
>> +};
>> +kunit_test_suite(uuid_test_suite);
>>
>>  MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
>>  MODULE_LICENSE("Dual BSD/GPL");
>> --
>> 2.32.0
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-21 21:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 13:31 [PATCH v4 0/1] lib: Convert UUID runtime test to KUnit André Almeida
2021-06-21 13:31 ` [PATCH v4 1/1] " André Almeida
2021-06-21 21:29   ` Daniel Latypov
2021-06-21 21:56     ` André Almeida

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).