* [Linux-kernel-mentees] [PATCH] lib: Convert test_hexdump.c to KUnit
@ 2020-08-06 9:44 ` Arpitha Raghunandan
0 siblings, 0 replies; 10+ messages in thread
From: Arpitha Raghunandan @ 2020-08-06 9:44 UTC (permalink / raw)
To: brendanhiggins, skhan, andriy.shevchenko
Cc: Arpitha Raghunandan, linux-kernel-mentees, linux-kernel,
linux-kselftest, kunit-dev
Converts test lib/test_hexdump.c to KUnit.
More information about KUnit can be found at
https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html.
KUnit provides a common framework for unit tests in the kernel.
Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com>
---
lib/Kconfig.debug | 7 ++-
lib/Makefile | 2 +-
lib/{test_hexdump.c => hexdump_kunit.c} | 81 ++++++++++---------------
3 files changed, 36 insertions(+), 54 deletions(-)
rename lib/{test_hexdump.c => hexdump_kunit.c} (74%)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index a164785c3b48..20efea177e02 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2003,9 +2003,6 @@ config ASYNC_RAID6_TEST
If unsure, say N.
-config TEST_HEXDUMP
- tristate "Test functions located in the hexdump module at runtime"
-
config TEST_STRING_HELPERS
tristate "Test functions located in the string_helpers module at runtime"
@@ -2224,6 +2221,10 @@ config LINEAR_RANGES_TEST
If unsure, say N.
+config HEXDUMP_KUNIT_TEST
+ tristate "KUnit test for functions located in the hexdump module at runtime"
+ depends on KUNIT
+
config TEST_UDELAY
tristate "udelay test driver"
help
diff --git a/lib/Makefile b/lib/Makefile
index 435f7f13b8aa..3819d42a4681 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -54,7 +54,6 @@ obj-$(CONFIG_STRING_SELFTEST) += test_string.o
obj-y += string_helpers.o
obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
obj-y += hexdump.o
-obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o
obj-y += kstrtox.o
obj-$(CONFIG_FIND_BIT_BENCHMARK) += find_bit_benchmark.o
obj-$(CONFIG_TEST_BPF) += test_bpf.o
@@ -346,3 +345,4 @@ obj-$(CONFIG_PLDMFW) += pldmfw/
# KUnit tests
obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o
+obj-$(CONFIG_HEXDUMP_KUNIT_TEST) += hexdump_kunit.o
diff --git a/lib/test_hexdump.c b/lib/hexdump_kunit.c
similarity index 74%
rename from lib/test_hexdump.c
rename to lib/hexdump_kunit.c
index 5144899d3c6b..8f8d80114a92 100644
--- a/lib/test_hexdump.c
+++ b/lib/hexdump_kunit.c
@@ -3,6 +3,7 @@
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <kunit/test.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -61,10 +62,7 @@ static const char * const test_data_8_be[] __initconst = {
#define FILL_CHAR '#'
-static unsigned total_tests __initdata;
-static unsigned failed_tests __initdata;
-
-static void __init test_hexdump_prepare_test(size_t len, int rowsize,
+static void test_hexdump_prepare_test(size_t len, int rowsize,
int groupsize, char *test,
size_t testlen, bool ascii)
{
@@ -122,14 +120,12 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
#define TEST_HEXDUMP_BUF_SIZE (32 * 3 + 2 + 32 + 1)
-static void __init test_hexdump(size_t len, int rowsize, int groupsize,
- bool ascii)
+static void test_hexdump(struct kunit *kunittest, size_t len, int rowsize,
+ int groupsize, bool ascii)
{
char test[TEST_HEXDUMP_BUF_SIZE];
char real[TEST_HEXDUMP_BUF_SIZE];
- total_tests++;
-
memset(real, FILL_CHAR, sizeof(real));
hex_dump_to_buffer(data_b, len, rowsize, groupsize, real, sizeof(real),
ascii);
@@ -138,28 +134,23 @@ static void __init test_hexdump(size_t len, int rowsize, int groupsize,
test_hexdump_prepare_test(len, rowsize, groupsize, test, sizeof(test),
ascii);
- if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) {
- pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize);
- pr_err("Result: '%s'\n", real);
- pr_err("Expect: '%s'\n", test);
- failed_tests++;
- }
+ KUNIT_EXPECT_EQ(kunittest, 0, memcmp(test, real, TEST_HEXDUMP_BUF_SIZE));
}
-static void __init test_hexdump_set(int rowsize, bool ascii)
+static void test_hexdump_set(struct kunit *test, int rowsize, bool ascii)
{
size_t d = min_t(size_t, sizeof(data_b), rowsize);
size_t len = get_random_int() % d + 1;
- test_hexdump(len, rowsize, 4, ascii);
- test_hexdump(len, rowsize, 2, ascii);
- test_hexdump(len, rowsize, 8, ascii);
- test_hexdump(len, rowsize, 1, ascii);
+ test_hexdump(test, len, rowsize, 4, ascii);
+ test_hexdump(test, len, rowsize, 2, ascii);
+ test_hexdump(test, len, rowsize, 8, ascii);
+ test_hexdump(test, len, rowsize, 1, ascii);
}
-static void __init test_hexdump_overflow(size_t buflen, size_t len,
- int rowsize, int groupsize,
- bool ascii)
+static void test_hexdump_overflow(struct kunit *kunittest, size_t buflen,
+ size_t len, int rowsize,
+ int groupsize, bool ascii)
{
char test[TEST_HEXDUMP_BUF_SIZE];
char buf[TEST_HEXDUMP_BUF_SIZE];
@@ -167,8 +158,6 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
int ae, he, e, f, r;
bool a;
- total_tests++;
-
memset(buf, FILL_CHAR, sizeof(buf));
r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii);
@@ -196,16 +185,10 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
buf[sizeof(buf) - 1] = '\0';
- if (!a) {
- pr_err("Len: %zu buflen: %zu strlen: %zu\n",
- len, buflen, strnlen(buf, sizeof(buf)));
- pr_err("Result: %d '%s'\n", r, buf);
- pr_err("Expect: %d '%s'\n", e, test);
- failed_tests++;
- }
+ KUNIT_EXPECT_NE(kunittest, 0, a);
}
-static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
+static void test_hexdump_overflow_set(struct kunit *test, size_t buflen, bool ascii)
{
unsigned int i = 0;
int rs = (get_random_int() % 2 + 1) * 16;
@@ -214,43 +197,41 @@ static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
int gs = 1 << i;
size_t len = get_random_int() % rs + gs;
- test_hexdump_overflow(buflen, rounddown(len, gs), rs, gs, ascii);
+ test_hexdump_overflow(test, buflen, rounddown(len, gs), rs, gs, ascii);
} while (i++ < 3);
}
-static int __init test_hexdump_init(void)
+static void test_hexdump_init(struct kunit *test)
{
unsigned int i;
int rowsize;
rowsize = (get_random_int() % 2 + 1) * 16;
for (i = 0; i < 16; i++)
- test_hexdump_set(rowsize, false);
+ test_hexdump_set(test, rowsize, false);
rowsize = (get_random_int() % 2 + 1) * 16;
for (i = 0; i < 16; i++)
- test_hexdump_set(rowsize, true);
+ test_hexdump_set(test, rowsize, true);
for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
- test_hexdump_overflow_set(i, false);
+ test_hexdump_overflow_set(test, i, false);
for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
- test_hexdump_overflow_set(i, true);
+ test_hexdump_overflow_set(test, i, true);
+}
- if (failed_tests == 0)
- pr_info("all %u tests passed\n", total_tests);
- else
- pr_err("failed %u out of %u tests\n", failed_tests, total_tests);
+static struct kunit_case hexdump_test_cases[] = {
+ KUNIT_CASE(test_hexdump_init),
+ {}
+};
- return failed_tests ? -EINVAL : 0;
-}
-module_init(test_hexdump_init);
+static struct kunit_suite hexdump_test_suite = {
+ .name = "hexdump-kunit-test",
+ .test_cases = hexdump_test_cases,
+};
-static void __exit test_hexdump_exit(void)
-{
- /* do nothing */
-}
-module_exit(test_hexdump_exit);
+kunit_test_suite(hexdump_test_suite);
MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>");
MODULE_LICENSE("Dual BSD/GPL");
--
2.25.1
_______________________________________________
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] 10+ messages in thread* [PATCH] lib: Convert test_hexdump.c to KUnit @ 2020-08-06 9:44 ` Arpitha Raghunandan 0 siblings, 0 replies; 10+ messages in thread From: Arpitha Raghunandan @ 2020-08-06 9:44 UTC (permalink / raw) To: brendanhiggins, skhan, andriy.shevchenko Cc: Arpitha Raghunandan, kunit-dev, linux-kselftest, linux-kernel, linux-kernel-mentees Converts test lib/test_hexdump.c to KUnit. More information about KUnit can be found at https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. KUnit provides a common framework for unit tests in the kernel. Signed-off-by: Arpitha Raghunandan <98.arpi@gmail.com> --- lib/Kconfig.debug | 7 ++- lib/Makefile | 2 +- lib/{test_hexdump.c => hexdump_kunit.c} | 81 ++++++++++--------------- 3 files changed, 36 insertions(+), 54 deletions(-) rename lib/{test_hexdump.c => hexdump_kunit.c} (74%) diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index a164785c3b48..20efea177e02 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2003,9 +2003,6 @@ config ASYNC_RAID6_TEST If unsure, say N. -config TEST_HEXDUMP - tristate "Test functions located in the hexdump module at runtime" - config TEST_STRING_HELPERS tristate "Test functions located in the string_helpers module at runtime" @@ -2224,6 +2221,10 @@ config LINEAR_RANGES_TEST If unsure, say N. +config HEXDUMP_KUNIT_TEST + tristate "KUnit test for functions located in the hexdump module at runtime" + depends on KUNIT + config TEST_UDELAY tristate "udelay test driver" help diff --git a/lib/Makefile b/lib/Makefile index 435f7f13b8aa..3819d42a4681 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -54,7 +54,6 @@ obj-$(CONFIG_STRING_SELFTEST) += test_string.o obj-y += string_helpers.o obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o obj-y += hexdump.o -obj-$(CONFIG_TEST_HEXDUMP) += test_hexdump.o obj-y += kstrtox.o obj-$(CONFIG_FIND_BIT_BENCHMARK) += find_bit_benchmark.o obj-$(CONFIG_TEST_BPF) += test_bpf.o @@ -346,3 +345,4 @@ obj-$(CONFIG_PLDMFW) += pldmfw/ # KUnit tests obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o obj-$(CONFIG_LINEAR_RANGES_TEST) += test_linear_ranges.o +obj-$(CONFIG_HEXDUMP_KUNIT_TEST) += hexdump_kunit.o diff --git a/lib/test_hexdump.c b/lib/hexdump_kunit.c similarity index 74% rename from lib/test_hexdump.c rename to lib/hexdump_kunit.c index 5144899d3c6b..8f8d80114a92 100644 --- a/lib/test_hexdump.c +++ b/lib/hexdump_kunit.c @@ -3,6 +3,7 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <kunit/test.h> #include <linux/init.h> #include <linux/kernel.h> #include <linux/module.h> @@ -61,10 +62,7 @@ static const char * const test_data_8_be[] __initconst = { #define FILL_CHAR '#' -static unsigned total_tests __initdata; -static unsigned failed_tests __initdata; - -static void __init test_hexdump_prepare_test(size_t len, int rowsize, +static void test_hexdump_prepare_test(size_t len, int rowsize, int groupsize, char *test, size_t testlen, bool ascii) { @@ -122,14 +120,12 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize, #define TEST_HEXDUMP_BUF_SIZE (32 * 3 + 2 + 32 + 1) -static void __init test_hexdump(size_t len, int rowsize, int groupsize, - bool ascii) +static void test_hexdump(struct kunit *kunittest, size_t len, int rowsize, + int groupsize, bool ascii) { char test[TEST_HEXDUMP_BUF_SIZE]; char real[TEST_HEXDUMP_BUF_SIZE]; - total_tests++; - memset(real, FILL_CHAR, sizeof(real)); hex_dump_to_buffer(data_b, len, rowsize, groupsize, real, sizeof(real), ascii); @@ -138,28 +134,23 @@ static void __init test_hexdump(size_t len, int rowsize, int groupsize, test_hexdump_prepare_test(len, rowsize, groupsize, test, sizeof(test), ascii); - if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) { - pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize); - pr_err("Result: '%s'\n", real); - pr_err("Expect: '%s'\n", test); - failed_tests++; - } + KUNIT_EXPECT_EQ(kunittest, 0, memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)); } -static void __init test_hexdump_set(int rowsize, bool ascii) +static void test_hexdump_set(struct kunit *test, int rowsize, bool ascii) { size_t d = min_t(size_t, sizeof(data_b), rowsize); size_t len = get_random_int() % d + 1; - test_hexdump(len, rowsize, 4, ascii); - test_hexdump(len, rowsize, 2, ascii); - test_hexdump(len, rowsize, 8, ascii); - test_hexdump(len, rowsize, 1, ascii); + test_hexdump(test, len, rowsize, 4, ascii); + test_hexdump(test, len, rowsize, 2, ascii); + test_hexdump(test, len, rowsize, 8, ascii); + test_hexdump(test, len, rowsize, 1, ascii); } -static void __init test_hexdump_overflow(size_t buflen, size_t len, - int rowsize, int groupsize, - bool ascii) +static void test_hexdump_overflow(struct kunit *kunittest, size_t buflen, + size_t len, int rowsize, + int groupsize, bool ascii) { char test[TEST_HEXDUMP_BUF_SIZE]; char buf[TEST_HEXDUMP_BUF_SIZE]; @@ -167,8 +158,6 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, int ae, he, e, f, r; bool a; - total_tests++; - memset(buf, FILL_CHAR, sizeof(buf)); r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii); @@ -196,16 +185,10 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, buf[sizeof(buf) - 1] = '\0'; - if (!a) { - pr_err("Len: %zu buflen: %zu strlen: %zu\n", - len, buflen, strnlen(buf, sizeof(buf))); - pr_err("Result: %d '%s'\n", r, buf); - pr_err("Expect: %d '%s'\n", e, test); - failed_tests++; - } + KUNIT_EXPECT_NE(kunittest, 0, a); } -static void __init test_hexdump_overflow_set(size_t buflen, bool ascii) +static void test_hexdump_overflow_set(struct kunit *test, size_t buflen, bool ascii) { unsigned int i = 0; int rs = (get_random_int() % 2 + 1) * 16; @@ -214,43 +197,41 @@ static void __init test_hexdump_overflow_set(size_t buflen, bool ascii) int gs = 1 << i; size_t len = get_random_int() % rs + gs; - test_hexdump_overflow(buflen, rounddown(len, gs), rs, gs, ascii); + test_hexdump_overflow(test, buflen, rounddown(len, gs), rs, gs, ascii); } while (i++ < 3); } -static int __init test_hexdump_init(void) +static void test_hexdump_init(struct kunit *test) { unsigned int i; int rowsize; rowsize = (get_random_int() % 2 + 1) * 16; for (i = 0; i < 16; i++) - test_hexdump_set(rowsize, false); + test_hexdump_set(test, rowsize, false); rowsize = (get_random_int() % 2 + 1) * 16; for (i = 0; i < 16; i++) - test_hexdump_set(rowsize, true); + test_hexdump_set(test, rowsize, true); for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++) - test_hexdump_overflow_set(i, false); + test_hexdump_overflow_set(test, i, false); for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++) - test_hexdump_overflow_set(i, true); + test_hexdump_overflow_set(test, i, true); +} - if (failed_tests == 0) - pr_info("all %u tests passed\n", total_tests); - else - pr_err("failed %u out of %u tests\n", failed_tests, total_tests); +static struct kunit_case hexdump_test_cases[] = { + KUNIT_CASE(test_hexdump_init), + {} +}; - return failed_tests ? -EINVAL : 0; -} -module_init(test_hexdump_init); +static struct kunit_suite hexdump_test_suite = { + .name = "hexdump-kunit-test", + .test_cases = hexdump_test_cases, +}; -static void __exit test_hexdump_exit(void) -{ - /* do nothing */ -} -module_exit(test_hexdump_exit); +kunit_test_suite(hexdump_test_suite); MODULE_AUTHOR("Andy Shevchenko <andriy.shevchenko@linux.intel.com>"); MODULE_LICENSE("Dual BSD/GPL"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] lib: Convert test_hexdump.c to KUnit 2020-08-06 9:44 ` Arpitha Raghunandan @ 2020-08-06 9:52 ` Andy Shevchenko -1 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2020-08-06 9:52 UTC (permalink / raw) To: Arpitha Raghunandan Cc: Brendan Higgins, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, Andy Shevchenko, linux-kernel-mentees, kunit-dev On Thu, Aug 6, 2020 at 12:48 PM Arpitha Raghunandan <98.arpi@gmail.com> wrote: > > Converts test lib/test_hexdump.c to KUnit. > More information about KUnit can be found at > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > KUnit provides a common framework for unit tests in the kernel. > -config TEST_HEXDUMP > - tristate "Test functions located in the hexdump module at runtime" We have a nice collection of tests starting with TEST_ in the configuration, now it's gone. I'm strongly against this change. Code itself okay, but without addressing above - NAK. -- With Best Regards, Andy Shevchenko _______________________________________________ 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] 10+ messages in thread
* Re: [PATCH] lib: Convert test_hexdump.c to KUnit @ 2020-08-06 9:52 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2020-08-06 9:52 UTC (permalink / raw) To: Arpitha Raghunandan Cc: Brendan Higgins, Shuah Khan, Andy Shevchenko, kunit-dev, open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List, linux-kernel-mentees On Thu, Aug 6, 2020 at 12:48 PM Arpitha Raghunandan <98.arpi@gmail.com> wrote: > > Converts test lib/test_hexdump.c to KUnit. > More information about KUnit can be found at > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > KUnit provides a common framework for unit tests in the kernel. > -config TEST_HEXDUMP > - tristate "Test functions located in the hexdump module at runtime" We have a nice collection of tests starting with TEST_ in the configuration, now it's gone. I'm strongly against this change. Code itself okay, but without addressing above - NAK. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] lib: Convert test_hexdump.c to KUnit 2020-08-06 9:52 ` Andy Shevchenko @ 2020-08-07 21:33 ` David Gow -1 siblings, 0 replies; 10+ messages in thread From: David Gow via Linux-kernel-mentees @ 2020-08-07 21:33 UTC (permalink / raw) To: Andy Shevchenko Cc: Arpitha Raghunandan, Brendan Higgins, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, Andy Shevchenko, linux-kernel-mentees, KUnit Development On Thu, Aug 6, 2020 at 5:53 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Thu, Aug 6, 2020 at 12:48 PM Arpitha Raghunandan <98.arpi@gmail.com> wrote: > > > > Converts test lib/test_hexdump.c to KUnit. > > More information about KUnit can be found at > > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > > KUnit provides a common framework for unit tests in the kernel. > > > -config TEST_HEXDUMP > > - tristate "Test functions located in the hexdump module at runtime" > > We have a nice collection of tests starting with TEST_ in the > configuration, now it's gone. > I'm strongly against this change. > Code itself okay, but without addressing above - NAK. > This change is to make the test naming compliant with the proposed KUnit test naming guidelines: - https://lore.kernel.org/linux-kselftest/20200702071416.1780522-1-davidgow@google.com/ The hope is that tests built on KUnit will all end up with the same [x]_KUNIT_TEST config options (which at least preserves the consistency of the test naming, even if they'll not all sort together), and should make it easier for people to know that the test results will be in a common format, and that the test can also be run using the KUnit tools. The naming guidelines haven't been upstreamed yet, though, so we'd definitely appreciate input on that thread if you've got comments more broadly than for this particular patch. Ultimately, I don't think it matters too much what we end up using, but having some consistency is the goal. _______________________________________________ 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] 10+ messages in thread
* Re: [PATCH] lib: Convert test_hexdump.c to KUnit @ 2020-08-07 21:33 ` David Gow 0 siblings, 0 replies; 10+ messages in thread From: David Gow @ 2020-08-07 21:33 UTC (permalink / raw) To: Andy Shevchenko Cc: Arpitha Raghunandan, Brendan Higgins, Shuah Khan, Andy Shevchenko, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List, linux-kernel-mentees On Thu, Aug 6, 2020 at 5:53 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Thu, Aug 6, 2020 at 12:48 PM Arpitha Raghunandan <98.arpi@gmail.com> wrote: > > > > Converts test lib/test_hexdump.c to KUnit. > > More information about KUnit can be found at > > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > > KUnit provides a common framework for unit tests in the kernel. > > > -config TEST_HEXDUMP > > - tristate "Test functions located in the hexdump module at runtime" > > We have a nice collection of tests starting with TEST_ in the > configuration, now it's gone. > I'm strongly against this change. > Code itself okay, but without addressing above - NAK. > This change is to make the test naming compliant with the proposed KUnit test naming guidelines: - https://lore.kernel.org/linux-kselftest/20200702071416.1780522-1-davidgow@google.com/ The hope is that tests built on KUnit will all end up with the same [x]_KUNIT_TEST config options (which at least preserves the consistency of the test naming, even if they'll not all sort together), and should make it easier for people to know that the test results will be in a common format, and that the test can also be run using the KUnit tools. The naming guidelines haven't been upstreamed yet, though, so we'd definitely appreciate input on that thread if you've got comments more broadly than for this particular patch. Ultimately, I don't think it matters too much what we end up using, but having some consistency is the goal. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] lib: Convert test_hexdump.c to KUnit 2020-08-06 9:44 ` Arpitha Raghunandan @ 2020-08-06 10:05 ` Andy Shevchenko -1 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2020-08-06 10:05 UTC (permalink / raw) To: Arpitha Raghunandan Cc: brendanhiggins, linux-kernel, linux-kselftest, linux-kernel-mentees, kunit-dev On Thu, Aug 06, 2020 at 03:14:40PM +0530, Arpitha Raghunandan wrote: > Converts test lib/test_hexdump.c to KUnit. > More information about KUnit can be found at > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > KUnit provides a common framework for unit tests in the kernel. ... > - if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) { > - pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize); > - pr_err("Result: '%s'\n", real); > - pr_err("Expect: '%s'\n", test); > - failed_tests++; > - } > + KUNIT_EXPECT_EQ(kunittest, 0, memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)); Ah, can you explain how user will see now what is being expected and what is in reality in the buffer? I'm not gonna accept such changes without showing in explicitly that user is not going to suffer of this change. -- With Best Regards, Andy Shevchenko _______________________________________________ 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] 10+ messages in thread
* Re: [PATCH] lib: Convert test_hexdump.c to KUnit @ 2020-08-06 10:05 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2020-08-06 10:05 UTC (permalink / raw) To: Arpitha Raghunandan Cc: brendanhiggins, skhan, kunit-dev, linux-kselftest, linux-kernel, linux-kernel-mentees On Thu, Aug 06, 2020 at 03:14:40PM +0530, Arpitha Raghunandan wrote: > Converts test lib/test_hexdump.c to KUnit. > More information about KUnit can be found at > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. > KUnit provides a common framework for unit tests in the kernel. ... > - if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) { > - pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize); > - pr_err("Result: '%s'\n", real); > - pr_err("Expect: '%s'\n", test); > - failed_tests++; > - } > + KUNIT_EXPECT_EQ(kunittest, 0, memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)); Ah, can you explain how user will see now what is being expected and what is in reality in the buffer? I'm not gonna accept such changes without showing in explicitly that user is not going to suffer of this change. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH] lib: Convert test_hexdump.c to KUnit 2020-08-06 10:05 ` Andy Shevchenko @ 2020-08-09 15:11 ` Arpitha Raghunandan -1 siblings, 0 replies; 10+ messages in thread From: Arpitha Raghunandan @ 2020-08-09 15:11 UTC (permalink / raw) To: Andy Shevchenko Cc: brendanhiggins, linux-kernel, linux-kselftest, linux-kernel-mentees, kunit-dev On 06/08/20 3:35 pm, Andy Shevchenko wrote: > On Thu, Aug 06, 2020 at 03:14:40PM +0530, Arpitha Raghunandan wrote: >> Converts test lib/test_hexdump.c to KUnit. >> More information about KUnit can be found at >> https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. >> KUnit provides a common framework for unit tests in the kernel. > > ... > >> - if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) { >> - pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize); >> - pr_err("Result: '%s'\n", real); >> - pr_err("Expect: '%s'\n", test); >> - failed_tests++; >> - } >> + KUNIT_EXPECT_EQ(kunittest, 0, memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)); > > > Ah, can you explain how user will see now what is being expected and what is in > reality in the buffer? I'm not gonna accept such changes without showing in > explicitly that user is not going to suffer of this change. > I have sent another patch replacing KUNIT_EXPECT_EQ() with KUNIT_EXPECT_EQ_MSG() and KUNIT_EXPECT_NE() with KUNIT_EXPECT_NE_MSG(). These methods log what is being expected and what is in reality in the buffer in case of test failure similar to the original test. _______________________________________________ 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] 10+ messages in thread
* Re: [PATCH] lib: Convert test_hexdump.c to KUnit @ 2020-08-09 15:11 ` Arpitha Raghunandan 0 siblings, 0 replies; 10+ messages in thread From: Arpitha Raghunandan @ 2020-08-09 15:11 UTC (permalink / raw) To: Andy Shevchenko Cc: brendanhiggins, skhan, kunit-dev, linux-kselftest, linux-kernel, linux-kernel-mentees On 06/08/20 3:35 pm, Andy Shevchenko wrote: > On Thu, Aug 06, 2020 at 03:14:40PM +0530, Arpitha Raghunandan wrote: >> Converts test lib/test_hexdump.c to KUnit. >> More information about KUnit can be found at >> https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html. >> KUnit provides a common framework for unit tests in the kernel. > > ... > >> - if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) { >> - pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize); >> - pr_err("Result: '%s'\n", real); >> - pr_err("Expect: '%s'\n", test); >> - failed_tests++; >> - } >> + KUNIT_EXPECT_EQ(kunittest, 0, memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)); > > > Ah, can you explain how user will see now what is being expected and what is in > reality in the buffer? I'm not gonna accept such changes without showing in > explicitly that user is not going to suffer of this change. > I have sent another patch replacing KUNIT_EXPECT_EQ() with KUNIT_EXPECT_EQ_MSG() and KUNIT_EXPECT_NE() with KUNIT_EXPECT_NE_MSG(). These methods log what is being expected and what is in reality in the buffer in case of test failure similar to the original test. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-08-09 15:12 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-06 9:44 [Linux-kernel-mentees] [PATCH] lib: Convert test_hexdump.c to KUnit Arpitha Raghunandan 2020-08-06 9:44 ` Arpitha Raghunandan 2020-08-06 9:52 ` [Linux-kernel-mentees] " Andy Shevchenko 2020-08-06 9:52 ` Andy Shevchenko 2020-08-07 21:33 ` [Linux-kernel-mentees] " David Gow via Linux-kernel-mentees 2020-08-07 21:33 ` David Gow 2020-08-06 10:05 ` [Linux-kernel-mentees] " Andy Shevchenko 2020-08-06 10:05 ` Andy Shevchenko 2020-08-09 15:11 ` [Linux-kernel-mentees] " Arpitha Raghunandan 2020-08-09 15:11 ` Arpitha Raghunandan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.