2020-07-29 18:01:31

by Vitor Massaru Iha

[permalink] [raw]
Subject: [PATCH] lib: kunit: add bitfield test conversion to KUnit

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

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

Code Style Documentation: [0]

Signed-off-by: Vitor Massaru Iha <[email protected]>
Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
---
lib/Kconfig.debug | 23 ++++--
lib/Makefile | 2 +-
lib/{test_bitfield.c => bitfield_kunit.c} | 92 ++++++++++-------------
3 files changed, 57 insertions(+), 60 deletions(-)
rename lib/{test_bitfield.c => bitfield_kunit.c} (66%)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 9ad9210d70a1..16c5574bf103 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1986,13 +1986,6 @@ config TEST_BITMAP

If unsure, say N.

-config TEST_BITFIELD
- tristate "Test bitfield functions at runtime"
- help
- Enable this option to test the bitfield functions at boot.
-
- If unsure, say N.
-
config TEST_UUID
tristate "Test functions located in the uuid module at runtime"

@@ -2142,6 +2135,22 @@ config TEST_SYSCTL

If unsure, say N.

+config BITFIELD_KUNIT
+ tristate "KUnit test bitfield functions at runtime"
+ depends on KUNIT
+ help
+ Enable this option to test the bitfield functions at boot.
+
+ 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 SYSCTL_KUNIT_TEST
tristate "KUnit test for sysctl" if !KUNIT_ALL_TESTS
depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index b1c42c10073b..56019c34f5ed 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -84,7 +84,6 @@ obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
obj-$(CONFIG_TEST_PRINTF) += test_printf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
obj-$(CONFIG_TEST_STRSCPY) += test_strscpy.o
-obj-$(CONFIG_TEST_BITFIELD) += test_bitfield.o
obj-$(CONFIG_TEST_UUID) += test_uuid.o
obj-$(CONFIG_TEST_XARRAY) += test_xarray.o
obj-$(CONFIG_TEST_PARMAN) += test_parman.o
@@ -316,5 +315,6 @@ obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
obj-$(CONFIG_OBJAGG) += objagg.o

# KUnit tests
+obj-$(CONFIG_BITFIELD_KUNIT) += bitfield_kunit.o
obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
diff --git a/lib/test_bitfield.c b/lib/bitfield_kunit.c
similarity index 66%
rename from lib/test_bitfield.c
rename to lib/bitfield_kunit.c
index 5b8f4108662d..d63a2be5aff8 100644
--- a/lib/test_bitfield.c
+++ b/lib/bitfield_kunit.c
@@ -5,8 +5,7 @@

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

-#include <linux/kernel.h>
-#include <linux/module.h>
+#include <kunit/test.h>
#include <linux/bitfield.h>

#define CHECK_ENC_GET_U(tp, v, field, res) do { \
@@ -14,13 +13,11 @@
u##tp _res; \
\
_res = u##tp##_encode_bits(v, field); \
- if (_res != res) { \
- pr_warn("u" #tp "_encode_bits(" #v ", " #field ") is 0x%llx != " #res "\n",\
- (u64)_res); \
- return -EINVAL; \
- } \
- if (u##tp##_get_bits(_res, field) != v) \
- return -EINVAL; \
+ KUNIT_ASSERT_FALSE_MSG(context, _res != res, \
+ "u" #tp "_encode_bits(" #v ", " #field ") is 0x%llx != " #res "\n", \
+ (u64)_res); \
+ KUNIT_ASSERT_FALSE(context, \
+ u##tp##_get_bits(_res, field) != v); \
} \
} while (0)

