| Message ID | 20201015014616.309000-1-vitor@massaru.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] lib: kunit: add list_sort test conversion to KUnit | expand |
On Wed, Oct 14, 2020 at 10:46:16PM -0300, Vitor Massaru Iha wrote: > This adds the conversion of the runtime tests of test_list_sort, > from `lib/test_list_sort.c` to KUnit tests. Please, provide better commit message. For example, add the output of _succeeded_ *and* _failed_ test cases (yes, you have to break code manually to reproduce failed test cases) _before_ *and* _after_ the change. W/o above information I'm going to NAK this as I promised before.
On Wed, Oct 14, 2020 at 10:46:16PM -0300, Vitor Massaru Iha wrote: > This adds the conversion of the runtime tests of test_list_sort, > from `lib/test_list_sort.c` to KUnit tests. > rename lib/{test_list_sort.c => list_sort_kunit.c} (62%) One more thing. The documentation [1] doesn't specify any name conventions. So, please leave file name untouched. [1]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html
On Thu, Oct 15, 2020 at 7:50 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Oct 14, 2020 at 10:46:16PM -0300, Vitor Massaru Iha wrote: > > This adds the conversion of the runtime tests of test_list_sort, > > from `lib/test_list_sort.c` to KUnit tests. > > Please, provide better commit message. For example, add the output of _succeeded_ > *and* _failed_ test cases (yes, you have to break code manually to reproduce failed > test cases) _before_ *and* _after_ the change. Sure, I'll do it. Thanks! > W/o above information I'm going to NAK this as I promised before. > > -- > With Best Regards, > Andy Shevchenko > >
On Thu, Oct 15, 2020 at 2:47 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Oct 14, 2020 at 10:46:16PM -0300, Vitor Massaru Iha wrote: > > This adds the conversion of the runtime tests of test_list_sort, > > from `lib/test_list_sort.c` to KUnit tests. > > > rename lib/{test_list_sort.c => list_sort_kunit.c} (62%) > > One more thing. The documentation [1] doesn't specify any name conventions. > So, please leave file name untouched. > > [1]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html This convention hasn't yet entered the Torvaldos tree, but it is in the Shuah tree: [1] +Test File and Module Names +========================== + +KUnit tests can often be compiled as a module. These modules should be named +after the test suite, followed by ``_test``. If this is likely to conflict with +non-KUnit tests, the suffix ``_kunit`` can also be used. + +The easiest way of achieving this is to name the file containing the test suite +``<suite>_test.c`` (or, as above, ``<suite>_kunit.c``). This file should be +placed next to the code under test. + +If the suite name contains some or all of the name of the test's parent +directory, it may make sense to modify the source filename to reduce redundancy. +For example, a ``foo_firmware`` suite could be in the ``foo/firmware_test.c`` +file. [1] https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit&id=2a41fc52c21b6ece49921716bd289bfebaadcc04 > > -- > With Best Regards, > Andy Shevchenko > > > -- > 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/20201015174823.GY4077%40smile.fi.intel.com.
On Thu, Oct 15, 2020 at 02:59:05PM -0300, Vitor Massaru Iha wrote: > On Thu, Oct 15, 2020 at 2:47 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Wed, Oct 14, 2020 at 10:46:16PM -0300, Vitor Massaru Iha wrote: > > > This adds the conversion of the runtime tests of test_list_sort, > > > from `lib/test_list_sort.c` to KUnit tests. > > > > > rename lib/{test_list_sort.c => list_sort_kunit.c} (62%) > > > > One more thing. The documentation [1] doesn't specify any name conventions. > > So, please leave file name untouched. > > > > [1]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html > > This convention hasn't yet entered the Torvaldos tree, but it is in > the Shuah tree: [1] > > +Test File and Module Names > +========================== > + > +KUnit tests can often be compiled as a module. These modules should be named > +after the test suite, followed by ``_test``. If this is likely to conflict with > +non-KUnit tests, the suffix ``_kunit`` can also be used. > + > +The easiest way of achieving this is to name the file containing the test suite > +``<suite>_test.c`` (or, as above, ``<suite>_kunit.c``). This file should be > +placed next to the code under test. > + > +If the suite name contains some or all of the name of the test's parent > +directory, it may make sense to modify the source filename to reduce > redundancy. > +For example, a ``foo_firmware`` suite could be in the ``foo/firmware_test.c`` > +file. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit&id=2a41fc52c21b6ece49921716bd289bfebaadcc04 I see. Can the rest of documentation be consistent with this? It seems half-baked, since examples show something different. (Entire documentation if full of example-test.c) -- With Best Regards, Andy Shevchenko
On Thu, Oct 15, 2020 at 09:21:21PM +0300, Andy Shevchenko wrote: > On Thu, Oct 15, 2020 at 02:59:05PM -0300, Vitor Massaru Iha wrote: > > On Thu, Oct 15, 2020 at 2:47 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > On Wed, Oct 14, 2020 at 10:46:16PM -0300, Vitor Massaru Iha wrote: > > > > This adds the conversion of the runtime tests of test_list_sort, > > > > from `lib/test_list_sort.c` to KUnit tests. > > > > > > > rename lib/{test_list_sort.c => list_sort_kunit.c} (62%) > > > > > > One more thing. The documentation [1] doesn't specify any name conventions. > > > So, please leave file name untouched. > > > > > > [1]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html > > > > This convention hasn't yet entered the Torvaldos tree, but it is in > > the Shuah tree: [1] > > > > +Test File and Module Names > > +========================== > > + > > +KUnit tests can often be compiled as a module. These modules should be named > > +after the test suite, followed by ``_test``. If this is likely to conflict with > > +non-KUnit tests, the suffix ``_kunit`` can also be used. > > + > > +The easiest way of achieving this is to name the file containing the test suite > > +``<suite>_test.c`` (or, as above, ``<suite>_kunit.c``). This file should be > > +placed next to the code under test. > > + > > +If the suite name contains some or all of the name of the test's parent > > +directory, it may make sense to modify the source filename to reduce > > redundancy. > > +For example, a ``foo_firmware`` suite could be in the ``foo/firmware_test.c`` > > +file. > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit&id=2a41fc52c21b6ece49921716bd289bfebaadcc04 > > I see. > > Can the rest of documentation be consistent with this? It seems half-baked, > since examples show something different. > > (Entire documentation if full of example-test.c) Also it doesn't really clear about the test cases against header files, like for list.h or bits.h. (There is no c-file and I'm not sure one desires to have include/linux/*_kunit.c)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 4f09c6505a2e..b4b1338c523a 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1905,16 +1905,6 @@ config LKDTM Documentation on how to use the module can be found in Documentation/fault-injection/provoke-crashes.rst -config TEST_LIST_SORT - tristate "Linked list sorting test" - depends on DEBUG_KERNEL || m - help - Enable this to turn on 'list_sort()' function test. 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_MIN_HEAP tristate "Min heap test" depends on DEBUG_KERNEL || m @@ -2233,6 +2223,20 @@ config LIST_KUNIT_TEST If unsure, say N. +config LIST_SORT_KUNIT_TEST + tristate "KUnit Linked list sorting test" if !KUNIT_ALL_TESTS + depends on KUNIT + default KUNIT_ALL_TESTS + help + Enable this to turn on 'list_sort()' function test. This test is + executed only once during system boot (so affects only boot time), + or at module load time. + + 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 LINEAR_RANGES_TEST tristate "KUnit test for linear_ranges" depends on KUNIT diff --git a/lib/Makefile b/lib/Makefile index d862d41fdc3d..a00e26d34263 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -67,7 +67,6 @@ obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o 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 @@ -341,5 +340,6 @@ obj-$(CONFIG_PLDMFW) += pldmfw/ # KUnit tests obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o +obj-$(CONFIG_LIST_SORT_KUNIT_TEST) += list_sort_kunit.o obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o obj-$(CONFIG_BITS_TEST) += test_bits.o diff --git a/lib/test_list_sort.c b/lib/list_sort_kunit.c similarity index 62% rename from lib/test_list_sort.c rename to lib/list_sort_kunit.c index 1f017d3b610e..20cbacbb7d6c 100644 --- a/lib/test_list_sort.c +++ b/lib/list_sort_kunit.c @@ -1,13 +1,10 @@ // SPDX-License-Identifier: GPL-2.0-only -#define pr_fmt(fmt) "list_sort_test: " fmt +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include <linux/kernel.h> #include <linux/list_sort.h> #include <linux/list.h> -#include <linux/module.h> -#include <linux/printk.h> -#include <linux/slab.h> #include <linux/random.h> +#include <kunit/test.h> /* * The pattern of set bits in the list length determines which cases @@ -29,28 +26,28 @@ struct debug_el { /* Array, containing pointers to all elements in the test list */ static struct debug_el **elts __initdata; -static int __init check(struct debug_el *ela, struct debug_el *elb) +static int __init check(struct kunit *context, struct debug_el *ela, struct debug_el *elb) { if (ela->serial >= TEST_LIST_LEN) { - pr_err("error: incorrect serial %d\n", ela->serial); + KUNIT_FAIL(context, "incorrect serial %d", ela->serial); return -EINVAL; } if (elb->serial >= TEST_LIST_LEN) { - pr_err("error: incorrect serial %d\n", elb->serial); + KUNIT_FAIL(context, "incorrect serial %d", elb->serial); return -EINVAL; } if (elts[ela->serial] != ela || elts[elb->serial] != elb) { - pr_err("error: phantom element\n"); + KUNIT_FAIL(context, "phantom element"); return -EINVAL; } if (ela->poison1 != TEST_POISON1 || ela->poison2 != TEST_POISON2) { - pr_err("error: bad poison: %#x/%#x\n", - ela->poison1, ela->poison2); + KUNIT_FAIL(context, "bad poison: %#x/%#x", + ela->poison1, ela->poison2); return -EINVAL; } if (elb->poison1 != TEST_POISON1 || elb->poison2 != TEST_POISON2) { - pr_err("error: bad poison: %#x/%#x\n", - elb->poison1, elb->poison2); + KUNIT_FAIL(context, "bad poison: %#x/%#x", + elb->poison1, elb->poison2); return -EINVAL; } return 0; @@ -63,27 +60,26 @@ static int __init cmp(void *priv, struct list_head *a, struct list_head *b) ela = container_of(a, struct debug_el, list); elb = container_of(b, struct debug_el, list); - check(ela, elb); + check(priv, ela, elb); return ela->value - elb->value; } -static int __init list_sort_test(void) +static void __init test_list_sort(struct kunit *context) { - int i, count = 1, err = -ENOMEM; + int i, count = 1; struct debug_el *el; struct list_head *cur; LIST_HEAD(head); - pr_debug("start testing list_sort()\n"); - elts = kcalloc(TEST_LIST_LEN, sizeof(*elts), GFP_KERNEL); - if (!elts) - return err; + KUNIT_ASSERT_FALSE_MSG(context, elts == NULL, "kcalloc failed"); for (i = 0; i < TEST_LIST_LEN; i++) { el = kmalloc(sizeof(*el), GFP_KERNEL); - if (!el) + if (!el) { + KUNIT_FAIL(context, "kmalloc failed"); goto exit; + } /* force some equivalencies */ el->value = prandom_u32() % (TEST_LIST_LEN / 3); @@ -94,55 +90,62 @@ static int __init list_sort_test(void) list_add_tail(&el->list, &head); } - list_sort(NULL, &head, cmp); + list_sort(context, &head, cmp); - err = -EINVAL; for (cur = head.next; cur->next != &head; cur = cur->next) { struct debug_el *el1; int cmp_result; if (cur->next->prev != cur) { - pr_err("error: list is corrupted\n"); + KUNIT_FAIL(context, "list is corrupted"); goto exit; } - cmp_result = cmp(NULL, cur, cur->next); + cmp_result = cmp(context, cur, cur->next); if (cmp_result > 0) { - pr_err("error: list is not sorted\n"); + KUNIT_FAIL(context, "list is not sorted"); goto exit; } el = container_of(cur, struct debug_el, list); el1 = container_of(cur->next, struct debug_el, list); if (cmp_result == 0 && el->serial >= el1->serial) { - pr_err("error: order of equivalent elements not " - "preserved\n"); + KUNIT_FAIL(context, "order of equivalent elements not preserved"); goto exit; } - if (check(el, el1)) { - pr_err("error: element check failed\n"); + if (check(context, el, el1)) { goto exit; } count++; } if (head.prev != cur) { - pr_err("error: list is corrupted\n"); + KUNIT_FAIL(context, "list is corrupted"); goto exit; } if (count != TEST_LIST_LEN) { - pr_err("error: bad list length %d", count); + KUNIT_FAIL(context, "bad list length %d", count); goto exit; } - err = 0; exit: for (i = 0; i < TEST_LIST_LEN; i++) kfree(elts[i]); kfree(elts); - return err; } -module_init(list_sort_test); + +static struct kunit_case __refdata list_sort_test_cases[] = { + KUNIT_CASE(test_list_sort), + {} +}; + +static struct kunit_suite list_sort_test_suite = { + .name = "list-sort", + .test_cases = list_sort_test_cases, +}; + +kunit_test_suites(&list_sort_test_suite); + MODULE_LICENSE("GPL");
This adds the conversion of the runtime tests of test_list_sort, from `lib/test_list_sort.c` to KUnit tests. Signed-off-by: Vitor Massaru Iha <vitor@massaru.org> --- v2: * fix KUNIT_TEST sufix; * make kconfig help more concise; * remove obsolete commit comments; * change Kconfig entries to be more adherent to KUnit documentation; --- lib/Kconfig.debug | 24 ++++--- lib/Makefile | 2 +- lib/{test_list_sort.c => list_sort_kunit.c} | 73 +++++++++++---------- 3 files changed, 53 insertions(+), 46 deletions(-) rename lib/{test_list_sort.c => list_sort_kunit.c} (62%) base-commit: d2585f5164c298aaaed14c2c8d313cbe7bd5b253