linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
@ 2020-07-29 20:11 Vitor Massaru Iha
  2020-07-29 20:39 ` peterz
  2020-07-29 22:56 ` Ian Rogers via Linux-kernel-mentees
  0 siblings, 2 replies; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-07-29 20:11 UTC (permalink / raw)
  To: kunit-dev
  Cc: irogers, peterz, brendanhiggins, linux-kernel, linux-kselftest,
	linux-kernel-mentees, mingo

This adds the conversion of the runtime tests of test_min_heap,
from `lib/test_min_heap.c` to KUnit tests.

Please apply this commit first (linux-kselftest/kunit-fixes):
3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space

Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
---
 lib/Kconfig.debug                         |  29 ++++--
 lib/Makefile                              |   2 +-
 lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
 3 files changed, 83 insertions(+), 65 deletions(-)
 rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210d70a1..46674fc4972c 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1864,16 +1864,6 @@ config TEST_LIST_SORT

 	  If unsure, say N.

-config TEST_MIN_HEAP
-	tristate "Min heap test"
-	depends on DEBUG_KERNEL || m
-	help
-	  Enable this to turn on min heap function tests. This test is
-	  executed only once during system boot (so affects only boot time),
-	  or at module load time.
-
-	  If unsure, say N.
-
 config TEST_SORT
 	tristate "Array-based sort test"
 	depends on DEBUG_KERNEL || m
@@ -2185,6 +2175,25 @@ config LINEAR_RANGES_TEST

 	  If unsure, say N.

+config MIN_HEAP_KUNIT
+        tristate "KUnit test for Min heap"
+        depends on KUNIT
+        depends on DEBUG_KERNEL || m
+        help
+          Enable this to turn on min heap function tests. This test is
+          executed only once during system boot (so affects only boot time),
+          or at module load time.
+
+          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 the KUnit test harness, and not intended 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.
+
 config TEST_UDELAY
 	tristate "udelay test driver"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index b1c42c10073b..748f57063160 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -72,7 +72,6 @@ CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
 UBSAN_SANITIZE_test_ubsan.o := y
 obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
 obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
-obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
 obj-$(CONFIG_TEST_LKM) += test_module.o
 obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
 obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
@@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
 # KUnit tests
 obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
 obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
+obj-$(CONFIG_MIN_HEAP_KUNIT) += min_heap_kunit.o
diff --git a/lib/test_min_heap.c b/lib/min_heap_kunit.c
similarity index 60%
rename from lib/test_min_heap.c
rename to lib/min_heap_kunit.c
index d19c8080fd4d..398db1c63146 100644
--- a/lib/test_min_heap.c
+++ b/lib/min_heap_kunit.c
@@ -7,9 +7,8 @@

 #include <linux/log2.h>
 #include <linux/min_heap.h>
-#include <linux/module.h>
-#include <linux/printk.h>
 #include <linux/random.h>
+#include <kunit/test.h>

 static __init bool less_than(const void *lhs, const void *rhs)
 {
@@ -29,37 +28,34 @@ static __init void swap_ints(void *lhs, void *rhs)
 	*(int *)rhs = temp;
 }

-static __init int pop_verify_heap(bool min_heap,
+static __init void pop_verify_heap(struct kunit *context,
+				bool min_heap,
 				struct min_heap *heap,
 				const struct min_heap_callbacks *funcs)
 {
 	int *values = heap->data;
-	int err = 0;
 	int last;

 	last = values[0];
 	min_heap_pop(heap, funcs);
 	while (heap->nr > 0) {
 		if (min_heap) {
-			if (last > values[0]) {
-				pr_err("error: expected %d <= %d\n", last,
-					values[0]);
-				err++;
-			}
+			KUNIT_EXPECT_FALSE_MSG(context,
+					       last > values[0],
+					       "expected %d <= %d\n",
+					       last, values[0]);
 		} else {
-			if (last < values[0]) {
-				pr_err("error: expected %d >= %d\n", last,
-					values[0]);
-				err++;
-			}
+			KUNIT_EXPECT_FALSE_MSG(context,
+					       last < values[0],
+					       "expected %d >= %d\n",
+					       last, values[0]);
 		}
 		last = values[0];
 		min_heap_pop(heap, funcs);
 	}
-	return err;
 }

-static __init int test_heapify_all(bool min_heap)
+static __init void test_heapify_all(struct kunit *context, bool min_heap)
 {
 	int values[] = { 3, 1, 2, 4, 0x8000000, 0x7FFFFFF, 0,
 			 -3, -1, -2, -4, 0x8000000, 0x7FFFFFF };
@@ -73,12 +69,11 @@ static __init int test_heapify_all(bool min_heap)
 		.less = min_heap ? less_than : greater_than,
 		.swp = swap_ints,
 	};
-	int i, err;
+	int i;

 	/* Test with known set of values. */
 	min_heapify_all(&heap, &funcs);
-	err = pop_verify_heap(min_heap, &heap, &funcs);
-
+	pop_verify_heap(context, min_heap, &heap, &funcs);

 	/* Test with randomly generated values. */
 	heap.nr = ARRAY_SIZE(values);
@@ -86,12 +81,10 @@ static __init int test_heapify_all(bool min_heap)
 		values[i] = get_random_int();

 	min_heapify_all(&heap, &funcs);
-	err += pop_verify_heap(min_heap, &heap, &funcs);
-
-	return err;
+	pop_verify_heap(context, min_heap, &heap, &funcs);
 }

-static __init int test_heap_push(bool min_heap)
+static __init void test_heap_push(struct kunit *context, bool min_heap)
 {
 	const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
 			     -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
@@ -106,25 +99,22 @@ static __init int test_heap_push(bool min_heap)
 		.less = min_heap ? less_than : greater_than,
 		.swp = swap_ints,
 	};
-	int i, temp, err;
+	int i, temp;

 	/* Test with known set of values copied from data. */
 	for (i = 0; i < ARRAY_SIZE(data); i++)
 		min_heap_push(&heap, &data[i], &funcs);
-
-	err = pop_verify_heap(min_heap, &heap, &funcs);
+	pop_verify_heap(context, min_heap, &heap, &funcs);

 	/* Test with randomly generated values. */
 	while (heap.nr < heap.size) {
 		temp = get_random_int();
 		min_heap_push(&heap, &temp, &funcs);
 	}
-	err += pop_verify_heap(min_heap, &heap, &funcs);
-
-	return err;
+	pop_verify_heap(context, min_heap, &heap, &funcs);
 }

-static __init int test_heap_pop_push(bool min_heap)
+static __init void test_heap_pop_push(struct kunit *context, bool min_heap)
 {
 	const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
 			     -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
@@ -139,7 +129,7 @@ static __init int test_heap_pop_push(bool min_heap)
 		.less = min_heap ? less_than : greater_than,
 		.swp = swap_ints,
 	};
-	int i, temp, err;
+	int i, temp;

 	/* Fill values with data to pop and replace. */
 	temp = min_heap ? 0x80000000 : 0x7FFFFFFF;
@@ -149,8 +139,7 @@ static __init int test_heap_pop_push(bool min_heap)
 	/* Test with known set of values copied from data. */
 	for (i = 0; i < ARRAY_SIZE(data); i++)
 		min_heap_pop_push(&heap, &data[i], &funcs);
-
-	err = pop_verify_heap(min_heap, &heap, &funcs);
+	pop_verify_heap(context, min_heap, &heap, &funcs);

 	heap.nr = 0;
 	for (i = 0; i < ARRAY_SIZE(data); i++)
@@ -161,34 +150,54 @@ static __init int test_heap_pop_push(bool min_heap)
 		temp = get_random_int();
 		min_heap_pop_push(&heap, &temp, &funcs);
 	}
-	err += pop_verify_heap(min_heap, &heap, &funcs);
+	pop_verify_heap(context, min_heap, &heap, &funcs);
+}

-	return err;
+static void __init test_heapify_all_true(struct kunit *context)
+{
+	test_heapify_all(context, true);
 }

-static int __init test_min_heap_init(void)
+static void __init test_heapify_all_false(struct kunit *context)
 {
-	int err = 0;
-
-	err += test_heapify_all(true);
-	err += test_heapify_all(false);
-	err += test_heap_push(true);
-	err += test_heap_push(false);
-	err += test_heap_pop_push(true);
-	err += test_heap_pop_push(false);
-	if (err) {
-		pr_err("test failed with %d errors\n", err);
-		return -EINVAL;
-	}
-	pr_info("test passed\n");
-	return 0;
+	test_heapify_all(context, true);
+}
+
+static void __init test_heap_push_true(struct kunit *context)
+{
+	test_heap_push(context, true);
+}
+
+static void __init test_heap_push_false(struct kunit *context)
+{
+	test_heap_push(context, false);
 }
-module_init(test_min_heap_init);

-static void __exit test_min_heap_exit(void)
+static void __init test_heap_pop_push_true(struct kunit *context)
 {
-	/* do nothing */
+	test_heap_pop_push(context, true);
 }
-module_exit(test_min_heap_exit);
+
+static void __init test_heap_pop_push_false(struct kunit *context)
+{
+	test_heap_pop_push(context, false);
+}
+
+static struct kunit_case __refdata min_heap_test_cases[] = {
+	KUNIT_CASE(test_heapify_all_true),
+	KUNIT_CASE(test_heapify_all_false),
+	KUNIT_CASE(test_heap_push_true),
+	KUNIT_CASE(test_heap_push_false),
+	KUNIT_CASE(test_heap_pop_push_true),
+	KUNIT_CASE(test_heap_pop_push_false),
+	{}
+};
+
+static struct kunit_suite min_heap_test_suite = {
+	.name = "min-heap",
+	.test_cases = min_heap_test_cases,
+};
+
+kunit_test_suites(&min_heap_test_suite);

 MODULE_LICENSE("GPL");

base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
--
2.26.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-07-29 20:11 [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit Vitor Massaru Iha
@ 2020-07-29 20:39 ` peterz
  2020-07-29 21:57   ` Vitor Massaru Iha
  2020-07-29 22:56 ` Ian Rogers via Linux-kernel-mentees
  1 sibling, 1 reply; 16+ messages in thread
From: peterz @ 2020-07-29 20:39 UTC (permalink / raw)
  To: Vitor Massaru Iha
  Cc: irogers, brendanhiggins, linux-kernel, linux-kselftest,
	linux-kernel-mentees, mingo, kunit-dev

On Wed, Jul 29, 2020 at 05:11:46PM -0300, Vitor Massaru Iha wrote:
> This adds the conversion of the runtime tests of test_min_heap,
> from `lib/test_min_heap.c` to KUnit tests.
> 
> Please apply this commit first (linux-kselftest/kunit-fixes):
> 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
> 
> Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> ---
>  lib/Kconfig.debug                         |  29 ++++--
>  lib/Makefile                              |   2 +-
>  lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
>  3 files changed, 83 insertions(+), 65 deletions(-)
>  rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)

So where's the win? What's KUnit, why should I care and more lines.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-07-29 20:39 ` peterz
@ 2020-07-29 21:57   ` Vitor Massaru Iha
  2020-08-04 13:25     ` peterz
  0 siblings, 1 reply; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-07-29 21:57 UTC (permalink / raw)
  To: peterz
  Cc: irogers, Brendan Higgins, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees, mingo,
	KUnit Development

Hi Peter,

On Wed, Jul 29, 2020 at 5:39 PM <peterz@infradead.org> wrote:
>
> On Wed, Jul 29, 2020 at 05:11:46PM -0300, Vitor Massaru Iha wrote:
> > This adds the conversion of the runtime tests of test_min_heap,
> > from `lib/test_min_heap.c` to KUnit tests.
> >
> > Please apply this commit first (linux-kselftest/kunit-fixes):
> > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
> >
> > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > ---
> >  lib/Kconfig.debug                         |  29 ++++--
> >  lib/Makefile                              |   2 +-
> >  lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
> >  3 files changed, 83 insertions(+), 65 deletions(-)
> >  rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)
>
> So where's the win? What's KUnit, why should I care and more lines.