@@ -29,14 +26,13 @@
__le##tp _res; \
\
_res = le##tp##_encode_bits(v, field); \
- if (_res != cpu_to_le##tp(res)) { \
- pr_warn("le" #tp "_encode_bits(" #v ", " #field ") is 0x%llx != 0x%llx\n",\
- (u64)le##tp##_to_cpu(_res), \
- (u64)(res)); \
- return -EINVAL; \
- } \
- if (le##tp##_get_bits(_res, field) != v) \
- return -EINVAL; \
+ KUNIT_ASSERT_FALSE_MSG(context, \
+ _res != cpu_to_le##tp(res), \
+ "le" #tp "_encode_bits(" #v ", " #field ") is 0x%llx != 0x%llx",\
+ (u64)le##tp##_to_cpu(_res), \
+ (u64)(res)); \
+ KUNIT_ASSERT_FALSE(context, \
+ le##tp##_get_bits(_res, field) != v);\
} \
} while (0)

@@ -45,14 +41,13 @@
__be##tp _res; \
\
_res = be##tp##_encode_bits(v, field); \
- if (_res != cpu_to_be##tp(res)) { \
- pr_warn("be" #tp "_encode_bits(" #v ", " #field ") is 0x%llx != 0x%llx\n",\
- (u64)be##tp##_to_cpu(_res), \
- (u64)(res)); \
- return -EINVAL; \
- } \
- if (be##tp##_get_bits(_res, field) != v) \
- return -EINVAL; \
+ KUNIT_ASSERT_FALSE_MSG(context, \
+ _res != cpu_to_be##tp(res), \
+ "be" #tp "_encode_bits(" #v ", " #field ") is 0x%llx != 0x%llx", \
+ (u64)be##tp##_to_cpu(_res), \
+ (u64)(res)); \
+ KUNIT_ASSERT_FALSE(context, \
+ be##tp##_get_bits(_res, field) != v);\
} \
} while (0)

@@ -62,7 +57,7 @@
CHECK_ENC_GET_BE(tp, v, field, res); \
} while (0)

-static int test_constants(void)
+static void __init test_bitfields_constants(struct kunit *context)
{
/*
* NOTE
@@ -95,19 +90,17 @@ static int test_constants(void)
CHECK_ENC_GET(64, 7, 0x00f0000000000000ull, 0x0070000000000000ull);
CHECK_ENC_GET(64, 14, 0x0f00000000000000ull, 0x0e00000000000000ull);
CHECK_ENC_GET(64, 15, 0xf000000000000000ull, 0xf000000000000000ull);
-
- return 0;
}

#define CHECK(tp, mask) do { \
u64 v; \
\
for (v = 0; v < 1 << hweight32(mask); v++) \
- if (tp##_encode_bits(v, mask) != v << __ffs64(mask)) \
- return -EINVAL; \
+ KUNIT_ASSERT_FALSE(context, \
+ tp##_encode_bits(v, mask) != v << __ffs64(mask));\
} while (0)

-static int test_variables(void)
+static void __init test_bitfields_variables(struct kunit *context)
{
CHECK(u8, 0x0f);
CHECK(u8, 0xf0);
@@ -130,39 +123,34 @@ static int test_variables(void)
CHECK(u64, 0x000000007f000000ull);
CHECK(u64, 0x0000000018000000ull);
CHECK(u64, 0x0000001f8000000ull);
-
- return 0;
}

-static int __init test_bitfields(void)
-{
- int ret = test_constants();
-
- if (ret) {
- pr_warn("constant tests failed!\n");
- return ret;
- }
-
- ret = test_variables();
- if (ret) {
- pr_warn("variable tests failed!\n");
- return ret;
- }

-#ifdef TEST_BITFIELD_COMPILE
+static void __init test_bitfields_compile(struct kunit *context)
+{
/* these should fail compilation */
CHECK_ENC_GET(16, 16, 0x0f00, 0x1000);
u32_encode_bits(7, 0x06000000);

/* this should at least give a warning */
u16_encode_bits(0, 0x60000);
+}
+
+static struct kunit_case __refdata bitfields_test_cases[] = {
+ KUNIT_CASE(test_bitfields_constants),
+ KUNIT_CASE(test_bitfields_variables),
+#ifdef TEST_BITFIELD_COMPILE
+ KUNIT_CASE(test_bitfields_compile),
#endif
+ {}
+};

