On 23/10/20 11:31 pm, Petr Mladek wrote: > On Fri 2020-10-23 19:13:00, Arpitha Raghunandan wrote: >> On 23/10/20 4:36 pm, Rasmus Villemoes wrote: >>> On 22/10/2020 21.16, Andy Shevchenko wrote: >>>> On Thu, Oct 22, 2020 at 08:43:49PM +0530, Arpitha Raghunandan wrote: >>>>> 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 >>>>> not ok 1 - printf-kunit-test >>>> >>>> Not bad. Rasmus, what do you think? >>> >>> Much better, but that '1..1' and reporting the entire test suite as 1 >>> single (failing or passing) test is (also) a regression. Look at the >>> original >>> >>>>> [ 0.591409] test_printf: all 388 tests passed >>> >>> or >>> >>>>> [ 0.619495] test_printf: failed 3 out of 388 tests >>> >>> That's far more informative, and I'd prefer if the summary information >>> (whether in the all-good case or some-failing) included something like >>> this. In particular, I have at some point spotted that I failed to >>> properly hook up a new test case (or perhaps failed to re-compile, or >>> somehow still ran the old kernel binary, don't remember which it was) by >>> noticing that the total number of tests hadn't increased. The new output >>> would not help catch such PEBKACs. >>> >>> Rasmus >>> >> >> Splitting the test into multiple test cases in KUnit will display >> the number and name of tests that pass or fail. This will be similar >> to the lib/list-test.c test as can be seen here: >> https://elixir.bootlin.com/linux/latest/source/lib/list-test.c. >> I will work on this for the next version of this patch. > > We should probably agree on the granularity first. > > It looks like an overkill to split the tests into 388 functions > and define KUNIT_CASE() lines. It might be possible to hide > this into macros but macros are hell for debugging. > > I suggest to split it by the current functions that do more test() > call inside. I mean to define something like: > > static struct kunit_case printf_test_cases[] = { > KUNIT_CASE(basic), > KUNIT_CASE(number), > KUNIT_CASE(string), > KUNIT_CASE(plain_pointer), > KUNIT_CASE(null_poiter), > KUNIT_CASE(error_pointer), > KUNIT_CASE(addr), > KUNIT_CASE(struct_resource), > KUNIT_CASE(dentry), > KUNIT_CASE(pointer_addr), > ..., > {} > }; > > Best Regards, > Petr > Okay, I will split it by the current functions that have more test() calls inside as suggested. I will also make changes as per your other suggestions for the next version. Thanks!
