On Thu 2020-10-22 20:43:49, Arpitha Raghunandan wrote:
> Convert test lib/test_printf.c to KUnit. More information about
> Converted test success:
> # Subtest: printf-kunit-test
> 1..1
> ok 1 - selftest
> ok 1 - printf-kunit-test
>
> Converted test failure:
> # Subtest: printf-kunit-test
> 1..1
> # selftest: EXPECTATION FAILED at lib/printf_kunit.c:82
> vsnprintf(buf, 256, "%pi4|%pI4", ...) wrote
> '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
> # selftest: EXPECTATION FAILED at lib/printf_kunit.c:82
> vsnprintf(buf, 5, "%pi4|%pI4", ...) wrote '127.', expected '127-'
> # selftest: EXPECTATION FAILED at lib/printf_kunit.c:118
> kvasprintf(..., "%pi4|%pI4", ...) returned
> '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1'
> not ok 1 - selftest
I agree with others that there should be more KUNIT_CASEs.
> not ok 1 - printf-kunit-test
> --- a/lib/test_printf.c
> +++ b/lib/printf_kunit.c
There is no standard at the moment. I see struct kunit_source defined,
for example, in the following files:
*test*.c:
drivers/base/power/qos-test.c:
drivers/base/test/property-entry-test.c:
drivers/thunderbolt/test.c:
fs/ext4/inode-test.c:
kernel/kcsan/kcsan-test.c:
kernel/sysctl-test.c:
lib/kunit/string-stream-test.c:
lib/list-test.c:
lib/test_bits.c:
lib/test_kasan.c:
lib/test_linear_ranges.c:
net/mptcp/crypto_test.c:
net/mptcp/token_test.c:
security/apparmor/policy_unpack_test.c:
kunit-*-test.c:
lib/kunit/kunit-example-test.c:
lib/kunit/kunit-test.c:
*_kunit.c
lib/bitfield_kunit.c:
Please, either unify names of all the above modules or keep test_printf.c
> @@ -5,6 +5,7 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <kunit/test.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> @@ -30,64 +31,57 @@
> #define PAD_SIZE 16
> #define FILL_CHAR '$'
>
> -static unsigned total_tests __initdata;
> -static unsigned failed_tests __initdata;
> static char *test_buffer __initdata;
> static char *alloced_buffer __initdata;
> +struct kunit *kunittest;
This should be static variable.
>
> -static int __printf(4, 0) __init
> +static void __printf(4, 0) __init
> do_test(int bufsize, const char *expect, int elen,
> const char *fmt, va_list ap)
> {
> va_list aq;
> int ret, written;
>
> @@ -696,8 +684,9 @@ test_pointer(void)
> fwnode_pointer();
> }
>
> -static void __init selftest(void)
> +static void __init selftest(struct kunit *ktest)
> {
> + kunittest = ktest;
> alloced_buffer = kmalloc(BUF_SIZE + 2*PAD_SIZE, GFP_KERNEL);
The allocation and freeing should be done by the init,exit
callbacs in struct kunit_testsuite. For example, see
lib/kunit/kunit-test.c
This function can then be removed. The particular tests will
be called via more KUNIT_CASE() entries.
> if (!alloced_buffer)
> return;
> @@ -711,6 +700,17 @@ static void __init selftest(void)
> kfree(alloced_buffer);
> }
>
> -KSTM_MODULE_LOADERS(test_printf);
> +static struct kunit_case printf_test_cases[] = {
> + KUNIT_CASE(selftest),
> + {}
> +};
> +
> +static struct kunit_suite printf_test_suite = {
> + .name = "printf-kunit-test",
Please, use:
.name = "printf"
The fact that it is kunit-test should be clear from the context.
> + .test_cases = printf_test_cases,
> +};
> +
> +kunit_test_suite(printf_test_suite);
> +
Best Regards,
Petr