- pr_info("tests passed\n");
+static struct kunit_suite bitfields_test_suite = {
+ .name = "bitfields",
+ .test_cases = bitfields_test_cases,
+};

- return 0;
-}
-module_init(test_bitfields)
+kunit_test_suites(&bitfields_test_suite);

MODULE_AUTHOR("Johannes Berg <[email protected]>");
MODULE_LICENSE("GPL");

base-commit: d43c7fb05765152d4d4a39a8ef957c4ea14d8847
--
2.26.2


2020-08-19 21:14:14

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH] lib: kunit: add bitfield test conversion to KUnit

On Wed, Jul 29, 2020 at 10:58 AM Vitor Massaru Iha <[email protected]> wrote:
>
> This adds the conversion of the runtime tests of test_bitfield,
> from `lib/test_bitfield.c` to KUnit tests.
>
> Please apply this commit first (linux-kselftest/kunit-fixes):
> 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
>
> Code Style Documentation: [0]
>
> Signed-off-by: Vitor Massaru Iha <[email protected]>
> Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u

Reviewed-by: Brendan Higgins <[email protected]>

Probably still want a review from Johannes though.

2020-09-17 10:03:14

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] lib: kunit: add bitfield test conversion to KUnit

On Wed, 2020-08-19 at 14:10 -0700, Brendan Higgins wrote:
> On Wed, Jul 29, 2020 at 10:58 AM Vitor Massaru Iha <[email protected]> wrote:
> > This adds the conversion of the runtime tests of test_bitfield,
> > from `lib/test_bitfield.c` to KUnit tests.
> >
> > Please apply this commit first (linux-kselftest/kunit-fixes):
> > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
> >
> > Code Style Documentation: [0]
> >
> > Signed-off-by: Vitor Massaru Iha <[email protected]>
> > Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
>
> Reviewed-by: Brendan Higgins <[email protected]>
>
> Probably still want a review from Johannes though.

Huh, sorry, this slipped through the cracks.

Yeah, don't really care, looks fine to me? I'm not familiar with the
kunit infrastructure much yet.

Not sure I see much value in converting TEST_BITFIELD_COMPILE to a
KUNIT_CASE though, because anyway it will not compile if you enable
that? IOW, just leaving the function there without any KUNIT_CASE()
reference to it should be fine and saves you an ifdef ...

johannes

2020-10-10 08:43:33

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH] lib: kunit: add bitfield test conversion to KUnit

On Thu, Sep 17, 2020 at 3:00 AM Johannes Berg <[email protected]> wrote:
>
> On Wed, 2020-08-19 at 14:10 -0700, Brendan Higgins wrote:
> > On Wed, Jul 29, 2020 at 10:58 AM Vitor Massaru Iha <[email protected]> wrote:
> > > This adds the conversion of the runtime tests of test_bitfield,
> > > from `lib/test_bitfield.c` to KUnit tests.
> > >
> > > Please apply this commit first (linux-kselftest/kunit-fixes):
> > > 3f37d14b8a3152441f36b6bc74000996679f0998 kunit: kunit_config: Fix parsing of CONFIG options with space
> > >
> > > Code Style Documentation: [0]
> > >
> > > Signed-off-by: Vitor Massaru Iha <[email protected]>
> > > Link: [0] https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
> >
> > Reviewed-by: Brendan Higgins <[email protected]>
> >
> > Probably still want a review from Johannes though.
>
> Huh, sorry, this slipped through the cracks.
>
> Yeah, don't really care, looks fine to me? I'm not familiar with the
> kunit infrastructure much yet.

Cool, well in that case I will apply it.

> Not sure I see much value in converting TEST_BITFIELD_COMPILE to a
> KUNIT_CASE though, because anyway it will not compile if you enable
> that? IOW, just leaving the function there without any KUNIT_CASE()
> reference to it should be fine and saves you an ifdef ...

Well I think it is also the case that we only want to count the test
case if it actually has everything to run; that is a point that is
somewhat up in the air. David is exploring adding the concept of
"skipped" tests to KUnit, but we don't have that yet.

Cheers!