KUnit is a unit testing and mocking framework for the Linux kernel. [0]

In Kconfig.debug you only have some more information about KUnit.

If the number of lines is a parameter that should be considered, I can
change sections like this

-                       if (last > values[0]) {
-                               pr_err("error: expected %d <= %d\n", last,
-                                       values[0]);
-                               err++;
-                       }
+                       KUNIT_EXPECT_FALSE_MSG(context,
+                                              last > values[0],
+                                              "expected %d <= %d\n",
+                                              last, values[0]);

To this:

-                       if (last > values[0]) {
-                               pr_err("error: expected %d <= %d\n", last,
-                                       values[0]);
-                               err++;
-                       }
+                       KUNIT_EXPECT_FALSE_MSG(context, last >
values[0],  "expected %d <= %d\n",  last, values[0]);

And from this:

+static struct kunit_case __refdata min_heap_test_cases[] = {
+       KUNIT_CASE(test_heapify_all_true),
+       KUNIT_CASE(test_heapify_all_false),
+       KUNIT_CASE(test_heap_push_true),
+       KUNIT_CASE(test_heap_push_false),
+       KUNIT_CASE(test_heap_pop_push_true),
+       KUNIT_CASE(test_heap_pop_push_false),
+       {}

To this:

+static struct kunit_case __refdata min_heap_test_cases[] = {
+       KUNIT_CASE(test_min_heap),
+       {}

I did the latter this way to be more informative, but if the goal is
to reduce lines of code, this is possible.

The results can be seen this way:

This is an excerpt from the test.log with the result in TAP format:
[snip]
ok 5 - example
    # Subtest: min-heap
    1..6
    ok 1 - test_heapify_all_true
    ok 2 - test_heapify_all_false
    ok 3 - test_heap_push_true
    ok 4 - test_heap_push_false
    ok 5 - test_heap_pop_push_true
    ok 6 - test_heap_pop_push_false
[snip]

And this from kunit-tool:
[snip]
[18:43:32] ============================================================
[18:43:32] ======== [PASSED] min-heap ========
[18:43:32] [PASSED] test_heapify_all_true
[18:43:32] [PASSED] test_heapify_all_false
[18:43:32] [PASSED] test_heap_push_true
[18:43:32] [PASSED] test_heap_push_false
[18:43:32] [PASSED] test_heap_pop_push_true
[18:43:32] [PASSED] test_heap_pop_push_false
[18:43:32] ============================================================
[18:43:32] Testing complete. 20 tests run. 0 failed. 0 crashed.
[18:43:32] Elapsed time: 9.758s total, 0.001s configuring, 6.012s
building, 0.000s running
[snip]

BR,
Vitor

[0] https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html#what-is-kunit
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-07-29 20:11 [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit Vitor Massaru Iha
  2020-07-29 20:39 ` peterz
@ 2020-07-29 22:56 ` Ian Rogers via Linux-kernel-mentees
  2020-07-29 23:12   ` Vitor Massaru Iha
  2020-10-14 23:49   ` Vitor Massaru Iha
  1 sibling, 2 replies; 16+ messages in thread
From: Ian Rogers via Linux-kernel-mentees @ 2020-07-29 22:56 UTC (permalink / raw)
  To: Vitor Massaru Iha
  Cc: Peter Zijlstra, brendanhiggins, LKML, linux-kselftest,
	linux-kernel-mentees, Ingo Molnar, kunit-dev

On Wed, Jul 29, 2020 at 1:11 PM Vitor Massaru Iha <vitor@massaru.org> wrote:
>
> This adds the conversion of the runtime tests of test_min_heap,
> from `lib/test_min_heap.c` to KUnit tests.
>
> Please apply this commit first (linux-kselftest/kunit-fixes):
> 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space

Thanks for this, I'm a fan of testing frameworks :-)

> Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> ---
>  lib/Kconfig.debug                         |  29 ++++--
>  lib/Makefile                              |   2 +-
>  lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
>  3 files changed, 83 insertions(+), 65 deletions(-)
>  rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 9ad9210d70a1..46674fc4972c 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1864,16 +1864,6 @@ config TEST_LIST_SORT
>
>           If unsure, say N.
>
> -config TEST_MIN_HEAP
> -       tristate "Min heap test"
> -       depends on DEBUG_KERNEL || m
> -       help
> -         Enable this to turn on min heap function tests. This test is
> -         executed only once during system boot (so affects only boot time),
> -         or at module load time.
> -
> -         If unsure, say N.
> -
>  config TEST_SORT
>         tristate "Array-based sort test"
>         depends on DEBUG_KERNEL || m
> @@ -2185,6 +2175,25 @@ config LINEAR_RANGES_TEST
>
>           If unsure, say N.
>
> +config MIN_HEAP_KUNIT
> +        tristate "KUnit test for Min heap"
> +        depends on KUNIT
> +        depends on DEBUG_KERNEL || m
> +        help
> +          Enable this to turn on min heap function tests. This test is
> +          executed only once during system boot (so affects only boot time),
> +          or at module load time.
> +
> +          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 the KUnit test harness, and not intended 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.
> +

It's a shame we need a config option for this. Could we have one
option to cover all basic library tests?

>  config TEST_UDELAY
>         tristate "udelay test driver"
>         help
> diff --git a/lib/Makefile b/lib/Makefile
> index b1c42c10073b..748f57063160 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -72,7 +72,6 @@ CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
>  UBSAN_SANITIZE_test_ubsan.o := y
>  obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
>  obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> -obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
>  obj-$(CONFIG_TEST_LKM) += test_module.o
>  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
>  obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
> @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
>  # KUnit tests
>  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
>  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> +obj-$(CONFIG_MIN_HEAP_KUNIT) += min_heap_kunit.o
> diff --git a/lib/test_min_heap.c b/lib/min_heap_kunit.c
> similarity index 60%
> rename from lib/test_min_heap.c
> rename to lib/min_heap_kunit.c
> index d19c8080fd4d..398db1c63146 100644
> --- a/lib/test_min_heap.c
> +++ b/lib/min_heap_kunit.c
> @@ -7,9 +7,8 @@
>
>  #include <linux/log2.h>
>  #include <linux/min_heap.h>
> -#include <linux/module.h>
> -#include <linux/printk.h>
>  #include <linux/random.h>
> +#include <kunit/test.h>
>
>  static __init bool less_than(const void *lhs, const void *rhs)
>  {
> @@ -29,37 +28,34 @@ static __init void swap_ints(void *lhs, void *rhs)
>         *(int *)rhs = temp;
>  }
>
> -static __init int pop_verify_heap(bool min_heap,
> +static __init void pop_verify_heap(struct kunit *context,
> +                               bool min_heap,
>                                 struct min_heap *heap,
>                                 const struct min_heap_callbacks *funcs)
>  {
>         int *values = heap->data;
> -       int err = 0;
>         int last;
>
>         last = values[0];
>         min_heap_pop(heap, funcs);
>         while (heap->nr > 0) {
>                 if (min_heap) {
> -                       if (last > values[0]) {
> -                               pr_err("error: expected %d <= %d\n", last,
> -                                       values[0]);
> -                               err++;
> -                       }
> +                       KUNIT_EXPECT_FALSE_MSG(context,
> +                                              last > values[0],
> +                                              "expected %d <= %d\n",
> +                                              last, values[0]);

I'm not familiar with kunit, is there a reason not to prefer:
KUNIT_EXPECT_LT(context, last, values[0]);

>                 } else {
> -                       if (last < values[0]) {
> -                               pr_err("error: expected %d >= %d\n", last,
> -                                       values[0]);
> -                               err++;
> -                       }
> +                       KUNIT_EXPECT_FALSE_MSG(context,
> +                                              last < values[0],
> +                                              "expected %d >= %d\n",
> +                                              last, values[0]);

Similarly KUNIT_EXPECT_GT.

Thanks,
Ian

>                 }
>                 last = values[0];
>                 min_heap_pop(heap, funcs);
>         }
> -       return err;
>  }
>
> -static __init int test_heapify_all(bool min_heap)
> +static __init void test_heapify_all(struct kunit *context, bool min_heap)
>  {
>         int values[] = { 3, 1, 2, 4, 0x8000000, 0x7FFFFFF, 0,
>                          -3, -1, -2, -4, 0x8000000, 0x7FFFFFF };
> @@ -73,12 +69,11 @@ static __init int test_heapify_all(bool min_heap)
>                 .less = min_heap ? less_than : greater_than,
>                 .swp = swap_ints,
>         };
> -       int i, err;
> +       int i;
>
>         /* Test with known set of values. */
>         min_heapify_all(&heap, &funcs);
> -       err = pop_verify_heap(min_heap, &heap, &funcs);
> -
> +       pop_verify_heap(context, min_heap, &heap, &funcs);
>
>         /* Test with randomly generated values. */
>         heap.nr = ARRAY_SIZE(values);
> @@ -86,12 +81,10 @@ static __init int test_heapify_all(bool min_heap)
>                 values[i] = get_random_int();
>
>         min_heapify_all(&heap, &funcs);
> -       err += pop_verify_heap(min_heap, &heap, &funcs);
> -
> -       return err;
> +       pop_verify_heap(context, min_heap, &heap, &funcs);
>  }
>
> -static __init int test_heap_push(bool min_heap)
> +static __init void test_heap_push(struct kunit *context, bool min_heap)
>  {
>         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
>                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> @@ -106,25 +99,22 @@ static __init int test_heap_push(bool min_heap)
>                 .less = min_heap ? less_than : greater_than,
>                 .swp = swap_ints,
>         };
> -       int i, temp, err;
> +       int i, temp;
>
>         /* Test with known set of values copied from data. */
>         for (i = 0; i < ARRAY_SIZE(data); i++)
>                 min_heap_push(&heap, &data[i], &funcs);
> -
> -       err = pop_verify_heap(min_heap, &heap, &funcs);
> +       pop_verify_heap(context, min_heap, &heap, &funcs);
>
>         /* Test with randomly generated values. */
>         while (heap.nr < heap.size) {
>                 temp = get_random_int();
>                 min_heap_push(&heap, &temp, &funcs);
>         }
> -       err += pop_verify_heap(min_heap, &heap, &funcs);
> -
> -       return err;
> +       pop_verify_heap(context, min_heap, &heap, &funcs);
>  }
>
> -static __init int test_heap_pop_push(bool min_heap)
> +static __init void test_heap_pop_push(struct kunit *context, bool min_heap)
>  {
>         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
>                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> @@ -139,7 +129,7 @@ static __init int test_heap_pop_push(bool min_heap)
>                 .less = min_heap ? less_than : greater_than,
>                 .swp = swap_ints,
>         };
> -       int i, temp, err;
> +       int i, temp;
>
>         /* Fill values with data to pop and replace. */
>         temp = min_heap ? 0x80000000 : 0x7FFFFFFF;
> @@ -149,8 +139,7 @@ static __init int test_heap_pop_push(bool min_heap)
>         /* Test with known set of values copied from data. */
>         for (i = 0; i < ARRAY_SIZE(data); i++)
>                 min_heap_pop_push(&heap, &data[i], &funcs);
> -
> -       err = pop_verify_heap(min_heap, &heap, &funcs);
> +       pop_verify_heap(context, min_heap, &heap, &funcs);
>
>         heap.nr = 0;
>         for (i = 0; i < ARRAY_SIZE(data); i++)
> @@ -161,34 +150,54 @@ static __init int test_heap_pop_push(bool min_heap)
>                 temp = get_random_int();
>                 min_heap_pop_push(&heap, &temp, &funcs);
>         }
> -       err += pop_verify_heap(min_heap, &heap, &funcs);
> +       pop_verify_heap(context, min_heap, &heap, &funcs);
> +}
>
> -       return err;
> +static void __init test_heapify_all_true(struct kunit *context)
> +{
> +       test_heapify_all(context, true);
>  }
>
> -static int __init test_min_heap_init(void)
> +static void __init test_heapify_all_false(struct kunit *context)
>  {
> -       int err = 0;
> -
> -       err += test_heapify_all(true);
> -       err += test_heapify_all(false);
> -       err += test_heap_push(true);
> -       err += test_heap_push(false);
> -       err += test_heap_pop_push(true);
> -       err += test_heap_pop_push(false);
> -       if (err) {
> -               pr_err("test failed with %d errors\n", err);
> -               return -EINVAL;
> -       }
> -       pr_info("test passed\n");
> -       return 0;
> +       test_heapify_all(context, true);
> +}
> +
> +static void __init test_heap_push_true(struct kunit *context)
> +{
> +       test_heap_push(context, true);
> +}
> +
> +static void __init test_heap_push_false(struct kunit *context)
> +{
> +       test_heap_push(context, false);
>  }
> -module_init(test_min_heap_init);
>
> -static void __exit test_min_heap_exit(void)
> +static void __init test_heap_pop_push_true(struct kunit *context)
>  {
> -       /* do nothing */
> +       test_heap_pop_push(context, true);
>  }
> -module_exit(test_min_heap_exit);
> +
> +static void __init test_heap_pop_push_false(struct kunit *context)
> +{
> +       test_heap_pop_push(context, false);
> +}
> +
> +static struct kunit_case __refdata min_heap_test_cases[] = {
> +       KUNIT_CASE(test_heapify_all_true),
> +       KUNIT_CASE(test_heapify_all_false),
> +       KUNIT_CASE(test_heap_push_true),
> +       KUNIT_CASE(test_heap_push_false),
> +       KUNIT_CASE(test_heap_pop_push_true),
> +       KUNIT_CASE(test_heap_pop_push_false),
> +       {}
> +};
> +
> +static struct kunit_suite min_heap_test_suite = {
> +       .name = "min-heap",
> +       .test_cases = min_heap_test_cases,
> +};
> +
> +kunit_test_suites(&min_heap_test_suite);
>
>  MODULE_LICENSE("GPL");
>
> base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
> --
> 2.26.2
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-07-29 22:56 ` Ian Rogers via Linux-kernel-mentees
@ 2020-07-29 23:12   ` Vitor Massaru Iha
  2020-10-14 23:49   ` Vitor Massaru Iha
  1 sibling, 0 replies; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-07-29 23:12 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Brendan Higgins, LKML, Ingo Molnar,
	linux-kselftest, linux-kernel-mentees, KUnit Development

Hi Ian,

On Wed, Jul 29, 2020 at 7:57 PM 'Ian Rogers' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> On Wed, Jul 29, 2020 at 1:11 PM Vitor Massaru Iha <vitor@massaru.org> wrote:
> >
> > This adds the conversion of the runtime tests of test_min_heap,
> > from `lib/test_min_heap.c` to KUnit tests.
> >
> > Please apply this commit first (linux-kselftest/kunit-fixes):
> > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
>
> Thanks for this, I'm a fan of testing frameworks :-)
>
> > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > ---
> >  lib/Kconfig.debug                         |  29 ++++--
> >  lib/Makefile                              |   2 +-
> >  lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
> >  3 files changed, 83 insertions(+), 65 deletions(-)
> >  rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 9ad9210d70a1..46674fc4972c 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1864,16 +1864,6 @@ config TEST_LIST_SORT
> >
> >           If unsure, say N.
> >
> > -config TEST_MIN_HEAP
> > -       tristate "Min heap test"
> > -       depends on DEBUG_KERNEL || m
> > -       help
> > -         Enable this to turn on min heap function tests. This test is
> > -         executed only once during system boot (so affects only boot time),
> > -         or at module load time.
> > -
> > -         If unsure, say N.
> > -
> >  config TEST_SORT
> >         tristate "Array-based sort test"
> >         depends on DEBUG_KERNEL || m
> > @@ -2185,6 +2175,25 @@ config LINEAR_RANGES_TEST
> >
> >           If unsure, say N.
> >
> > +config MIN_HEAP_KUNIT
> > +        tristate "KUnit test for Min heap"
> > +        depends on KUNIT
> > +        depends on DEBUG_KERNEL || m
> > +        help
> > +          Enable this to turn on min heap function tests. This test is
> > +          executed only once during system boot (so affects only boot time),
> > +          or at module load time.
> > +
> > +          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 the KUnit test harness, and not intended 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.
> > +
>
> It's a shame we need a config option for this. Could we have one
> option to cover all basic library tests?

Sure, I think so.

>
> >  config TEST_UDELAY
> >         tristate "udelay test driver"
> >         help
> > diff --git a/lib/Makefile b/lib/Makefile
> > index b1c42c10073b..748f57063160 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -72,7 +72,6 @@ CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
> >  UBSAN_SANITIZE_test_ubsan.o := y
> >  obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
> >  obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> > -obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
> >  obj-$(CONFIG_TEST_LKM) += test_module.o
> >  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
> >  obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
> > @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
> >  # KUnit tests
> >  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> >  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> > +obj-$(CONFIG_MIN_HEAP_KUNIT) += min_heap_kunit.o
> > diff --git a/lib/test_min_heap.c b/lib/min_heap_kunit.c
> > similarity index 60%
> > rename from lib/test_min_heap.c
> > rename to lib/min_heap_kunit.c
> > index d19c8080fd4d..398db1c63146 100644
> > --- a/lib/test_min_heap.c
> > +++ b/lib/min_heap_kunit.c
> > @@ -7,9 +7,8 @@
> >
> >  #include <linux/log2.h>
> >  #include <linux/min_heap.h>
> > -#include <linux/module.h>
> > -#include <linux/printk.h>
> >  #include <linux/random.h>
> > +#include <kunit/test.h>
> >
> >  static __init bool less_than(const void *lhs, const void *rhs)
> >  {
> > @@ -29,37 +28,34 @@ static __init void swap_ints(void *lhs, void *rhs)
> >         *(int *)rhs = temp;
> >  }
> >
> > -static __init int pop_verify_heap(bool min_heap,
> > +static __init void pop_verify_heap(struct kunit *context,
> > +                               bool min_heap,
> >                                 struct min_heap *heap,
> >                                 const struct min_heap_callbacks *funcs)
> >  {
> >         int *values = heap->data;
> > -       int err = 0;
> >         int last;
> >
> >         last = values[0];
> >         min_heap_pop(heap, funcs);
> >         while (heap->nr > 0) {
> >                 if (min_heap) {
> > -                       if (last > values[0]) {
> > -                               pr_err("error: expected %d <= %d\n", last,
> > -                                       values[0]);
> > -                               err++;
> > -                       }
> > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > +                                              last > values[0],
> > +                                              "expected %d <= %d\n",
> > +                                              last, values[0]);
>
> I'm not familiar with kunit, is there a reason not to prefer:
> KUNIT_EXPECT_LT(context, last, values[0]);

Nope. I will fix this and the other one that you pointed down below.

Thanks!

>
> >                 } else {
> > -                       if (last < values[0]) {
> > -                               pr_err("error: expected %d >= %d\n", last,
> > -                                       values[0]);
> > -                               err++;
> > -                       }
> > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > +                                              last < values[0],
> > +                                              "expected %d >= %d\n",
> > +                                              last, values[0]);
>
> Similarly KUNIT_EXPECT_GT.
>
> Thanks,
> Ian
>
> >                 }
> >                 last = values[0];
> >                 min_heap_pop(heap, funcs);
> >         }
> > -       return err;
> >  }
> >
> > -static __init int test_heapify_all(bool min_heap)
> > +static __init void test_heapify_all(struct kunit *context, bool min_heap)
> >  {
> >         int values[] = { 3, 1, 2, 4, 0x8000000, 0x7FFFFFF, 0,
> >                          -3, -1, -2, -4, 0x8000000, 0x7FFFFFF };
> > @@ -73,12 +69,11 @@ static __init int test_heapify_all(bool min_heap)
> >                 .less = min_heap ? less_than : greater_than,
> >                 .swp = swap_ints,
> >         };
> > -       int i, err;
> > +       int i;
> >
> >         /* Test with known set of values. */
> >         min_heapify_all(&heap, &funcs);
> > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > -
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >
> >         /* Test with randomly generated values. */
> >         heap.nr = ARRAY_SIZE(values);
> > @@ -86,12 +81,10 @@ static __init int test_heapify_all(bool min_heap)
> >                 values[i] = get_random_int();
> >
> >         min_heapify_all(&heap, &funcs);
> > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > -
> > -       return err;
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >  }
> >
> > -static __init int test_heap_push(bool min_heap)
> > +static __init void test_heap_push(struct kunit *context, bool min_heap)
> >  {
> >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > @@ -106,25 +99,22 @@ static __init int test_heap_push(bool min_heap)
> >                 .less = min_heap ? less_than : greater_than,
> >                 .swp = swap_ints,
> >         };
> > -       int i, temp, err;
> > +       int i, temp;
> >
> >         /* Test with known set of values copied from data. */
> >         for (i = 0; i < ARRAY_SIZE(data); i++)
> >                 min_heap_push(&heap, &data[i], &funcs);
> > -
> > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >
> >         /* Test with randomly generated values. */
> >         while (heap.nr < heap.size) {
> >                 temp = get_random_int();
> >                 min_heap_push(&heap, &temp, &funcs);
> >         }
> > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > -
> > -       return err;
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >  }
> >
> > -static __init int test_heap_pop_push(bool min_heap)
> > +static __init void test_heap_pop_push(struct kunit *context, bool min_heap)
> >  {
> >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > @@ -139,7 +129,7 @@ static __init int test_heap_pop_push(bool min_heap)
> >                 .less = min_heap ? less_than : greater_than,
> >                 .swp = swap_ints,
> >         };
> > -       int i, temp, err;
> > +       int i, temp;
> >
> >         /* Fill values with data to pop and replace. */
> >         temp = min_heap ? 0x80000000 : 0x7FFFFFFF;
> > @@ -149,8 +139,7 @@ static __init int test_heap_pop_push(bool min_heap)
> >         /* Test with known set of values copied from data. */
> >         for (i = 0; i < ARRAY_SIZE(data); i++)
> >                 min_heap_pop_push(&heap, &data[i], &funcs);
> > -
> > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >
> >         heap.nr = 0;
> >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > @@ -161,34 +150,54 @@ static __init int test_heap_pop_push(bool min_heap)
> >                 temp = get_random_int();
> >                 min_heap_pop_push(&heap, &temp, &funcs);
> >         }
> > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > +}
> >
> > -       return err;
> > +static void __init test_heapify_all_true(struct kunit *context)
> > +{
> > +       test_heapify_all(context, true);
> >  }
> >
> > -static int __init test_min_heap_init(void)
> > +static void __init test_heapify_all_false(struct kunit *context)
> >  {
> > -       int err = 0;
> > -
> > -       err += test_heapify_all(true);
> > -       err += test_heapify_all(false);
> > -       err += test_heap_push(true);
> > -       err += test_heap_push(false);
> > -       err += test_heap_pop_push(true);
> > -       err += test_heap_pop_push(false);
> > -       if (err) {
> > -               pr_err("test failed with %d errors\n", err);
> > -               return -EINVAL;
> > -       }
> > -       pr_info("test passed\n");
> > -       return 0;
> > +       test_heapify_all(context, true);
> > +}
> > +
> > +static void __init test_heap_push_true(struct kunit *context)
> > +{
> > +       test_heap_push(context, true);
> > +}
> > +
> > +static void __init test_heap_push_false(struct kunit *context)
> > +{
> > +       test_heap_push(context, false);
> >  }
> > -module_init(test_min_heap_init);
> >
> > -static void __exit test_min_heap_exit(void)
> > +static void __init test_heap_pop_push_true(struct kunit *context)
> >  {
> > -       /* do nothing */
> > +       test_heap_pop_push(context, true);
> >  }
> > -module_exit(test_min_heap_exit);
> > +
> > +static void __init test_heap_pop_push_false(struct kunit *context)
> > +{
> > +       test_heap_pop_push(context, false);
> > +}
> > +
> > +static struct kunit_case __refdata min_heap_test_cases[] = {
> > +       KUNIT_CASE(test_heapify_all_true),
> > +       KUNIT_CASE(test_heapify_all_false),
> > +       KUNIT_CASE(test_heap_push_true),
> > +       KUNIT_CASE(test_heap_push_false),
> > +       KUNIT_CASE(test_heap_pop_push_true),
> > +       KUNIT_CASE(test_heap_pop_push_false),
> > +       {}
> > +};
> > +
> > +static struct kunit_suite min_heap_test_suite = {
> > +       .name = "min-heap",
> > +       .test_cases = min_heap_test_cases,
> > +};
> > +
> > +kunit_test_suites(&min_heap_test_suite);
> >
> >  MODULE_LICENSE("GPL");
> >
> > base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
> > --
> > 2.26.2
> >
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/CAP-5%3DfWCMPyBYqn4p%2B_%3DRa5_sXqUbr4h_OCuYS4iY-6fsnevvA%40mail.gmail.com.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-07-29 21:57   ` Vitor Massaru Iha
@ 2020-08-04 13:25     ` peterz
  2020-08-04 13:46       ` Vitor Massaru Iha
  0 siblings, 1 reply; 16+ messages in thread
