linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit
@ 2021-06-09 23:37 André Almeida
  2021-06-09 23:37 ` [PATCH v2 1/1] " André Almeida
  2021-06-10  9:13 ` [PATCH v2 0/1] " Andy Shevchenko
  0 siblings, 2 replies; 8+ messages in thread
From: André Almeida @ 2021-06-09 23:37 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 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

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

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

-- 
2.31.1


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

* [PATCH v2 1/1] lib: Convert UUID runtime test to KUnit
  2021-06-09 23:37 [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit André Almeida
@ 2021-06-09 23:37 ` André Almeida
  2021-06-10  9:13 ` [PATCH v2 0/1] " Andy Shevchenko
  1 sibling, 0 replies; 8+ messages in thread
From: André Almeida @ 2021-06-09 23:37 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. Test cases
and test data remains the same.

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

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 678c13967580..1d879197f303 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2188,8 +2188,15 @@ 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
+	  This builds the UUID unit test.
+	  Tests parsing functions for UUID/GUID strings.
+
+	  If unsure, say N.
 
 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..65394ec5501e 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 = (const struct test_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 = (const struct test_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 **data = (const char **)(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 **data = (const char **)(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 related	[flat|nested] 8+ messages in thread

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

On Wed, Jun 09, 2021 at 08:37:29PM -0300, André Almeida wrote:
> 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

Thanks!

It's not your fault but I think we need to defer this until KUnit gains support
of the run statistics. My guts telling me if we allow more and more conversions
like this the point will vanish and nobody will care.

I like the code, but I can give my tag after KUnit prints some kind of this:

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

   test_uuid: all 18 tests passed

 * And when it fails:

   test_uuid: failed 18 out of 18 tests

> 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
> 
> André Almeida (1):
>   lib: Convert UUID runtime test to KUnit
> 
>  lib/Kconfig.debug |  11 +++-
>  lib/Makefile      |   2 +-
>  lib/test_uuid.c   | 137 +++++++++++++++++++---------------------------
>  3 files changed, 67 insertions(+), 83 deletions(-)
> 
> -- 
> 2.31.1
> 

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit
  2021-06-10  9:13 ` [PATCH v2 0/1] " Andy Shevchenko
@ 2021-06-10 11:53   ` David Gow
  2021-06-10 12:39     ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: David Gow @ 2021-06-10 11:53 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: André Almeida, Christoph Hellwig, Linux Kernel Mailing List,
	Brendan Higgins, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Shuah Khan, ~lkcamp/patches, nfraprado,
	leandro.ribeiro, Vitor Massaru Iha, lucmaga, Daniel Latypov,
	tales.aparecida

On Thu, Jun 10, 2021 at 5:14 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Jun 09, 2021 at 08:37:29PM -0300, André Almeida wrote:
> > 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

Note that this output is from the kunit_tool script, which parses the
test output.
It does include a summary line:
[04:41:01] Testing complete. 4 tests run. 0 failed. 0 crashed.

Note that this does only count the number of "tests" run --- the
individual UUIDs are parameters to the same test, so aren't counted
independently by the wrapper at the moment.

That being said, the raw output looks like this (all tests passed):
TAP version 14
1..1
   # Subtest: uuid
   1..4
   # uuid_correct_be: ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
   # uuid_correct_be: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
   # uuid_correct_be: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
   ok 1 - uuid_correct_be
   # uuid_correct_le: ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
   # uuid_correct_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
   # uuid_correct_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
   ok 2 - uuid_correct_le
   # uuid_wrong_be: ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
   # uuid_wrong_be: ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
   # uuid_wrong_be: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
   ok 3 - uuid_wrong_be
   # uuid_wrong_le: ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
   # uuid_wrong_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
   # uuid_wrong_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
   ok 4 - uuid_wrong_le
ok 1 - uuid

A test which failed could look like this:
    # uuid_correct_le: ASSERTION FAILED at lib/test_uuid.c:46
   Expected guid_parse(data->uuid, &le) == 0, but
       guid_parse(data->uuid, &le) == -22

failed to parse 'c33f499x5-3701-450e-9fbf-206a2e98e576'
   # uuid_correct_le: not ok 1 - c33f499x5-3701-450e-9fbf-206a2e98e576
   # uuid_correct_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
   # uuid_correct_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
   not ok 2 - uuid_correct_le

>
> Thanks!
>
> It's not your fault but I think we need to defer this until KUnit gains support
> of the run statistics. My guts telling me if we allow more and more conversions
> like this the point will vanish and nobody will care.

Did the test statistics patch we sent out before meet your expectations?
https://patchwork.kernel.org/project/linux-kselftest/patch/20201211072319.533803-1-davidgow@google.com/

If so, we can tidy it up and try to push it through straight away, we
were just waiting for a review from someone who wanted the feature.


> I like the code, but I can give my tag after KUnit prints some kind of this:
>
>  * This is how the current output looks like in success:
>
>    test_uuid: all 18 tests passed
>
>  * And when it fails:
>
>    test_uuid: failed 18 out of 18 tests
>

There are some small restrictions on the exact format KUnit can use
for this if we want to continue to match the (K)TAP specification
which is being adopted by kselftest. The patch linked above should
give something formatted like:

# test_uuid: (0 / 4) tests failed (0 / 12 test parameters)

Would that work for you?

Cheers,
-- David

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

* Re: [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit
  2021-06-10 11:53   ` David Gow
@ 2021-06-10 12:39     ` Andy Shevchenko
  2021-06-10 13:08       ` André Almeida
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2021-06-10 12:39 UTC (permalink / raw)
  To: David Gow
  Cc: Andy Shevchenko, André Almeida, Christoph Hellwig,
	Linux Kernel Mailing List, Brendan Higgins,
	open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Shuah Khan, ~lkcamp/patches, nfraprado, leandro.ribeiro,
	Vitor Massaru Iha, lucmaga, Daniel Latypov, tales.aparecida

On Thu, Jun 10, 2021 at 2:54 PM David Gow <davidgow@google.com> wrote:
> On Thu, Jun 10, 2021 at 5:14 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Wed, Jun 09, 2021 at 08:37:29PM -0300, André Almeida wrote:

...

> Note that this output is from the kunit_tool script, which parses the
> test output.
> It does include a summary line:
> [04:41:01] Testing complete. 4 tests run. 0 failed. 0 crashed.

> Note that this does only count the number of "tests" run --- the
> individual UUIDs are parameters to the same test, so aren't counted
> independently by the wrapper at the moment.
>
> That being said, the raw output looks like this (all tests passed):
> TAP version 14
> 1..1
>    # Subtest: uuid
>    1..4
>    # uuid_correct_be: ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
>    # uuid_correct_be: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
>    # uuid_correct_be: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
>    ok 1 - uuid_correct_be
>    # uuid_correct_le: ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
>    # uuid_correct_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
>    # uuid_correct_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
>    ok 2 - uuid_correct_le
>    # uuid_wrong_be: ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
>    # uuid_wrong_be: ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
>    # uuid_wrong_be: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
>    ok 3 - uuid_wrong_be
>    # uuid_wrong_le: ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
>    # uuid_wrong_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
>    # uuid_wrong_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
>    ok 4 - uuid_wrong_le
> ok 1 - uuid
>
> A test which failed could look like this:
>     # uuid_correct_le: ASSERTION FAILED at lib/test_uuid.c:46
>    Expected guid_parse(data->uuid, &le) == 0, but
>        guid_parse(data->uuid, &le) == -22
>
> failed to parse 'c33f499x5-3701-450e-9fbf-206a2e98e576'
>    # uuid_correct_le: not ok 1 - c33f499x5-3701-450e-9fbf-206a2e98e576
>    # uuid_correct_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
>    # uuid_correct_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
>    not ok 2 - uuid_correct_le
>
> >
> > Thanks!
> >
> > It's not your fault but I think we need to defer this until KUnit gains support
> > of the run statistics. My guts telling me if we allow more and more conversions
> > like this the point will vanish and nobody will care.
>
> Did the test statistics patch we sent out before meet your expectations?
> https://patchwork.kernel.org/project/linux-kselftest/patch/20201211072319.533803-1-davidgow@google.com/

Let me look at it at some point.

> If so, we can tidy it up and try to push it through straight away, we
> were just waiting for a review from someone who wanted the feature.
>
>
> > I like the code, but I can give my tag after KUnit prints some kind of this:
> >
> >  * This is how the current output looks like in success:
> >
> >    test_uuid: all 18 tests passed
> >
> >  * And when it fails:
> >
> >    test_uuid: failed 18 out of 18 tests
> >
>
> There are some small restrictions on the exact format KUnit can use
> for this if we want to continue to match the (K)TAP specification
> which is being adopted by kselftest. The patch linked above should
> give something formatted like:
>
> # test_uuid: (0 / 4) tests failed (0 / 12 test parameters)
>
> Would that work for you?

Can you decode it for me, please?

(Assuming that the above question arisen, perhaps some rephrasing is
needed. The idea that user should have clear understanding on how many
test cases were run and how many of them successfully finished or
failed. According to this thread I have to see the cumulative number
of 18 (either as one number or sum over test cases or how you call
them, I see 4 here).



-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit
  2021-06-10 12:39     ` Andy Shevchenko
@ 2021-06-10 13:08       ` André Almeida
  2021-06-10 13:52         ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: André Almeida @ 2021-06-10 13:08 UTC (permalink / raw)
  To: Andy Shevchenko, David Gow
  Cc: Andy Shevchenko, Christoph Hellwig, Linux Kernel Mailing List,
	Brendan Higgins, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Shuah Khan, ~lkcamp/patches, nfraprado,
	leandro.ribeiro, Vitor Massaru Iha, lucmaga, Daniel Latypov,
	tales.aparecida

Às 09:39 de 10/06/21, Andy Shevchenko escreveu:
> On Thu, Jun 10, 2021 at 2:54 PM David Gow <davidgow@google.com> wrote:
>> On Thu, Jun 10, 2021 at 5:14 PM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>> On Wed, Jun 09, 2021 at 08:37:29PM -0300, André Almeida wrote:
> 
> ...
> 
>> Note that this output is from the kunit_tool script, which parses the
>> test output.
>> It does include a summary line:
>> [04:41:01] Testing complete. 4 tests run. 0 failed. 0 crashed.
> 
>> Note that this does only count the number of "tests" run --- the
>> individual UUIDs are parameters to the same test, so aren't counted
>> independently by the wrapper at the moment.
>>
>> That being said, the raw output looks like this (all tests passed):
>> TAP version 14
>> 1..1
>>    # Subtest: uuid
>>    1..4
>>    # uuid_correct_be: ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
>>    # uuid_correct_be: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
>>    # uuid_correct_be: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
>>    ok 1 - uuid_correct_be
>>    # uuid_correct_le: ok 1 - c33f4995-3701-450e-9fbf-206a2e98e576
>>    # uuid_correct_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
>>    # uuid_correct_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
>>    ok 2 - uuid_correct_le
>>    # uuid_wrong_be: ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
>>    # uuid_wrong_be: ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
>>    # uuid_wrong_be: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
>>    ok 3 - uuid_wrong_be
>>    # uuid_wrong_le: ok 1 - c33f4995-3701-450e-9fbf206a2e98e576
>>    # uuid_wrong_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054XX023b
>>    # uuid_wrong_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e
>>    ok 4 - uuid_wrong_le
>> ok 1 - uuid
>>
>> A test which failed could look like this:
>>     # uuid_correct_le: ASSERTION FAILED at lib/test_uuid.c:46
>>    Expected guid_parse(data->uuid, &le) == 0, but
>>        guid_parse(data->uuid, &le) == -22
>>
>> failed to parse 'c33f499x5-3701-450e-9fbf-206a2e98e576'
>>    # uuid_correct_le: not ok 1 - c33f499x5-3701-450e-9fbf-206a2e98e576
>>    # uuid_correct_le: ok 2 - 64b4371c-77c1-48f9-8221-29f054fc023b
>>    # uuid_correct_le: ok 3 - 0cb4ddff-a545-4401-9d06-688af53e7f84
>>    not ok 2 - uuid_correct_le
>>
>>>
>>> Thanks!
>>>
>>> It's not your fault but I think we need to defer this until KUnit gains support
>>> of the run statistics. My guts telling me if we allow more and more conversions
>>> like this the point will vanish and nobody will care.
>>
>> Did the test statistics patch we sent out before meet your expectations?
>> https://patchwork.kernel.org/project/linux-kselftest/patch/20201211072319.533803-1-davidgow@google.com/
> 
> Let me look at it at some point.
> 
>> If so, we can tidy it up and try to push it through straight away, we
>> were just waiting for a review from someone who wanted the feature.
>>
>>
>>> I like the code, but I can give my tag after KUnit prints some kind of this:
>>>
>>>  * This is how the current output looks like in success:
>>>
>>>    test_uuid: all 18 tests passed
>>>
>>>  * And when it fails:
>>>
>>>    test_uuid: failed 18 out of 18 tests
>>>
>>
>> There are some small restrictions on the exact format KUnit can use
>> for this if we want to continue to match the (K)TAP specification
>> which is being adopted by kselftest. The patch linked above should
>> give something formatted like:
>>
>> # test_uuid: (0 / 4) tests failed (0 / 12 test parameters)
>>
>> Would that work for you?
> 
> Can you decode it for me, please?
> 
> (Assuming that the above question arisen, perhaps some rephrasing is
> needed. The idea that user should have clear understanding on how many
> test cases were run and how many of them successfully finished or
> failed. According to this thread I have to see the cumulative number
> of 18 (either as one number or sum over test cases or how you call
> them, I see 4 here).
> 
> 

In the original code, each `if(uuid/guid_parse/equal)` was considered as
a test, so there were 4 tests for the 3 correct inputs and 2 tests for
the 3 wrong inputs: 4 * 3 + 2 * 3 = 18 tests.

In my patch, I've organized in a different way, with 4 test cases:

- A test case for guid_parse and guid_equal for correct inputs
- A test case for uuid_parse and uuid_equal for correct inputs
- A test case for guid_parse for incorrect inputs
- A test case for uuid_parse for incorrect inputs

So now we have 4 test cases, instead of the 6 test cases in the original
code, because I've united _parse and _equal in a single test case. Given
that each test has 3 parameters, this is why we see 12 test parameters
and that's why there's no "18 tests" around anymore.

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

* Re: [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit
  2021-06-10 13:08       ` André Almeida
@ 2021-06-10 13:52         ` Andy Shevchenko
  2021-06-10 13:56           ` André Almeida
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2021-06-10 13:52 UTC (permalink / raw)
  To: André Almeida
  Cc: David Gow, Christoph Hellwig, Linux Kernel Mailing List,
	Brendan Higgins, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Shuah Khan, ~lkcamp/patches, nfraprado,
	leandro.ribeiro, Vitor Massaru Iha, lucmaga, Daniel Latypov,
	tales.aparecida

On Thu, Jun 10, 2021 at 10:08:30AM -0300, André Almeida wrote:
> Às 09:39 de 10/06/21, Andy Shevchenko escreveu:
> > On Thu, Jun 10, 2021 at 2:54 PM David Gow <davidgow@google.com> wrote:
> >> On Thu, Jun 10, 2021 at 5:14 PM Andy Shevchenko
> >> <andriy.shevchenko@linux.intel.com> wrote:
> >>> On Wed, Jun 09, 2021 at 08:37:29PM -0300, André Almeida wrote:

...

> >>> It's not your fault but I think we need to defer this until KUnit gains support
> >>> of the run statistics. My guts telling me if we allow more and more conversions
> >>> like this the point will vanish and nobody will care.
> >>
> >> Did the test statistics patch we sent out before meet your expectations?
> >> https://patchwork.kernel.org/project/linux-kselftest/patch/20201211072319.533803-1-davidgow@google.com/
> > 
> > Let me look at it at some point.
> > 
> >> If so, we can tidy it up and try to push it through straight away, we
> >> were just waiting for a review from someone who wanted the feature.
> >>
> >>
> >>> I like the code, but I can give my tag after KUnit prints some kind of this:
> >>>
> >>>  * This is how the current output looks like in success:
> >>>
> >>>    test_uuid: all 18 tests passed
> >>>
> >>>  * And when it fails:
> >>>
> >>>    test_uuid: failed 18 out of 18 tests
> >>>
> >>
> >> There are some small restrictions on the exact format KUnit can use
> >> for this if we want to continue to match the (K)TAP specification
> >> which is being adopted by kselftest. The patch linked above should
> >> give something formatted like:
> >>
> >> # test_uuid: (0 / 4) tests failed (0 / 12 test parameters)
> >>
> >> Would that work for you?
> > 
> > Can you decode it for me, please?
> > 
> > (Assuming that the above question arisen, perhaps some rephrasing is
> > needed. The idea that user should have clear understanding on how many
> > test cases were run and how many of them successfully finished or
> > failed. According to this thread I have to see the cumulative number
> > of 18 (either as one number or sum over test cases or how you call
> > them, I see 4 here).
> 
> In the original code, each `if(uuid/guid_parse/equal)` was considered as
> a test, so there were 4 tests for the 3 correct inputs and 2 tests for
> the 3 wrong inputs: 4 * 3 + 2 * 3 = 18 tests.
> 
> In my patch, I've organized in a different way, with 4 test cases:
> 
> - A test case for guid_parse and guid_equal for correct inputs
> - A test case for uuid_parse and uuid_equal for correct inputs
> - A test case for guid_parse for incorrect inputs
> - A test case for uuid_parse for incorrect inputs
> 
> So now we have 4 test cases, instead of the 6 test cases in the original
> code, because I've united _parse and _equal in a single test case. Given
> that each test has 3 parameters, this is why we see 12 test parameters
> and that's why there's no "18 tests" around anymore.

I see, is it mentioned in the commit message? If no, please add this
explanation.

Let's assume 12 now is the correct number, then the output can be somehow
rephrased, but again, it's not in your patch anyway :-)

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit
  2021-06-10 13:52         ` Andy Shevchenko
@ 2021-06-10 13:56           ` André Almeida
  0 siblings, 0 replies; 8+ messages in thread
From: André Almeida @ 2021-06-10 13:56 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: David Gow, Christoph Hellwig, Linux Kernel Mailing List,
	Brendan Higgins, open list:KERNEL SELFTEST FRAMEWORK,
	KUnit Development, Shuah Khan, ~lkcamp/patches, nfraprado,
	leandro.ribeiro, Vitor Massaru Iha, lucmaga, Daniel Latypov,
	tales.aparecida



Às 10:52 de 10/06/21, Andy Shevchenko escreveu:
> On Thu, Jun 10, 2021 at 10:08:30AM -0300, André Almeida wrote:
>> Às 09:39 de 10/06/21, Andy Shevchenko escreveu:
>>> On Thu, Jun 10, 2021 at 2:54 PM David Gow <davidgow@google.com> wrote:
>>>> On Thu, Jun 10, 2021 at 5:14 PM Andy Shevchenko
>>>> <andriy.shevchenko@linux.intel.com> wrote:
>>>>> On Wed, Jun 09, 2021 at 08:37:29PM -0300, André Almeida wrote:
> 
> ...
> 
>>>>> It's not your fault but I think we need to defer this until KUnit gains support
>>>>> of the run statistics. My guts telling me if we allow more and more conversions
>>>>> like this the point will vanish and nobody will care.
>>>>
>>>> Did the test statistics patch we sent out before meet your expectations?
>>>> https://patchwork.kernel.org/project/linux-kselftest/patch/20201211072319.533803-1-davidgow@google.com/
>>>
>>> Let me look at it at some point.
>>>
>>>> If so, we can tidy it up and try to push it through straight away, we
>>>> were just waiting for a review from someone who wanted the feature.
>>>>
>>>>
>>>>> I like the code, but I can give my tag after KUnit prints some kind of this:
>>>>>
>>>>>  * This is how the current output looks like in success:
>>>>>
>>>>>    test_uuid: all 18 tests passed
>>>>>
>>>>>  * And when it fails:
>>>>>
>>>>>    test_uuid: failed 18 out of 18 tests
>>>>>
>>>>
>>>> There are some small restrictions on the exact format KUnit can use
>>>> for this if we want to continue to match the (K)TAP specification
>>>> which is being adopted by kselftest. The patch linked above should
>>>> give something formatted like:
>>>>
>>>> # test_uuid: (0 / 4) tests failed (0 / 12 test parameters)
>>>>
>>>> Would that work for you?
>>>
>>> Can you decode it for me, please?
>>>
>>> (Assuming that the above question arisen, perhaps some rephrasing is
>>> needed. The idea that user should have clear understanding on how many
>>> test cases were run and how many of them successfully finished or
>>> failed. According to this thread I have to see the cumulative number
>>> of 18 (either as one number or sum over test cases or how you call
>>> them, I see 4 here).
>>
>> In the original code, each `if(uuid/guid_parse/equal)` was considered as
>> a test, so there were 4 tests for the 3 correct inputs and 2 tests for
>> the 3 wrong inputs: 4 * 3 + 2 * 3 = 18 tests.
>>
>> In my patch, I've organized in a different way, with 4 test cases:
>>
>> - A test case for guid_parse and guid_equal for correct inputs
>> - A test case for uuid_parse and uuid_equal for correct inputs
>> - A test case for guid_parse for incorrect inputs
>> - A test case for uuid_parse for incorrect inputs
>>
>> So now we have 4 test cases, instead of the 6 test cases in the original
>> code, because I've united _parse and _equal in a single test case. Given
>> that each test has 3 parameters, this is why we see 12 test parameters
>> and that's why there's no "18 tests" around anymore.
> 
> I see, is it mentioned in the commit message? If no, please add this
> explanation.
> 
> Let's assume 12 now is the correct number, then the output can be somehow
> rephrased, but again, it's not in your patch anyway :-)
> 

Sure! I'll add this for v3 :)

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

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

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 23:37 [PATCH v2 0/1] lib: Convert UUID runtime test to KUnit André Almeida
2021-06-09 23:37 ` [PATCH v2 1/1] " André Almeida
2021-06-10  9:13 ` [PATCH v2 0/1] " Andy Shevchenko
2021-06-10 11:53   ` David Gow
2021-06-10 12:39     ` Andy Shevchenko
2021-06-10 13:08       ` André Almeida
2021-06-10 13:52         ` Andy Shevchenko
2021-06-10 13: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).