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: >>> Convert test lib/test_printf.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. >>> KUnit and kselftest are standardizing around KTAP, converting this >>> test to KUnit makes this test output in KTAP which we are trying to >>> make the standard test result format for the kernel. More about >>> the KTAP format can be found at: >>> https://lore.kernel.org/linux-kselftest/CY4PR13MB1175B804E31E502221BC8163FD830@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/. >>> I ran both the original and converted tests as is to produce the >>> output for success of the test in the two cases. I also ran these >>> tests with a small modification to show the difference in the output >>> for failure of the test in both cases. The modification I made is: >>> - test("127.000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, &sa.sin_addr); >>> + test("127-000.000.001|127.0.0.1", "%pi4|%pI4", &sa.sin_addr, &sa.sin_addr); >>> >>> Original test success: >>> [ 0.591262] test_printf: loaded. >>> [ 0.591409] test_printf: all 388 tests passed >>> >>> Original test failure: >>> [ 0.619345] test_printf: loaded. >>> [ 0.619394] test_printf: vsnprintf(buf, 256, "%piS|%pIS", ...) >>> wrote '127.000.000.001|127.0.0.1', expected >>> '127-000.000.001|127.0.0.1' >>> [ 0.619395] test_printf: vsnprintf(buf, 25, "%piS|%pIS", ...) wrote >>> '127.000.000.001|127.0.0.', expected '127-000.000.001|127.0.0.' >>> [ 0.619396] test_printf: kvasprintf(..., "%piS|%pIS", ...) returned >>> '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' >>> [ 0.619495] test_printf: failed 3 out of 388 tests >>> >>> 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 >>> 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.