From: peterz @ 2020-08-04 13:25 UTC (permalink / raw)
  To: Vitor Massaru Iha
  Cc: irogers, Brendan Higgins, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees, mingo,
	KUnit Development

On Wed, Jul 29, 2020 at 06:57:17PM -0300, Vitor Massaru Iha wrote:

> The results can be seen this way:
> 
> This is an excerpt from the test.log with the result in TAP format:
> [snip]
> ok 5 - example
>     # Subtest: min-heap
>     1..6
>     ok 1 - test_heapify_all_true
>     ok 2 - test_heapify_all_false
>     ok 3 - test_heap_push_true
>     ok 4 - test_heap_push_false
>     ok 5 - test_heap_pop_push_true
>     ok 6 - test_heap_pop_push_false
> [snip]
> 
> And this from kunit-tool:
> [snip]
> [18:43:32] ============================================================
> [18:43:32] ======== [PASSED] min-heap ========
> [18:43:32] [PASSED] test_heapify_all_true
> [18:43:32] [PASSED] test_heapify_all_false
> [18:43:32] [PASSED] test_heap_push_true
> [18:43:32] [PASSED] test_heap_push_false
> [18:43:32] [PASSED] test_heap_pop_push_true
> [18:43:32] [PASSED] test_heap_pop_push_false
> [18:43:32] ============================================================
> [18:43:32] Testing complete. 20 tests run. 0 failed. 0 crashed.
> [18:43:32] Elapsed time: 9.758s total, 0.001s configuring, 6.012s
> building, 0.000s running
> [snip]

