| Message ID | 20250210-printf-kunit-convert-v3-0-ee6ac5500f5e@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | printf: convert self-test to KUnit | expand |
On Mon 2025-02-10 13:23:21, Tamir Duberstein wrote: > This is one of just 3 remaining "Test Module" kselftests (the others > being bitmap and scanf), the rest having been converted to KUnit. > > I tested this using: > > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 printf > > I have also sent out a series converting scanf[0]. > > Link: https://lore.kernel.org/all/20250204-scanf-kunit-convert-v3-0-386d7c3ee714@gmail.com/T/#u [0] > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> I have just quickly tested this before leaving for a week. And I am fine with the result. I tried to simmulate an error. diff --git a/lib/test_printf.c b/lib/test_printf.c index 59dbe4f9a4cb..d2a1af31a540 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -590,7 +590,7 @@ time_and_date(void) test("15:32:23|0119-00-04", "%ptTtr|%ptTdr", &t, &t); test("2019-01-04 15:32:23", "%ptTs", &t); - test("0119-00-04 15:32:23", "%ptTsr", &t); + test("0119-00-04 15:32:24", "%ptTsr", &t); test("15:32:23|2019-01-04", "%ptTts|%ptTds", &t, &t); test("15:32:23|0119-00-04", "%ptTtrs|%ptTdrs", &t, &t); } The original result was: [ 787.626709] test_printf: loaded. [ 787.627398] test_printf: vsnprintf(buf, 256, "%ptTsr", ...) wrote '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 787.628496] test_printf: kvasprintf(..., "%ptTsr", ...) returned '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 787.629939] test_printf: failed 2 out of 448 tests The new output is: [ 585.652278] KTAP version 1 [ 585.652675] 1..1 [ 585.653085] KTAP version 1 [ 585.653382] # Subtest: printf [ 585.653702] # module: printf_kunit [ 585.653716] 1..28 [ 585.655223] ok 1 test_basic [ 585.655908] ok 2 test_number [ 585.656824] ok 3 test_string [ 585.657522] ok 4 hash_pointer [ 585.658547] ok 5 null_pointer [ 585.659572] ok 6 error_pointer [ 585.661057] ok 7 invalid_pointer [ 585.662290] ok 8 symbol_ptr [ 585.663390] ok 9 kernel_ptr [ 585.665162] ok 10 struct_resource [ 585.666231] ok 11 struct_range [ 585.667257] ok 12 addr [ 585.668399] ok 13 escaped_str [ 585.670212] ok 14 hex_string [ 585.671903] ok 15 mac [ 585.673389] ok 16 ip4 [ 585.674886] ok 17 ip6 [ 585.676255] ok 18 uuid [ 585.677875] ok 19 dentry [ 585.679138] ok 20 struct_va_format [ 585.679783] # time_and_date: EXPECTATION FAILED at lib/printf_kunit.c:97 vsnprintf(buf, 256, "%ptTsr", ...) wrote '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 585.680264] # time_and_date: EXPECTATION FAILED at lib/printf_kunit.c:135 kvasprintf(..., "%ptTsr", ...) returned '0119-00-04 15:32:23', expected '0119-00-04 15:32:24' [ 585.682436] not ok 21 time_and_date [ 585.683115] ok 22 struct_clk [ 585.685807] ok 23 bitmap [ 585.686576] ok 24 netdev_features [ 585.687243] ok 25 flags [ 585.687875] ok 26 errptr [ 585.688930] ok 27 fwnode_pointer [ 585.689544] ok 28 fourcc_pointer [ 585.689886] # printf: ran 448 tests [ 585.690215] # printf: pass:27 fail:1 skip:0 total:28 [ 585.690582] # Totals: pass:27 fail:1 skip:0 total:28 [ 585.691013] not ok 1 printf I still have to look at the implementation before I add an ack. And of course, I am also curious about what other reviewers think about it. And if this is OK for Rasmus. Best Regards, Petr
On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote: > On Mon 2025-02-10 13:23:21, Tamir Duberstein wrote: > > This is one of just 3 remaining "Test Module" kselftests (the others > > being bitmap and scanf), the rest having been converted to KUnit. > > > > I tested this using: > > > > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 printf > > > > I have also sent out a series converting scanf[0]. > > > > Link: https://lore.kernel.org/all/20250204-scanf-kunit-convert-v3-0-386d7c3ee714@gmail.com/T/#u [0] > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > I have just quickly tested this before leaving for a week. > And I am fine with the result. Seems reasonable to me. But I want a consensus with Rasmus.
On Fri, Feb 14, 2025 at 11:02 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote: > > On Mon 2025-02-10 13:23:21, Tamir Duberstein wrote: > > > This is one of just 3 remaining "Test Module" kselftests (the others > > > being bitmap and scanf), the rest having been converted to KUnit. > > > > > > I tested this using: > > > > > > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 printf > > > > > > I have also sent out a series converting scanf[0]. > > > > > > Link: https://lore.kernel.org/all/20250204-scanf-kunit-convert-v3-0-386d7c3ee714@gmail.com/T/#u [0] > > > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > > > I have just quickly tested this before leaving for a week. > > And I am fine with the result. > > Seems reasonable to me. But I want a consensus with Rasmus. I have a local v4 where I've added the same enhancement as the scanf patches so that assertions log the line in the top-level test. I'll wait for Rasmus' reply before sending. Tamir
On Fri, 14 Feb 2025 at 17:53, Tamir Duberstein <tamird@gmail.com> wrote: > > On Fri, Feb 14, 2025 at 11:02 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote: > > > I have just quickly tested this before leaving for a week. > > > And I am fine with the result. > > Thanks, Petr, for demonstrating how it looks in a failure case. > > Seems reasonable to me. But I want a consensus with Rasmus. > > I have a local v4 where I've added the same enhancement as the scanf > patches so that assertions log the line in the top-level test. > > I'll wait for Rasmus' reply before sending. I think all my concerns are addressed, with the lines printed in case of error telling what is wrong and not that memcmp() evaluating to 1 instead of 0, and with the final free-form comment including that "ran 448 tests". If you feel that word is confusing when there's "obviously" only 28 "test" being done, feel free to change that to "did 448 checks" or "did 448 individual checks" any other better wording. Rasmus
On Fri, Feb 14, 2025 at 4:47 PM Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > > On Fri, 14 Feb 2025 at 17:53, Tamir Duberstein <tamird@gmail.com> wrote: > > > > On Fri, Feb 14, 2025 at 11:02 AM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > On Fri, Feb 14, 2025 at 04:35:12PM +0100, Petr Mladek wrote: > > > > > I have just quickly tested this before leaving for a week. > > > > And I am fine with the result. > > > > > Thanks, Petr, for demonstrating how it looks in a failure case. > > > > Seems reasonable to me. But I want a consensus with Rasmus. > > > > I have a local v4 where I've added the same enhancement as the scanf > > patches so that assertions log the line in the top-level test. > > > > I'll wait for Rasmus' reply before sending. > > I think all my concerns are addressed, with the lines printed in case > of error telling what is wrong and not that memcmp() evaluating to 1 > instead of 0, and with the final free-form comment including that "ran > 448 tests". If you feel that word is confusing when there's > "obviously" only 28 "test" being done, feel free to change that to > "did 448 checks" or "did 448 individual checks" any other better > wording. > > Rasmus Personally, I don't feel strongly about this wording, so I'm hewing close to the original: .... ok 25 flags ok 26 errptr ok 27 fwnode_pointer ok 28 fourcc_pointer # printf: ran 448 tests # printf: pass:28 fail:0 skip:0 total:28 # Totals: pass:28 fail:0 skip:0 total:28 ok 1 printf I'll send v4 momentarily. Thanks, all!
This is one of just 3 remaining "Test Module" kselftests (the others being bitmap and scanf), the rest having been converted to KUnit. I tested this using: $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 printf I have also sent out a series converting scanf[0]. Link: https://lore.kernel.org/all/20250204-scanf-kunit-convert-v3-0-386d7c3ee714@gmail.com/T/#u [0] Signed-off-by: Tamir Duberstein <tamird@gmail.com> --- Changes in v3: - Remove extraneous trailing newlines from failure messages. - Replace `pr_warn` with `kunit_warn`. - Drop arch changes. - Remove KUnit boilerplate from CONFIG_PRINTF_KUNIT_TEST help text. - Restore `total_tests` counting. - Remove tc_fail macro in last patch. - Link to v2: https://lore.kernel.org/r/20250207-printf-kunit-convert-v2-0-057b23860823@gmail.com Changes in v2: - Incorporate code review from prior work[0] by Arpitha Raghunandan. - Link to v1: https://lore.kernel.org/r/20250204-printf-kunit-convert-v1-0-ecf1b846a4de@gmail.com Link: https://lore.kernel.org/lkml/20200817043028.76502-1-98.arpi@gmail.com/t/#u [0] --- Tamir Duberstein (2): printf: convert self-test to KUnit printf: break kunit into test cases Documentation/core-api/printk-formats.rst | 2 +- MAINTAINERS | 2 +- lib/Kconfig.debug | 12 +- lib/Makefile | 2 +- lib/{test_printf.c => printf_kunit.c} | 429 +++++++++++++----------------- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/printf.sh | 4 - 7 files changed, 192 insertions(+), 260 deletions(-) --- base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 change-id: 20250131-printf-kunit-convert-fd4012aa2ec6 Best regards,