This adds the conversion of the test_sort.c to KUnit test.
Please apply this commit first (linux-kselftest/kunit-fixes): 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
Code Style Documentation: [0]
Fix these warnings Reported-by lkp@intel.com
WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test The variable sort_test_cases references the variable __init sort_test If the reference is valid then annotate the variable with or __refdata (see linux/init.h) or name the variable
WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test() The variable sort_test_cases references the function __init sort_test()
Signed-off-by: Vitor Massaru Iha vitor@massaru.org Reported-by: kernel test robot lkp@intel.com Link: [0] https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@goo... --- v2: * Add Kunit Code Style reference in commit message; * Fix lkp@intel.com warning report; --- lib/Kconfig.debug | 26 +++++++++++++++++--------- lib/Makefile | 2 +- lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++---------------- 3 files changed, 33 insertions(+), 26 deletions(-) rename lib/{test_sort.c => sort_kunit.c} (55%)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 9ad9210d70a1..1fe19e78d7ca 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -1874,15 +1874,6 @@ config TEST_MIN_HEAP
If unsure, say N.
-config TEST_SORT - tristate "Array-based sort test" - depends on DEBUG_KERNEL || m - help - This option enables the self-test function of 'sort()' at boot, - or at module load time. - - If unsure, say N. - config KPROBES_SANITY_TEST bool "Kprobes sanity tests" depends on DEBUG_KERNEL @@ -2185,6 +2176,23 @@ config LINEAR_RANGES_TEST
If unsure, say N.
+config SORT_KUNIT + tristate "KUnit test for Array-based sort" + depends on DEBUG_KERNEL || m + help + This option enables the KUnit function of 'sort()' at boot, + 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..c22bb13b0a08 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -77,7 +77,6 @@ obj-$(CONFIG_TEST_LKM) += test_module.o obj-$(CONFIG_TEST_VMALLOC) += test_vmalloc.o obj-$(CONFIG_TEST_OVERFLOW) += test_overflow.o obj-$(CONFIG_TEST_RHASHTABLE) += test_rhashtable.o -obj-$(CONFIG_TEST_SORT) += test_sort.o obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.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_SORT_KUNIT) += sort_kunit.o diff --git a/lib/test_sort.c b/lib/sort_kunit.c similarity index 55% rename from lib/test_sort.c rename to lib/sort_kunit.c index 52edbe10f2e5..602a234f1e7d 100644 --- a/lib/test_sort.c +++ b/lib/sort_kunit.c @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only #include <linux/sort.h> -#include <linux/slab.h> -#include <linux/module.h> +#include <kunit/test.h>
/* a simple boot-time regression test */
@@ -12,13 +11,12 @@ static int __init cmpint(const void *a, const void *b) return *(int *)a - *(int *)b; }
-static int __init test_sort_init(void) +static void __init sort_test(struct kunit *test) { - int *a, i, r = 1, err = -ENOMEM; + int *a, i, r = 1;
a = kmalloc_array(TEST_LEN, sizeof(*a), GFP_KERNEL); - if (!a) - return err; + KUNIT_ASSERT_FALSE_MSG(test, a == NULL, "kmalloc_array failed");
for (i = 0; i < TEST_LEN; i++) { r = (r * 725861) % 6599; @@ -27,24 +25,25 @@ static int __init test_sort_init(void)
sort(a, TEST_LEN, sizeof(*a), cmpint, NULL);
- err = -EINVAL; for (i = 0; i < TEST_LEN-1; i++) if (a[i] > a[i+1]) { - pr_err("test has failed\n"); + KUNIT_FAIL(test, "test has failed"); goto exit; } - err = 0; - pr_info("test passed\n"); exit: kfree(a); - return err; }
-static void __exit test_sort_exit(void) -{ -} +static struct kunit_case __refdata sort_test_cases[] = { + KUNIT_CASE(sort_test), + {} +}; + +static struct kunit_suite sort_test_suite = { + .name = "sort", + .test_cases = sort_test_cases, +};
-module_init(test_sort_init); -module_exit(test_sort_exit); +kunit_test_suites(&sort_test_suite);
MODULE_LICENSE("GPL");
base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847 -- 2.26.2
On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
This adds the conversion of the test_sort.c to KUnit test.
Please apply this commit first (linux-kselftest/kunit-fixes): 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
Looks like you mixed up commit message and changelog / comments.
Code Style Documentation: [0]
Fix these warnings Reported-by lkp@intel.com
WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test The variable sort_test_cases references the variable __init sort_test If the reference is valid then annotate the variable with or __refdata (see linux/init.h) or name the variable
WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test() The variable sort_test_cases references the function __init sort_test()
Signed-off-by: Vitor Massaru Iha vitor@massaru.org Reported-by: kernel test robot lkp@intel.com Link: [0] https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@goo...
This should be in different order: Link, Reported-by, SoB. Also [0] is unnecessary
lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
Still opened question why kunit is a suffix? Can't we leave same name? Can't we do it rather prefix?
On Wed, Jul 29, 2020 at 12:19 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
This adds the conversion of the test_sort.c to KUnit test.
Please apply this commit first (linux-kselftest/kunit-fixes): 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
Looks like you mixed up commit message and changelog / comments.
Code Style Documentation: [0]
Fix these warnings Reported-by lkp@intel.com
WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test The variable sort_test_cases references the variable __init sort_test If the reference is valid then annotate the variable with or __refdata (see linux/init.h) or name the variable
WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test() The variable sort_test_cases references the function __init sort_test()
Signed-off-by: Vitor Massaru Iha vitor@massaru.org Reported-by: kernel test robot lkp@intel.com Link: [0] https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@goo...
This should be in different order: Link, Reported-by, SoB. Also [0] is unnecessary
lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
Still opened question why kunit is a suffix? Can't we leave same name? Can't we do it rather prefix?
Sorry to jump in now; I thought Vitor's reply with a link to the new proposed documentation[1] addressed this. The naming convention issue came up about a month ago[2]. The people in the discussion (including myself) came to an agreement on _kunit.c. That being said, the documentation hasn't been accepted yet, so if you really feel strongly about it, now is the time to change it.
All that being said, I would rather not discuss that issue here for the benefit of the participants in the preceding discussions.
I posted lore links for the relevant threads, which should be easy enough to In-Reply-To your way into. Nevertheless, if it makes it easier, let me know and I can CC you into the discussions.
[1] https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@goo... [2] https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
Cheers
On Wed, Jul 29, 2020 at 12:59:28PM -0700, Brendan Higgins wrote:
On Wed, Jul 29, 2020 at 12:19 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
...
lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
Still opened question why kunit is a suffix? Can't we leave same name? Can't we do it rather prefix?
Sorry to jump in now; I thought Vitor's reply with a link to the new proposed documentation[1] addressed this. The naming convention issue came up about a month ago[2]. The people in the discussion (including myself) came to an agreement on _kunit.c. That being said, the documentation hasn't been accepted yet, so if you really feel strongly about it, now is the time to change it.
My argument is to do something like
- ls .../test* vs. ls .../*_kunit.c
- use shell completion vs. no completion when looking if there is a test module for something
But I agree that this is matter of style.
All that being said, I would rather not discuss that issue here for the benefit of the participants in the preceding discussions.
I posted lore links for the relevant threads, which should be easy enough to In-Reply-To your way into. Nevertheless, if it makes it easier, let me know and I can CC you into the discussions.
No need. I think you have enough clever folks and good ideas behind this. Just put a reference to all these conversion patches to the summary of pros and cons of renaming.
[1] https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@goo... [2] https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
On Wed, Jul 29, 2020 at 4:19 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Wed, Jul 29, 2020 at 04:11:51PM -0300, Vitor Massaru Iha wrote:
This adds the conversion of the test_sort.c to KUnit test.
Please apply this commit first (linux-kselftest/kunit-fixes): 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
Looks like you mixed up commit message and changelog / comments.
Code Style Documentation: [0]
Fix these warnings Reported-by lkp@intel.com
WARNING: modpost: vmlinux.o(.data+0x4fc70): Section mismatch in reference from the variable sort_test_cases to the variable .init.text:sort_test The variable sort_test_cases references the variable __init sort_test If the reference is valid then annotate the variable with or __refdata (see linux/init.h) or name the variable
WARNING: modpost: lib/sort_kunit.o(.data+0x11c): Section mismatch in reference from the variable sort_test_cases to the function .init.text:sort_test() The variable sort_test_cases references the function __init sort_test()
Signed-off-by: Vitor Massaru Iha vitor@massaru.org Reported-by: kernel test robot lkp@intel.com Link: [0] https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@goo...
This should be in different order: Link, Reported-by, SoB. Also [0] is unnecessary
Sure, thanks!
lib/{test_sort.c => sort_kunit.c} | 31 +++++++++++++++----------------
Still opened question why kunit is a suffix? Can't we leave same name? Can't we do it rather prefix?
-- With Best Regards, Andy Shevchenko
Hi Vitor,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on d43c7fb05765152d4d4a39a8ef957c4ea14d8847]
url: https://github.com/0day-ci/linux/commits/Vitor-Massaru-Iha/lib-kunit-Convert... base: d43c7fb05765152d4d4a39a8ef957c4ea14d8847 config: x86_64-randconfig-s022-20200803 (attached as .config) compiler: gcc-9 (Debian 9.3.0-14) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-117-g8c7aee71-dirty # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
ld: lib/sort_kunit.o: in function `kunit_test_suites_init':
lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_init'
ld: lib/sort_kunit.o: in function `sort_test':
lib/sort_kunit.c:19: undefined reference to `kunit_unary_assert_format' ld: lib/sort_kunit.c:19: undefined reference to `kunit_do_assertion' ld: lib/sort_kunit.c:30: undefined reference to `kunit_fail_assert_format'
ld: lib/sort_kunit.c:30: undefined reference to `kunit_do_assertion' ld: lib/sort_kunit.o: in function `kunit_test_suites_exit':
lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_exit'
vim +47 lib/sort_kunit.c
13 14 static void __init sort_test(struct kunit *test) 15 { 16 int *a, i, r = 1; 17 18 a = kmalloc_array(TEST_LEN, sizeof(*a), GFP_KERNEL);
19 KUNIT_ASSERT_FALSE_MSG(test, a == NULL, "kmalloc_array failed");
20 21 for (i = 0; i < TEST_LEN; i++) { 22 r = (r * 725861) % 6599; 23 a[i] = r; 24 } 25 26 sort(a, TEST_LEN, sizeof(*a), cmpint, NULL); 27 28 for (i = 0; i < TEST_LEN-1; i++) 29 if (a[i] > a[i+1]) {
30 KUNIT_FAIL(test, "test has failed");
31 goto exit; 32 } 33 exit: 34 kfree(a); 35 } 36 37 static struct kunit_case __refdata sort_test_cases[] = { 38 KUNIT_CASE(sort_test), 39 {} 40 }; 41 42 static struct kunit_suite sort_test_suite = { 43 .name = "sort", 44 .test_cases = sort_test_cases, 45 }; 46
47 kunit_test_suites(&sort_test_suite);
48
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Vitor,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on d43c7fb05765152d4d4a39a8ef957c4ea14d8847]
url: https://github.com/0day-ci/linux/commits/Vitor-Massaru-Iha/lib-kunit-Convert... base: d43c7fb05765152d4d4a39a8ef957c4ea14d8847 config: microblaze-randconfig-s031-20200803 (attached as .config) compiler: microblaze-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.2-117-g8c7aee71-dirty # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=microblaze
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
microblaze-linux-ld: lib/sort_kunit.o: in function `kunit_test_suites_init': lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_init' microblaze-linux-ld: lib/sort_kunit.o: in function `sort_test': lib/sort_kunit.c:19: undefined reference to `kunit_unary_assert_format'
microblaze-linux-ld: lib/sort_kunit.c:19: undefined reference to `kunit_do_assertion' microblaze-linux-ld: lib/sort_kunit.c:30: undefined reference to `kunit_fail_assert_format'
microblaze-linux-ld: lib/sort_kunit.c:30: undefined reference to `kunit_do_assertion' microblaze-linux-ld: lib/sort_kunit.o: in function `kunit_test_suites_exit': lib/sort_kunit.c:47: undefined reference to `__kunit_test_suites_exit'
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
linux-kselftest-mirror@lists.linaro.org