I don't care or care to use either; what does dmesg do? It used to be
that just building the self-tests was sufficient and any error would
show in dmesg when you boot the machine.

But if I now have to use some damn tool, this is a regression.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-08-04 13:25     ` peterz
@ 2020-08-04 13:46       ` Vitor Massaru Iha
  2020-08-04 14:23         ` peterz
  0 siblings, 1 reply; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-08-04 13:46 UTC (permalink / raw)
  To: peterz
  Cc: Ian Rogers, Brendan Higgins, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees, mingo,
	KUnit Development

Hi Peter,

On Tue, Aug 4, 2020 at 10:25 AM <peterz@infradead.org> wrote:
>
> On Wed, Jul 29, 2020 at 06:57:17PM -0300, Vitor Massaru Iha wrote:
>
> > The results can be seen this way:
> >
> > This is an excerpt from the test.log with the result in TAP format:
> > [snip]
> > ok 5 - example
> >     # Subtest: min-heap
> >     1..6
> >     ok 1 - test_heapify_all_true
> >     ok 2 - test_heapify_all_false
> >     ok 3 - test_heap_push_true
> >     ok 4 - test_heap_push_false
> >     ok 5 - test_heap_pop_push_true
> >     ok 6 - test_heap_pop_push_false
> > [snip]
> >
> > And this from kunit-tool:
> > [snip]
> > [18:43:32] ============================================================
> > [18:43:32] ======== [PASSED] min-heap ========
> > [18:43:32] [PASSED] test_heapify_all_true
> > [18:43:32] [PASSED] test_heapify_all_false
> > [18:43:32] [PASSED] test_heap_push_true
> > [18:43:32] [PASSED] test_heap_push_false
> > [18:43:32] [PASSED] test_heap_pop_push_true
> > [18:43:32] [PASSED] test_heap_pop_push_false
> > [18:43:32] ============================================================
> > [18:43:32] Testing complete. 20 tests run. 0 failed. 0 crashed.
> > [18:43:32] Elapsed time: 9.758s total, 0.001s configuring, 6.012s
> > building, 0.000s running
> > [snip]
>
> I don't care or care to use either; what does dmesg do? It used to be
> that just building the self-tests was sufficient and any error would
> show in dmesg when you boot the machine.
>
> But if I now have to use some damn tool, this is a regression.

If you don't want to, you don't need to use the kunit-tool. If you
compile the tests as builtin and run the Kernel on your machine
the test result will be shown in dmesg in TAP format.

BR,
Vitor
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-08-04 13:46       ` Vitor Massaru Iha
@ 2020-08-04 14:23         ` peterz
  2020-08-04 16:22           ` Vitor Massaru Iha
  0 siblings, 1 reply; 16+ messages in thread
From: peterz @ 2020-08-04 14:23 UTC (permalink / raw)
  To: Vitor Massaru Iha
  Cc: Ian Rogers, Brendan Higgins, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees, mingo,
	KUnit Development

On Tue, Aug 04, 2020 at 10:46:21AM -0300, Vitor Massaru Iha wrote:
> On Tue, Aug 4, 2020 at 10:25 AM <peterz@infradead.org> wrote:
> > On Wed, Jul 29, 2020 at 06:57:17PM -0300, Vitor Massaru Iha wrote:
> >
> > > The results can be seen this way:
> > >
> > > This is an excerpt from the test.log with the result in TAP format:
> > > [snip]
> > > ok 5 - example
> > >     # Subtest: min-heap
> > >     1..6
> > >     ok 1 - test_heapify_all_true
> > >     ok 2 - test_heapify_all_false
> > >     ok 3 - test_heap_push_true
> > >     ok 4 - test_heap_push_false
> > >     ok 5 - test_heap_pop_push_true
> > >     ok 6 - test_heap_pop_push_false
> > > [snip]

So ^ is TAP format?

> > I don't care or care to use either; what does dmesg do? It used to be
> > that just building the self-tests was sufficient and any error would
> > show in dmesg when you boot the machine.
> >
> > But if I now have to use some damn tool, this is a regression.
> 
> If you don't want to, you don't need to use the kunit-tool. If you
> compile the tests as builtin and run the Kernel on your machine
> the test result will be shown in dmesg in TAP format.

That's seems a lot more verbose than it is now. I've recently even done
a bunch of tests that don't print anything on success, dmesg is clutter
enough already.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-08-04 14:23         ` peterz
@ 2020-08-04 16:22           ` Vitor Massaru Iha
  2020-10-12 21:02             ` Brendan Higgins via Linux-kernel-mentees
  0 siblings, 1 reply; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-08-04 16:22 UTC (permalink / raw)
  To: peterz
  Cc: Ian Rogers, Brendan Higgins, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees, mingo,
	KUnit Development

Hi Peter,

On Tue, Aug 4, 2020 at 11:23 AM <peterz@infradead.org> wrote:
>
> On Tue, Aug 04, 2020 at 10:46:21AM -0300, Vitor Massaru Iha wrote:
> > On Tue, Aug 4, 2020 at 10:25 AM <peterz@infradead.org> wrote:
> > > On Wed, Jul 29, 2020 at 06:57:17PM -0300, Vitor Massaru Iha wrote:
> > >
> > > > The results can be seen this way:
> > > >
> > > > This is an excerpt from the test.log with the result in TAP format:
> > > > [snip]
> > > > ok 5 - example
> > > >     # Subtest: min-heap
> > > >     1..6
> > > >     ok 1 - test_heapify_all_true
> > > >     ok 2 - test_heapify_all_false
> > > >     ok 3 - test_heap_push_true
> > > >     ok 4 - test_heap_push_false
> > > >     ok 5 - test_heap_pop_push_true
> > > >     ok 6 - test_heap_pop_push_false
> > > > [snip]
>
> So ^ is TAP format?

Yep, you can see the spec here: https://testanything.org/tap-specification.html

>
> > > I don't care or care to use either; what does dmesg do? It used to be
> > > that just building the self-tests was sufficient and any error would
> > > show in dmesg when you boot the machine.
> > >
> > > But if I now have to use some damn tool, this is a regression.
> >
> > If you don't want to, you don't need to use the kunit-tool. If you
> > compile the tests as builtin and run the Kernel on your machine
> > the test result will be shown in dmesg in TAP format.
>
> That's seems a lot more verbose than it is now. I've recently even done
> a bunch of tests that don't print anything on success, dmesg is clutter
> enough already.

What tests do you refer to?

Running the test_min_heap.c, I got this from dmesg:

min_heap_test: test passed

And running min_heap_kunit.c:

ok 1 - min-heap

BR,
Vitor
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-08-04 16:22           ` Vitor Massaru Iha
@ 2020-10-12 21:02             ` Brendan Higgins via Linux-kernel-mentees
  2020-10-14 18:16               ` Ian Rogers via Linux-kernel-mentees
  0 siblings, 1 reply; 16+ messages in thread
From: Brendan Higgins via Linux-kernel-mentees @ 2020-10-12 21:02 UTC (permalink / raw)
  To: Peter Zijlstra, Vitor Massaru Iha
  Cc: Ian Rogers, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	Ingo Molnar, KUnit Development

On Tue, Aug 4, 2020 at 9:22 AM Vitor Massaru Iha <vitor@massaru.org> wrote:
>
> Hi Peter,
>
> On Tue, Aug 4, 2020 at 11:23 AM <peterz@infradead.org> wrote:
> >
> > On Tue, Aug 04, 2020 at 10:46:21AM -0300, Vitor Massaru Iha wrote:
> > > On Tue, Aug 4, 2020 at 10:25 AM <peterz@infradead.org> wrote:
> > > > On Wed, Jul 29, 2020 at 06:57:17PM -0300, Vitor Massaru Iha wrote:
> > > >
> > > > > The results can be seen this way:
> > > > >
> > > > > This is an excerpt from the test.log with the result in TAP format:
> > > > > [snip]
> > > > > ok 5 - example
> > > > >     # Subtest: min-heap
> > > > >     1..6
> > > > >     ok 1 - test_heapify_all_true
> > > > >     ok 2 - test_heapify_all_false
> > > > >     ok 3 - test_heap_push_true
> > > > >     ok 4 - test_heap_push_false
> > > > >     ok 5 - test_heap_pop_push_true
> > > > >     ok 6 - test_heap_pop_push_false
> > > > > [snip]
> >
> > So ^ is TAP format?
>
> Yep, you can see the spec here: https://testanything.org/tap-specification.html
>
> >
> > > > I don't care or care to use either; what does dmesg do? It used to be
> > > > that just building the self-tests was sufficient and any error would
> > > > show in dmesg when you boot the machine.
> > > >
> > > > But if I now have to use some damn tool, this is a regression.
> > >
> > > If you don't want to, you don't need to use the kunit-tool. If you
> > > compile the tests as builtin and run the Kernel on your machine
> > > the test result will be shown in dmesg in TAP format.
> >
> > That's seems a lot more verbose than it is now. I've recently even done
> > a bunch of tests that don't print anything on success, dmesg is clutter
> > enough already.
>
> What tests do you refer to?
>
> Running the test_min_heap.c, I got this from dmesg:
>
> min_heap_test: test passed
>
> And running min_heap_kunit.c:
>
> ok 1 - min-heap

Gentle poke. I think Vitor was looking for a response. My guess is
that Ian was waiting for a follow up patch.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-10-12 21:02             ` Brendan Higgins via Linux-kernel-mentees
@ 2020-10-14 18:16               ` Ian Rogers via Linux-kernel-mentees
  2020-10-14 19:59                 ` Peter Zijlstra
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Rogers via Linux-kernel-mentees @ 2020-10-14 18:16 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Peter Zijlstra, Linux Kernel Mailing List, Ingo Molnar,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	KUnit Development

On Mon, Oct 12, 2020 at 2:03 PM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> On Tue, Aug 4, 2020 at 9:22 AM Vitor Massaru Iha <vitor@massaru.org> wrote:
> >
> > Hi Peter,
> >
> > On Tue, Aug 4, 2020 at 11:23 AM <peterz@infradead.org> wrote:
> > >
> > > On Tue, Aug 04, 2020 at 10:46:21AM -0300, Vitor Massaru Iha wrote:
> > > > On Tue, Aug 4, 2020 at 10:25 AM <peterz@infradead.org> wrote:
> > > > > On Wed, Jul 29, 2020 at 06:57:17PM -0300, Vitor Massaru Iha wrote:
> > > > >
> > > > > > The results can be seen this way:
> > > > > >
> > > > > > This is an excerpt from the test.log with the result in TAP format:
> > > > > > [snip]
> > > > > > ok 5 - example
> > > > > >     # Subtest: min-heap
> > > > > >     1..6
> > > > > >     ok 1 - test_heapify_all_true
> > > > > >     ok 2 - test_heapify_all_false
> > > > > >     ok 3 - test_heap_push_true
> > > > > >     ok 4 - test_heap_push_false
> > > > > >     ok 5 - test_heap_pop_push_true
> > > > > >     ok 6 - test_heap_pop_push_false
> > > > > > [snip]
> > >
> > > So ^ is TAP format?
> >
> > Yep, you can see the spec here: https://testanything.org/tap-specification.html
> >
> > >
> > > > > I don't care or care to use either; what does dmesg do? It used to be
> > > > > that just building the self-tests was sufficient and any error would
> > > > > show in dmesg when you boot the machine.
> > > > >
> > > > > But if I now have to use some damn tool, this is a regression.
> > > >
> > > > If you don't want to, you don't need to use the kunit-tool. If you
> > > > compile the tests as builtin and run the Kernel on your machine
> > > > the test result will be shown in dmesg in TAP format.
> > >
> > > That's seems a lot more verbose than it is now. I've recently even done
> > > a bunch of tests that don't print anything on success, dmesg is clutter
> > > enough already.
> >
> > What tests do you refer to?
> >
> > Running the test_min_heap.c, I got this from dmesg:
> >
> > min_heap_test: test passed
> >
> > And running min_heap_kunit.c:
> >
> > ok 1 - min-heap
>
> Gentle poke. I think Vitor was looking for a response. My guess is
> that Ian was waiting for a follow up patch.

There were some issues in the original patch, they should be easy to
fix. I'm more concerned that Peter's issues are addressed about the
general direction of the patch, verbosity and testing frameworks. I
see Vitor followed up with Peter but I'm not sure that means the
approach has been accepted.

Thanks,
Ian
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-10-14 18:16               ` Ian Rogers via Linux-kernel-mentees
@ 2020-10-14 19:59                 ` Peter Zijlstra
  2020-10-14 20:03                   ` Vitor Massaru Iha
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Zijlstra @ 2020-10-14 19:59 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Brendan Higgins, Linux Kernel Mailing List, Ingo Molnar,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	KUnit Development

On Wed, Oct 14, 2020 at 11:16:10AM -0700, Ian Rogers wrote:

> There were some issues in the original patch, they should be easy to
> fix. I'm more concerned that Peter's issues are addressed about the
> general direction of the patch, verbosity and testing frameworks. I
> see Vitor followed up with Peter but I'm not sure that means the
> approach has been accepted.

I kinda lost track, as long as it doesn't get more verbose I suppose I'm
fine.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-10-14 19:59                 ` Peter Zijlstra
@ 2020-10-14 20:03                   ` Vitor Massaru Iha
  0 siblings, 0 replies; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-10-14 20:03 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ian Rogers, Brendan Higgins, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	Ingo Molnar, KUnit Development

On Wed, Oct 14, 2020 at 5:00 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Wed, Oct 14, 2020 at 11:16:10AM -0700, Ian Rogers wrote:
>
> > There were some issues in the original patch, they should be easy to
> > fix. I'm more concerned that Peter's issues are addressed about the
> > general direction of the patch, verbosity and testing frameworks. I
> > see Vitor followed up with Peter but I'm not sure that means the
> > approach has been accepted.
>
> I kinda lost track, as long as it doesn't get more verbose I suppose I'm
> fine.


Ok. I'll send the v2 later with Ian's suggestions.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-07-29 22:56 ` Ian Rogers via Linux-kernel-mentees
  2020-07-29 23:12   ` Vitor Massaru Iha
@ 2020-10-14 23:49   ` Vitor Massaru Iha
  2020-10-15 16:30     ` Ian Rogers via Linux-kernel-mentees
  1 sibling, 1 reply; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-10-14 23:49 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Brendan Higgins, LKML,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	Ingo Molnar, KUnit Development

Hi Ian,


On Wed, Jul 29, 2020 at 7:57 PM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Jul 29, 2020 at 1:11 PM Vitor Massaru Iha <vitor@massaru.org> wrote:
> >
> > This adds the conversion of the runtime tests of test_min_heap,
> > from `lib/test_min_heap.c` to KUnit tests.
> >
> > Please apply this commit first (linux-kselftest/kunit-fixes):
> > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
>
> Thanks for this, I'm a fan of testing frameworks :-)
>
> > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > ---
> >  lib/Kconfig.debug                         |  29 ++++--
> >  lib/Makefile                              |   2 +-
> >  lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
> >  3 files changed, 83 insertions(+), 65 deletions(-)
> >  rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index 9ad9210d70a1..46674fc4972c 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1864,16 +1864,6 @@ config TEST_LIST_SORT
> >
> >           If unsure, say N.
> >
> > -config TEST_MIN_HEAP
> > -       tristate "Min heap test"
> > -       depends on DEBUG_KERNEL || m
> > -       help
> > -         Enable this to turn on min heap function tests. This test is
> > -         executed only once during system boot (so affects only boot time),
> > -         or at module load time.
> > -
> > -         If unsure, say N.
> > -
> >  config TEST_SORT
> >         tristate "Array-based sort test"
> >         depends on DEBUG_KERNEL || m
> > @@ -2185,6 +2175,25 @@ config LINEAR_RANGES_TEST
> >
> >           If unsure, say N.
> >
> > +config MIN_HEAP_KUNIT
> > +        tristate "KUnit test for Min heap"
> > +        depends on KUNIT
> > +        depends on DEBUG_KERNEL || m
> > +        help
> > +          Enable this to turn on min heap function tests. This test is
> > +          executed only once during system boot (so affects only boot time),
> > +          or at module load time.
> > +
> > +          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 the KUnit test harness, and not intended 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.
> > +
>
> It's a shame we need a config option for this. Could we have one
> option to cover all basic library tests?
>
> >  config TEST_UDELAY
> >         tristate "udelay test driver"
> >         help
> > diff --git a/lib/Makefile b/lib/Makefile
> > index b1c42c10073b..748f57063160 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -72,7 +72,6 @@ CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
> >  UBSAN_SANITIZE_test_ubsan.o := y
> >  obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
> >  obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> > -obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
> >  obj-$(CONFIG_TEST_LKM) += test_module.o
> >  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
> >  obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
> > @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
> >  # KUnit tests
> >  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> >  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> > +obj-$(CONFIG_MIN_HEAP_KUNIT) += min_heap_kunit.o
> > diff --git a/lib/test_min_heap.c b/lib/min_heap_kunit.c
> > similarity index 60%
> > rename from lib/test_min_heap.c
> > rename to lib/min_heap_kunit.c
> > index d19c8080fd4d..398db1c63146 100644
> > --- a/lib/test_min_heap.c
> > +++ b/lib/min_heap_kunit.c
> > @@ -7,9 +7,8 @@
> >
> >  #include <linux/log2.h>
> >  #include <linux/min_heap.h>
> > -#include <linux/module.h>
> > -#include <linux/printk.h>
> >  #include <linux/random.h>
> > +#include <kunit/test.h>
> >
> >  static __init bool less_than(const void *lhs, const void *rhs)
> >  {
> > @@ -29,37 +28,34 @@ static __init void swap_ints(void *lhs, void *rhs)
> >         *(int *)rhs = temp;
> >  }
> >
> > -static __init int pop_verify_heap(bool min_heap,
> > +static __init void pop_verify_heap(struct kunit *context,
> > +                               bool min_heap,
> >                                 struct min_heap *heap,
> >                                 const struct min_heap_callbacks *funcs)
> >  {
> >         int *values = heap->data;
> > -       int err = 0;
> >         int last;
> >
> >         last = values[0];
> >         min_heap_pop(heap, funcs);
> >         while (heap->nr > 0) {
> >                 if (min_heap) {
> > -                       if (last > values[0]) {
> > -                               pr_err("error: expected %d <= %d\n", last,
> > -                                       values[0]);
> > -                               err++;
> > -                       }
> > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > +                                              last > values[0],
> > +                                              "expected %d <= %d\n",
> > +                                              last, values[0]);
>
> I'm not familiar with kunit, is there a reason not to prefer:
> KUNIT_EXPECT_LT(context, last, values[0]);
>
> >                 } else {
> > -                       if (last < values[0]) {
> > -                               pr_err("error: expected %d >= %d\n", last,
> > -                                       values[0]);
> > -                               err++;
> > -                       }
> > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > +                                              last < values[0],
> > +                                              "expected %d >= %d\n",
> > +                                              last, values[0]);
>
> Similarly KUNIT_EXPECT_GT.

In relation to this, instead of KUNIT_EXPECT_LT, we would have to have
something like KUNIT_EXPECT_LT_FALSE.
Otherwise the test will always fail.

>
> Thanks,
> Ian
>
> >                 }
> >                 last = values[0];
> >                 min_heap_pop(heap, funcs);
> >         }
> > -       return err;
> >  }
> >
> > -static __init int test_heapify_all(bool min_heap)
> > +static __init void test_heapify_all(struct kunit *context, bool min_heap)
> >  {
> >         int values[] = { 3, 1, 2, 4, 0x8000000, 0x7FFFFFF, 0,
> >                          -3, -1, -2, -4, 0x8000000, 0x7FFFFFF };
> > @@ -73,12 +69,11 @@ static __init int test_heapify_all(bool min_heap)
> >                 .less = min_heap ? less_than : greater_than,
> >                 .swp = swap_ints,
> >         };
> > -       int i, err;
> > +       int i;
> >
> >         /* Test with known set of values. */
> >         min_heapify_all(&heap, &funcs);
> > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > -
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >
> >         /* Test with randomly generated values. */
> >         heap.nr = ARRAY_SIZE(values);
> > @@ -86,12 +81,10 @@ static __init int test_heapify_all(bool min_heap)
> >                 values[i] = get_random_int();
> >
> >         min_heapify_all(&heap, &funcs);
> > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > -
> > -       return err;
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >  }
> >
> > -static __init int test_heap_push(bool min_heap)
> > +static __init void test_heap_push(struct kunit *context, bool min_heap)
> >  {
> >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > @@ -106,25 +99,22 @@ static __init int test_heap_push(bool min_heap)
> >                 .less = min_heap ? less_than : greater_than,
> >                 .swp = swap_ints,
> >         };
> > -       int i, temp, err;
> > +       int i, temp;
> >
> >         /* Test with known set of values copied from data. */
> >         for (i = 0; i < ARRAY_SIZE(data); i++)
> >                 min_heap_push(&heap, &data[i], &funcs);
> > -
> > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >
> >         /* Test with randomly generated values. */
> >         while (heap.nr < heap.size) {
> >                 temp = get_random_int();
> >                 min_heap_push(&heap, &temp, &funcs);
> >         }
> > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > -
> > -       return err;
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >  }
> >
> > -static __init int test_heap_pop_push(bool min_heap)
> > +static __init void test_heap_pop_push(struct kunit *context, bool min_heap)
> >  {
> >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > @@ -139,7 +129,7 @@ static __init int test_heap_pop_push(bool min_heap)
> >                 .less = min_heap ? less_than : greater_than,
> >                 .swp = swap_ints,
> >         };
> > -       int i, temp, err;
> > +       int i, temp;
> >
> >         /* Fill values with data to pop and replace. */
> >         temp = min_heap ? 0x80000000 : 0x7FFFFFFF;
> > @@ -149,8 +139,7 @@ static __init int test_heap_pop_push(bool min_heap)
> >         /* Test with known set of values copied from data. */
> >         for (i = 0; i < ARRAY_SIZE(data); i++)
> >                 min_heap_pop_push(&heap, &data[i], &funcs);
> > -
> > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> >
> >         heap.nr = 0;
> >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > @@ -161,34 +150,54 @@ static __init int test_heap_pop_push(bool min_heap)
> >                 temp = get_random_int();
> >                 min_heap_pop_push(&heap, &temp, &funcs);
> >         }
> > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > +}
> >
> > -       return err;
> > +static void __init test_heapify_all_true(struct kunit *context)
> > +{
> > +       test_heapify_all(context, true);
> >  }
> >
> > -static int __init test_min_heap_init(void)
> > +static void __init test_heapify_all_false(struct kunit *context)
> >  {
> > -       int err = 0;
> > -
> > -       err += test_heapify_all(true);
> > -       err += test_heapify_all(false);
> > -       err += test_heap_push(true);
> > -       err += test_heap_push(false);
> > -       err += test_heap_pop_push(true);
> > -       err += test_heap_pop_push(false);
> > -       if (err) {
> > -               pr_err("test failed with %d errors\n", err);
> > -               return -EINVAL;
> > -       }
> > -       pr_info("test passed\n");
> > -       return 0;
> > +       test_heapify_all(context, true);
> > +}
> > +
> > +static void __init test_heap_push_true(struct kunit *context)
> > +{
> > +       test_heap_push(context, true);
> > +}
> > +
> > +static void __init test_heap_push_false(struct kunit *context)
> > +{
> > +       test_heap_push(context, false);
> >  }
> > -module_init(test_min_heap_init);
> >
> > -static void __exit test_min_heap_exit(void)
> > +static void __init test_heap_pop_push_true(struct kunit *context)
> >  {
> > -       /* do nothing */
> > +       test_heap_pop_push(context, true);
> >  }
> > -module_exit(test_min_heap_exit);
> > +
> > +static void __init test_heap_pop_push_false(struct kunit *context)
> > +{
> > +       test_heap_pop_push(context, false);
> > +}
> > +
> > +static struct kunit_case __refdata min_heap_test_cases[] = {
> > +       KUNIT_CASE(test_heapify_all_true),
> > +       KUNIT_CASE(test_heapify_all_false),
> > +       KUNIT_CASE(test_heap_push_true),
> > +       KUNIT_CASE(test_heap_push_false),
> > +       KUNIT_CASE(test_heap_pop_push_true),
> > +       KUNIT_CASE(test_heap_pop_push_false),
> > +       {}
> > +};
> > +
> > +static struct kunit_suite min_heap_test_suite = {
> > +       .name = "min-heap",
> > +       .test_cases = min_heap_test_cases,
> > +};
> > +
> > +kunit_test_suites(&min_heap_test_suite);
> >
> >  MODULE_LICENSE("GPL");
> >
> > base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
> > --
> > 2.26.2
> >
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-10-14 23:49   ` Vitor Massaru Iha
@ 2020-10-15 16:30     ` Ian Rogers via Linux-kernel-mentees
  2020-10-15 16:56       ` Vitor Massaru Iha
  0 siblings, 1 reply; 16+ messages in thread
From: Ian Rogers via Linux-kernel-mentees @ 2020-10-15 16:30 UTC (permalink / raw)
  To: Vitor Massaru Iha
  Cc: Peter Zijlstra, Brendan Higgins, LKML,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	Ingo Molnar, KUnit Development

On Wed, Oct 14, 2020 at 4:49 PM Vitor Massaru Iha <vitor@massaru.org> wrote:
>
> Hi Ian,
>
>
> On Wed, Jul 29, 2020 at 7:57 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Wed, Jul 29, 2020 at 1:11 PM Vitor Massaru Iha <vitor@massaru.org> wrote:
> > >
> > > This adds the conversion of the runtime tests of test_min_heap,
> > > from `lib/test_min_heap.c` to KUnit tests.
> > >
> > > Please apply this commit first (linux-kselftest/kunit-fixes):
> > > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
> >
> > Thanks for this, I'm a fan of testing frameworks :-)
> >
> > > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > > ---
> > >  lib/Kconfig.debug                         |  29 ++++--
> > >  lib/Makefile                              |   2 +-
> > >  lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
> > >  3 files changed, 83 insertions(+), 65 deletions(-)
> > >  rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)
> > >
> > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > > index 9ad9210d70a1..46674fc4972c 100644
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -1864,16 +1864,6 @@ config TEST_LIST_SORT
> > >
> > >           If unsure, say N.
> > >
> > > -config TEST_MIN_HEAP
> > > -       tristate "Min heap test"
> > > -       depends on DEBUG_KERNEL || m
> > > -       help
> > > -         Enable this to turn on min heap function tests. This test is
> > > -         executed only once during system boot (so affects only boot time),
> > > -         or at module load time.
> > > -
> > > -         If unsure, say N.
> > > -
> > >  config TEST_SORT
> > >         tristate "Array-based sort test"
> > >         depends on DEBUG_KERNEL || m
> > > @@ -2185,6 +2175,25 @@ config LINEAR_RANGES_TEST
> > >
> > >           If unsure, say N.
> > >
> > > +config MIN_HEAP_KUNIT
> > > +        tristate "KUnit test for Min heap"
> > > +        depends on KUNIT
> > > +        depends on DEBUG_KERNEL || m
> > > +        help
> > > +          Enable this to turn on min heap function tests. This test is
> > > +          executed only once during system boot (so affects only boot time),
> > > +          or at module load time.
> > > +
> > > +          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 the KUnit test harness, and not intended 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.
> > > +
> >
> > It's a shame we need a config option for this. Could we have one
> > option to cover all basic library tests?
> >
> > >  config TEST_UDELAY
> > >         tristate "udelay test driver"
> > >         help
> > > diff --git a/lib/Makefile b/lib/Makefile
> > > index b1c42c10073b..748f57063160 100644
> > > --- a/lib/Makefile
> > > +++ b/lib/Makefile
> > > @@ -72,7 +72,6 @@ CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
> > >  UBSAN_SANITIZE_test_ubsan.o := y
> > >  obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
> > >  obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> > > -obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
> > >  obj-$(CONFIG_TEST_LKM) += test_module.o
> > >  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
> > >  obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
> > > @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
> > >  # KUnit tests
> > >  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> > >  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> > > +obj-$(CONFIG_MIN_HEAP_KUNIT) += min_heap_kunit.o
> > > diff --git a/lib/test_min_heap.c b/lib/min_heap_kunit.c
> > > similarity index 60%
> > > rename from lib/test_min_heap.c
> > > rename to lib/min_heap_kunit.c
> > > index d19c8080fd4d..398db1c63146 100644
> > > --- a/lib/test_min_heap.c
> > > +++ b/lib/min_heap_kunit.c
> > > @@ -7,9 +7,8 @@
> > >
> > >  #include <linux/log2.h>
> > >  #include <linux/min_heap.h>
> > > -#include <linux/module.h>
> > > -#include <linux/printk.h>
> > >  #include <linux/random.h>
> > > +#include <kunit/test.h>
> > >
> > >  static __init bool less_than(const void *lhs, const void *rhs)
> > >  {
> > > @@ -29,37 +28,34 @@ static __init void swap_ints(void *lhs, void *rhs)
> > >         *(int *)rhs = temp;
> > >  }
> > >
> > > -static __init int pop_verify_heap(bool min_heap,
> > > +static __init void pop_verify_heap(struct kunit *context,
> > > +                               bool min_heap,
> > >                                 struct min_heap *heap,
> > >                                 const struct min_heap_callbacks *funcs)
> > >  {
> > >         int *values = heap->data;
> > > -       int err = 0;
> > >         int last;
> > >
> > >         last = values[0];
> > >         min_heap_pop(heap, funcs);
> > >         while (heap->nr > 0) {
> > >                 if (min_heap) {
> > > -                       if (last > values[0]) {
> > > -                               pr_err("error: expected %d <= %d\n", last,
> > > -                                       values[0]);
> > > -                               err++;
> > > -                       }
> > > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > > +                                              last > values[0],
> > > +                                              "expected %d <= %d\n",
> > > +                                              last, values[0]);
> >
> > I'm not familiar with kunit, is there a reason not to prefer:
> > KUNIT_EXPECT_LT(context, last, values[0]);
> >
> > >                 } else {
> > > -                       if (last < values[0]) {
> > > -                               pr_err("error: expected %d >= %d\n", last,
> > > -                                       values[0]);
> > > -                               err++;
> > > -                       }
> > > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > > +                                              last < values[0],
> > > +                                              "expected %d >= %d\n",
> > > +                                              last, values[0]);
> >
> > Similarly KUNIT_EXPECT_GT.
>
> In relation to this, instead of KUNIT_EXPECT_LT, we would have to have
> something like KUNIT_EXPECT_LT_FALSE.
> Otherwise the test will always fail.

Does KUNIT_EXPECT_GE not work? The error message suggests it should at least.

Thanks,
Ian

> >
> > Thanks,
> > Ian
> >
> > >                 }
> > >                 last = values[0];
> > >                 min_heap_pop(heap, funcs);
> > >         }
> > > -       return err;
> > >  }
> > >
> > > -static __init int test_heapify_all(bool min_heap)
> > > +static __init void test_heapify_all(struct kunit *context, bool min_heap)
> > >  {
> > >         int values[] = { 3, 1, 2, 4, 0x8000000, 0x7FFFFFF, 0,
> > >                          -3, -1, -2, -4, 0x8000000, 0x7FFFFFF };
> > > @@ -73,12 +69,11 @@ static __init int test_heapify_all(bool min_heap)
> > >                 .less = min_heap ? less_than : greater_than,
> > >                 .swp = swap_ints,
> > >         };
> > > -       int i, err;
> > > +       int i;
> > >
> > >         /* Test with known set of values. */
> > >         min_heapify_all(&heap, &funcs);
> > > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > > -
> > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > >
> > >         /* Test with randomly generated values. */
> > >         heap.nr = ARRAY_SIZE(values);
> > > @@ -86,12 +81,10 @@ static __init int test_heapify_all(bool min_heap)
> > >                 values[i] = get_random_int();
> > >
> > >         min_heapify_all(&heap, &funcs);
> > > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > > -
> > > -       return err;
> > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > >  }
> > >
> > > -static __init int test_heap_push(bool min_heap)
> > > +static __init void test_heap_push(struct kunit *context, bool min_heap)
> > >  {
> > >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> > >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > > @@ -106,25 +99,22 @@ static __init int test_heap_push(bool min_heap)
> > >                 .less = min_heap ? less_than : greater_than,
> > >                 .swp = swap_ints,
> > >         };
> > > -       int i, temp, err;
> > > +       int i, temp;
> > >
> > >         /* Test with known set of values copied from data. */
> > >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > >                 min_heap_push(&heap, &data[i], &funcs);
> > > -
> > > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > >
> > >         /* Test with randomly generated values. */
> > >         while (heap.nr < heap.size) {
> > >                 temp = get_random_int();
> > >                 min_heap_push(&heap, &temp, &funcs);
> > >         }
> > > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > > -
> > > -       return err;
> > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > >  }
> > >
> > > -static __init int test_heap_pop_push(bool min_heap)
> > > +static __init void test_heap_pop_push(struct kunit *context, bool min_heap)
> > >  {
> > >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> > >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > > @@ -139,7 +129,7 @@ static __init int test_heap_pop_push(bool min_heap)
> > >                 .less = min_heap ? less_than : greater_than,
> > >                 .swp = swap_ints,
> > >         };
> > > -       int i, temp, err;
> > > +       int i, temp;
> > >
> > >         /* Fill values with data to pop and replace. */
> > >         temp = min_heap ? 0x80000000 : 0x7FFFFFFF;
> > > @@ -149,8 +139,7 @@ static __init int test_heap_pop_push(bool min_heap)
> > >         /* Test with known set of values copied from data. */
> > >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > >                 min_heap_pop_push(&heap, &data[i], &funcs);
> > > -
> > > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > >
> > >         heap.nr = 0;
> > >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > > @@ -161,34 +150,54 @@ static __init int test_heap_pop_push(bool min_heap)
> > >                 temp = get_random_int();
> > >                 min_heap_pop_push(&heap, &temp, &funcs);
> > >         }
> > > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > > +}
> > >
> > > -       return err;
> > > +static void __init test_heapify_all_true(struct kunit *context)
> > > +{
> > > +       test_heapify_all(context, true);
> > >  }
> > >
> > > -static int __init test_min_heap_init(void)
> > > +static void __init test_heapify_all_false(struct kunit *context)
> > >  {
> > > -       int err = 0;
> > > -
> > > -       err += test_heapify_all(true);
> > > -       err += test_heapify_all(false);
> > > -       err += test_heap_push(true);
> > > -       err += test_heap_push(false);
> > > -       err += test_heap_pop_push(true);
> > > -       err += test_heap_pop_push(false);
> > > -       if (err) {
> > > -               pr_err("test failed with %d errors\n", err);
> > > -               return -EINVAL;
> > > -       }
> > > -       pr_info("test passed\n");
> > > -       return 0;
> > > +       test_heapify_all(context, true);
> > > +}
> > > +
> > > +static void __init test_heap_push_true(struct kunit *context)
> > > +{
> > > +       test_heap_push(context, true);
> > > +}
> > > +
> > > +static void __init test_heap_push_false(struct kunit *context)
> > > +{
> > > +       test_heap_push(context, false);
> > >  }
> > > -module_init(test_min_heap_init);
> > >
> > > -static void __exit test_min_heap_exit(void)
> > > +static void __init test_heap_pop_push_true(struct kunit *context)
> > >  {
> > > -       /* do nothing */
> > > +       test_heap_pop_push(context, true);
> > >  }
> > > -module_exit(test_min_heap_exit);
> > > +
> > > +static void __init test_heap_pop_push_false(struct kunit *context)
> > > +{
> > > +       test_heap_pop_push(context, false);
> > > +}
> > > +
> > > +static struct kunit_case __refdata min_heap_test_cases[] = {
> > > +       KUNIT_CASE(test_heapify_all_true),
> > > +       KUNIT_CASE(test_heapify_all_false),
> > > +       KUNIT_CASE(test_heap_push_true),
> > > +       KUNIT_CASE(test_heap_push_false),
> > > +       KUNIT_CASE(test_heap_pop_push_true),
> > > +       KUNIT_CASE(test_heap_pop_push_false),
> > > +       {}
> > > +};
> > > +
> > > +static struct kunit_suite min_heap_test_suite = {
> > > +       .name = "min-heap",
> > > +       .test_cases = min_heap_test_cases,
> > > +};
> > > +
> > > +kunit_test_suites(&min_heap_test_suite);
> > >
> > >  MODULE_LICENSE("GPL");
> > >
> > > base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
> > > --
> > > 2.26.2
> > >
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit
  2020-10-15 16:30     ` Ian Rogers via Linux-kernel-mentees
@ 2020-10-15 16:56       ` Vitor Massaru Iha
  0 siblings, 0 replies; 16+ messages in thread
From: Vitor Massaru Iha @ 2020-10-15 16:56 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Brendan Higgins, LKML,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel-mentees,
	Ingo Molnar, KUnit Development

Hi Ian,

On Thu, Oct 15, 2020 at 1:30 PM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Oct 14, 2020 at 4:49 PM Vitor Massaru Iha <vitor@massaru.org> wrote:
> >
> > Hi Ian,
> >
> >
> > On Wed, Jul 29, 2020 at 7:57 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > On Wed, Jul 29, 2020 at 1:11 PM Vitor Massaru Iha <vitor@massaru.org> wrote:
> > > >
> > > > This adds the conversion of the runtime tests of test_min_heap,
> > > > from `lib/test_min_heap.c` to KUnit tests.
> > > >
> > > > Please apply this commit first (linux-kselftest/kunit-fixes):
> > > > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
> > >
> > > Thanks for this, I'm a fan of testing frameworks :-)
> > >
> > > > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > > > ---
> > > >  lib/Kconfig.debug                         |  29 ++++--
> > > >  lib/Makefile                              |   2 +-
> > > >  lib/{test_min_heap.c => min_heap_kunit.c} | 117 ++++++++++++----------
> > > >  3 files changed, 83 insertions(+), 65 deletions(-)
> > > >  rename lib/{test_min_heap.c => min_heap_kunit.c} (60%)
> > > >
> > > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > > > index 9ad9210d70a1..46674fc4972c 100644
> > > > --- a/lib/Kconfig.debug
> > > > +++ b/lib/Kconfig.debug
> > > > @@ -1864,16 +1864,6 @@ config TEST_LIST_SORT
> > > >
> > > >           If unsure, say N.
> > > >
> > > > -config TEST_MIN_HEAP
> > > > -       tristate "Min heap test"
> > > > -       depends on DEBUG_KERNEL || m
> > > > -       help
> > > > -         Enable this to turn on min heap function tests. This test is
> > > > -         executed only once during system boot (so affects only boot time),
> > > > -         or at module load time.
> > > > -
> > > > -         If unsure, say N.
> > > > -
> > > >  config TEST_SORT
> > > >         tristate "Array-based sort test"
> > > >         depends on DEBUG_KERNEL || m
> > > > @@ -2185,6 +2175,25 @@ config LINEAR_RANGES_TEST
> > > >
> > > >           If unsure, say N.
> > > >
> > > > +config MIN_HEAP_KUNIT
> > > > +        tristate "KUnit test for Min heap"
> > > > +        depends on KUNIT
> > > > +        depends on DEBUG_KERNEL || m
> > > > +        help
> > > > +          Enable this to turn on min heap function tests. This test is
> > > > +          executed only once during system boot (so affects only boot time),
> > > > +          or at module load time.
> > > > +
> > > > +          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 the KUnit test harness, and not intended 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.
> > > > +
> > >
> > > It's a shame we need a config option for this. Could we have one
> > > option to cover all basic library tests?
> > >
> > > >  config TEST_UDELAY
> > > >         tristate "udelay test driver"
> > > >         help
> > > > diff --git a/lib/Makefile b/lib/Makefile
> > > > index b1c42c10073b..748f57063160 100644
> > > > --- a/lib/Makefile
> > > > +++ b/lib/Makefile
> > > > @@ -72,7 +72,6 @@ CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
> > > >  UBSAN_SANITIZE_test_ubsan.o := y
> > > >  obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
> > > >  obj-$(CONFIG_TEST_LIST_SORT) += test_list_sort.o
> > > > -obj-$(CONFIG_TEST_MIN_HEAP) += test_min_heap.o
> > > >  obj-$(CONFIG_TEST_LKM) += test_module.o
> > > >  obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o
> > > >  obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o
> > > > @@ -318,3 +317,4 @@ obj-$(CONFIG_OBJAGG) += objagg.o
> > > >  # KUnit tests
> > > >  obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> > > >  obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
> > > > +obj-$(CONFIG_MIN_HEAP_KUNIT) += min_heap_kunit.o
> > > > diff --git a/lib/test_min_heap.c b/lib/min_heap_kunit.c
> > > > similarity index 60%
> > > > rename from lib/test_min_heap.c
> > > > rename to lib/min_heap_kunit.c
> > > > index d19c8080fd4d..398db1c63146 100644
> > > > --- a/lib/test_min_heap.c
> > > > +++ b/lib/min_heap_kunit.c
> > > > @@ -7,9 +7,8 @@
> > > >
> > > >  #include <linux/log2.h>
> > > >  #include <linux/min_heap.h>
> > > > -#include <linux/module.h>
> > > > -#include <linux/printk.h>
> > > >  #include <linux/random.h>
> > > > +#include <kunit/test.h>
> > > >
> > > >  static __init bool less_than(const void *lhs, const void *rhs)
> > > >  {
> > > > @@ -29,37 +28,34 @@ static __init void swap_ints(void *lhs, void *rhs)
> > > >         *(int *)rhs = temp;
> > > >  }
> > > >
> > > > -static __init int pop_verify_heap(bool min_heap,
> > > > +static __init void pop_verify_heap(struct kunit *context,
> > > > +                               bool min_heap,
> > > >                                 struct min_heap *heap,
> > > >                                 const struct min_heap_callbacks *funcs)
> > > >  {
> > > >         int *values = heap->data;
> > > > -       int err = 0;
> > > >         int last;
> > > >
> > > >         last = values[0];
> > > >         min_heap_pop(heap, funcs);
> > > >         while (heap->nr > 0) {
> > > >                 if (min_heap) {
> > > > -                       if (last > values[0]) {
> > > > -                               pr_err("error: expected %d <= %d\n", last,
> > > > -                                       values[0]);
> > > > -                               err++;
> > > > -                       }
> > > > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > > > +                                              last > values[0],
> > > > +                                              "expected %d <= %d\n",
> > > > +                                              last, values[0]);
> > >
> > > I'm not familiar with kunit, is there a reason not to prefer:
> > > KUNIT_EXPECT_LT(context, last, values[0]);
> > >
> > > >                 } else {
> > > > -                       if (last < values[0]) {
> > > > -                               pr_err("error: expected %d >= %d\n", last,
> > > > -                                       values[0]);
> > > > -                               err++;
> > > > -                       }
> > > > +                       KUNIT_EXPECT_FALSE_MSG(context,
> > > > +                                              last < values[0],
> > > > +                                              "expected %d >= %d\n",
> > > > +                                              last, values[0]);
> > >
> > > Similarly KUNIT_EXPECT_GT.
> >
> > In relation to this, instead of KUNIT_EXPECT_LT, we would have to have
> > something like KUNIT_EXPECT_LT_FALSE.
> > Otherwise the test will always fail.
>
> Does KUNIT_EXPECT_GE not work? The error message suggests it should at least.

My bad, both expect work, I will send v3 now.

I'm working with other projects in parallel, and my context switch failed :(

> Thanks,
> Ian
>
> > >
> > > Thanks,
> > > Ian
> > >
> > > >                 }
> > > >                 last = values[0];
> > > >                 min_heap_pop(heap, funcs);
> > > >         }
> > > > -       return err;
> > > >  }
> > > >
> > > > -static __init int test_heapify_all(bool min_heap)
> > > > +static __init void test_heapify_all(struct kunit *context, bool min_heap)
> > > >  {
> > > >         int values[] = { 3, 1, 2, 4, 0x8000000, 0x7FFFFFF, 0,
> > > >                          -3, -1, -2, -4, 0x8000000, 0x7FFFFFF };
> > > > @@ -73,12 +69,11 @@ static __init int test_heapify_all(bool min_heap)
> > > >                 .less = min_heap ? less_than : greater_than,
> > > >                 .swp = swap_ints,
> > > >         };
> > > > -       int i, err;
> > > > +       int i;
> > > >
> > > >         /* Test with known set of values. */
> > > >         min_heapify_all(&heap, &funcs);
> > > > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > > > -
> > > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > > >
> > > >         /* Test with randomly generated values. */
> > > >         heap.nr = ARRAY_SIZE(values);
> > > > @@ -86,12 +81,10 @@ static __init int test_heapify_all(bool min_heap)
> > > >                 values[i] = get_random_int();
> > > >
> > > >         min_heapify_all(&heap, &funcs);
> > > > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > > > -
> > > > -       return err;
> > > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > > >  }
> > > >
> > > > -static __init int test_heap_push(bool min_heap)
> > > > +static __init void test_heap_push(struct kunit *context, bool min_heap)
> > > >  {
> > > >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> > > >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > > > @@ -106,25 +99,22 @@ static __init int test_heap_push(bool min_heap)
> > > >                 .less = min_heap ? less_than : greater_than,
> > > >                 .swp = swap_ints,
> > > >         };
> > > > -       int i, temp, err;
> > > > +       int i, temp;
> > > >
> > > >         /* Test with known set of values copied from data. */
> > > >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > > >                 min_heap_push(&heap, &data[i], &funcs);
> > > > -
> > > > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > > >
> > > >         /* Test with randomly generated values. */
> > > >         while (heap.nr < heap.size) {
> > > >                 temp = get_random_int();
> > > >                 min_heap_push(&heap, &temp, &funcs);
> > > >         }
> > > > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > > > -
> > > > -       return err;
> > > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > > >  }
> > > >
> > > > -static __init int test_heap_pop_push(bool min_heap)
> > > > +static __init void test_heap_pop_push(struct kunit *context, bool min_heap)
> > > >  {
> > > >         const int data[] = { 3, 1, 2, 4, 0x80000000, 0x7FFFFFFF, 0,
> > > >                              -3, -1, -2, -4, 0x80000000, 0x7FFFFFFF };
> > > > @@ -139,7 +129,7 @@ static __init int test_heap_pop_push(bool min_heap)
> > > >                 .less = min_heap ? less_than : greater_than,
> > > >                 .swp = swap_ints,
> > > >         };
> > > > -       int i, temp, err;
> > > > +       int i, temp;
> > > >
> > > >         /* Fill values with data to pop and replace. */
> > > >         temp = min_heap ? 0x80000000 : 0x7FFFFFFF;
> > > > @@ -149,8 +139,7 @@ static __init int test_heap_pop_push(bool min_heap)
> > > >         /* Test with known set of values copied from data. */
> > > >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > > >                 min_heap_pop_push(&heap, &data[i], &funcs);
> > > > -
> > > > -       err = pop_verify_heap(min_heap, &heap, &funcs);
> > > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > > >
> > > >         heap.nr = 0;
> > > >         for (i = 0; i < ARRAY_SIZE(data); i++)
> > > > @@ -161,34 +150,54 @@ static __init int test_heap_pop_push(bool min_heap)
> > > >                 temp = get_random_int();
> > > >                 min_heap_pop_push(&heap, &temp, &funcs);
> > > >         }
> > > > -       err += pop_verify_heap(min_heap, &heap, &funcs);
> > > > +       pop_verify_heap(context, min_heap, &heap, &funcs);
> > > > +}
> > > >
> > > > -       return err;
> > > > +static void __init test_heapify_all_true(struct kunit *context)
> > > > +{
> > > > +       test_heapify_all(context, true);
> > > >  }
> > > >
> > > > -static int __init test_min_heap_init(void)
> > > > +static void __init test_heapify_all_false(struct kunit *context)
> > > >  {
> > > > -       int err = 0;
> > > > -
> > > > -       err += test_heapify_all(true);
> > > > -       err += test_heapify_all(false);
> > > > -       err += test_heap_push(true);
> > > > -       err += test_heap_push(false);
> > > > -       err += test_heap_pop_push(true);
> > > > -       err += test_heap_pop_push(false);
> > > > -       if (err) {
> > > > -               pr_err("test failed with %d errors\n", err);
> > > > -               return -EINVAL;
> > > > -       }
> > > > -       pr_info("test passed\n");
> > > > -       return 0;
> > > > +       test_heapify_all(context, true);
> > > > +}
> > > > +
> > > > +static void __init test_heap_push_true(struct kunit *context)
> > > > +{
> > > > +       test_heap_push(context, true);
> > > > +}
> > > > +
> > > > +static void __init test_heap_push_false(struct kunit *context)
> > > > +{
> > > > +       test_heap_push(context, false);
> > > >  }
> > > > -module_init(test_min_heap_init);
> > > >
> > > > -static void __exit test_min_heap_exit(void)
> > > > +static void __init test_heap_pop_push_true(struct kunit *context)
> > > >  {
> > > > -       /* do nothing */
> > > > +       test_heap_pop_push(context, true);
> > > >  }
> > > > -module_exit(test_min_heap_exit);
> > > > +
> > > > +static void __init test_heap_pop_push_false(struct kunit *context)
> > > > +{
> > > > +       test_heap_pop_push(context, false);
> > > > +}
> > > > +
> > > > +static struct kunit_case __refdata min_heap_test_cases[] = {
> > > > +       KUNIT_CASE(test_heapify_all_true),
> > > > +       KUNIT_CASE(test_heapify_all_false),
> > > > +       KUNIT_CASE(test_heap_push_true),
> > > > +       KUNIT_CASE(test_heap_push_false),
> > > > +       KUNIT_CASE(test_heap_pop_push_true),
> > > > +       KUNIT_CASE(test_heap_pop_push_false),
> > > > +       {}
> > > > +};
> > > > +
> > > > +static struct kunit_suite min_heap_test_suite = {
> > > > +       .name = "min-heap",
> > > > +       .test_cases = min_heap_test_cases,
> > > > +};
> > > > +
> > > > +kunit_test_suites(&min_heap_test_suite);
> > > >
> > > >  MODULE_LICENSE("GPL");
> > > >
> > > > base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
> > > > --
> > > > 2.26.2
> > > >
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

end of thread, other threads:[~2020-10-15 16:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 20:11 [Linux-kernel-mentees] [PATCH] lib: kunit: add test_min_heap test conversion to KUnit Vitor Massaru Iha
2020-07-29 20:39 ` peterz
2020-07-29 21:57   ` Vitor Massaru Iha
2020-08-04 13:25     ` peterz
2020-08-04 13:46       ` Vitor Massaru Iha
2020-08-04 14:23         ` peterz
2020-08-04 16:22           ` Vitor Massaru Iha
2020-10-12 21:02             ` Brendan Higgins via Linux-kernel-mentees
2020-10-14 18:16               ` Ian Rogers via Linux-kernel-mentees
2020-10-14 19:59                 ` Peter Zijlstra
2020-10-14 20:03                   ` Vitor Massaru Iha
2020-07-29 22:56 ` Ian Rogers via Linux-kernel-mentees
2020-07-29 23:12   ` Vitor Massaru Iha
2020-10-14 23:49   ` Vitor Massaru Iha
2020-10-15 16:30     ` Ian Rogers via Linux-kernel-mentees
2020-10-15 16:56       ` Vitor Massaru Iha

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).