| Message ID | 20250211-scanf-kunit-convert-v7-0-c057f0a3d9d8@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | scanf: convert self-test to KUnit | expand |
On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote: > This is one of just 3 remaining "Test Module" kselftests (the others > being bitmap and printf), the rest having been converted to KUnit. In > addition to the enclosed patch, please consider this an RFC on the > removal of the "Test Module" kselftest machinery. > > I tested this using: > > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > --- > Changes in v7: > - Remove redundant debug logs. (Petr Mladek) > - Drop Petr's Acked-by. > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse > messages. The new failure output is: It would be good if you put into cover letter, or even in the respectful patch the example of the error report for the old code and new code that it will be clear how it changes. > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > not ok 1 " " > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > not ok 2 ":" > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > not ok 3 "," > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > not ok 4 "-" > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > not ok 5 "/" > # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5 > not ok 4 numbers_list_field_width_val_width > # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92 > vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518
On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote: > > This is one of just 3 remaining "Test Module" kselftests (the others > > being bitmap and printf), the rest having been converted to KUnit. In > > addition to the enclosed patch, please consider this an RFC on the > > removal of the "Test Module" kselftest machinery. > > > > I tested this using: > > > > $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > --- > > Changes in v7: > > - Remove redundant debug logs. (Petr Mladek) > > - Drop Petr's Acked-by. > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse > > messages. The new failure output is: > > It would be good if you put into cover letter, or even in the respectful patch > the example of the error report for the old code and new code that it will be > clear how it changes. > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > not ok 1 " " > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > > not ok 2 ":" > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > > not ok 3 "," > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > > not ok 4 "-" > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > > not ok 5 "/" > > # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5 > > not ok 4 numbers_list_field_width_val_width > > # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92 > > vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518 Makes sense. As you can see the error report for the new code is included here. I'll add the old code's error report if I have to respin v8.
On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote: > On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote: ... > > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse > > > messages. The new failure output is: > > > > It would be good if you put into cover letter, or even in the respectful patch > > the example of the error report for the old code and new code that it will be > > clear how it changes. > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > not ok 1 " " > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > > > not ok 2 ":" > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > > > not ok 3 "," > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > > > not ok 4 "-" > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > > > not ok 5 "/" > > > # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5 > > > not ok 4 numbers_list_field_width_val_width > > > # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518 > > Makes sense. As you can see the error report for the new code is > included here. I'll add the old code's error report if I have to > respin v8. At a bare minimum. can you add in the reply to this email?
On Tue, Feb 11, 2025 at 10:54 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote: > > On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote: > > ... > > > > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse > > > > messages. The new failure output is: > > > > > > It would be good if you put into cover letter, or even in the respectful patch > > > the example of the error report for the old code and new code that it will be > > > clear how it changes. > > > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > > not ok 1 " " > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > > > > not ok 2 ":" > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > > > > not ok 3 "," > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > > > > not ok 4 "-" > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > > > > not ok 5 "/" > > > > # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5 > > > > not ok 4 numbers_list_field_width_val_width > > > > # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518 > > > > Makes sense. As you can see the error report for the new code is > > included here. I'll add the old code's error report if I have to > > respin v8. > > At a bare minimum. can you add in the reply to this email? Oh, sure: On Tue, Feb 11, 2025 at 6:54 AM Petr Mladek <pmladek@suse.com> wrote: > > [...] > > [ 383.100048] test_scanf: vsscanf("1574 9 64ca 935b 7 142d ff58 0", "%4hx %1hx %4hx %4hx %1hx %4hx %4hx %1hx", ...) expected 2472240330 got 1690959881 > [ 383.102843] test_scanf: vsscanf("f12:2:d:2:c166:1:36b:1906", "%3hx:%1hx:%1hx:%1hx:%4hx:%1hx:%3hx:%4hx", ...) expected 131085 got 851970 > [ 383.105376] test_scanf: vsscanf("4,b2fe,3,593,6,0,3bde,0", "%1hx,%4hx,%1hx,%3hx,%1hx,%1hx,%4hx,%1hx", ...) expected 93519875 got 242430 > [ 383.105659] test_scanf: vsscanf("6-1-2-1-d9e6-f-93e-e567", "%1hx-%1hx-%1hx-%1hx-%4hx-%1hx-%3hx-%4hx", ...) expected 65538 got 131073 > [ 383.106127] test_scanf: vsscanf("72d6/35/e88d/1/0/6c8c/7/1", "%4hx/%2hx/%4hx/%1hx/%1hx/%4hx/%1hx/%1hx", ...) expected 125069 got 3901554741 > [ 383.106235] test_scanf: vsscanf("c9bea1b8122113e9a168df573", "%4hx%4hx%1hx%4hx%4hx%1hx%4hx%3hx", ...) expected 571539457 got 106936 > [ 383.106398] test_scanf: failed 6 out of 2545 tests This is from https://lore.kernel.org/all/Z6s6WsIw3VCGys6K@pathway.suse.cz/ (doesn't load for me, seems lore is having problems).
On Tue, Feb 11, 2025 at 10:57:11AM -0500, Tamir Duberstein wrote: > On Tue, Feb 11, 2025 at 10:54 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote: > > > On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote: ... > > > > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse > > > > > messages. The new failure output is: > > > > > > > > It would be good if you put into cover letter, or even in the respectful patch > > > > the example of the error report for the old code and new code that it will be > > > > clear how it changes. > > > > > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > > > not ok 1 " " > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > > > > > not ok 2 ":" > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > > > > > not ok 3 "," > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > > > > > not ok 4 "-" > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > > > > > not ok 5 "/" > > > > > # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5 > > > > > not ok 4 numbers_list_field_width_val_width > > > > > # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518 > > > > > > Makes sense. As you can see the error report for the new code is > > > included here. I'll add the old code's error report if I have to > > > respin v8. > > > > At a bare minimum. can you add in the reply to this email? > > Oh, sure: > > On Tue, Feb 11, 2025 at 6:54 AM Petr Mladek <pmladek@suse.com> wrote: > > > > [...] > > > > [ 383.100048] test_scanf: vsscanf("1574 9 64ca 935b 7 142d ff58 0", "%4hx %1hx %4hx %4hx %1hx %4hx %4hx %1hx", ...) expected 2472240330 got 1690959881 > > [ 383.102843] test_scanf: vsscanf("f12:2:d:2:c166:1:36b:1906", "%3hx:%1hx:%1hx:%1hx:%4hx:%1hx:%3hx:%4hx", ...) expected 131085 got 851970 > > [ 383.105376] test_scanf: vsscanf("4,b2fe,3,593,6,0,3bde,0", "%1hx,%4hx,%1hx,%3hx,%1hx,%1hx,%4hx,%1hx", ...) expected 93519875 got 242430 > > [ 383.105659] test_scanf: vsscanf("6-1-2-1-d9e6-f-93e-e567", "%1hx-%1hx-%1hx-%1hx-%4hx-%1hx-%3hx-%4hx", ...) expected 65538 got 131073 > > [ 383.106127] test_scanf: vsscanf("72d6/35/e88d/1/0/6c8c/7/1", "%4hx/%2hx/%4hx/%1hx/%1hx/%4hx/%1hx/%1hx", ...) expected 125069 got 3901554741 > > [ 383.106235] test_scanf: vsscanf("c9bea1b8122113e9a168df573", "%4hx%4hx%1hx%4hx%4hx%1hx%4hx%3hx", ...) expected 571539457 got 106936 > > [ 383.106398] test_scanf: failed 6 out of 2545 tests Is it me who cut something or the above missing this information (total tests)? If the latter, how are we supposed to answer to the question if the failed test is from new bunch of cases I hypothetically added or regression of the existing ones? Without this it seems like I need to go through all failures. OTOH it may be needed anyway as failing test case needs an investigation. > This is from https://lore.kernel.org/all/Z6s6WsIw3VCGys6K@pathway.suse.cz/ > (doesn't load for me, seems lore is having problems).
On Tue, Feb 11, 2025 at 12:17 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Tue, Feb 11, 2025 at 10:57:11AM -0500, Tamir Duberstein wrote: > > On Tue, Feb 11, 2025 at 10:54 AM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Tue, Feb 11, 2025 at 10:47:03AM -0500, Tamir Duberstein wrote: > > > > On Tue, Feb 11, 2025 at 10:40 AM Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > On Tue, Feb 11, 2025 at 10:13:36AM -0500, Tamir Duberstein wrote: > > ... > > > > > > > - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse > > > > > > messages. The new failure output is: > > > > > > > > > > It would be good if you put into cover letter, or even in the respectful patch > > > > > the example of the error report for the old code and new code that it will be > > > > > clear how it changes. > > > > > > > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > > > > not ok 1 " " > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > > vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > > > > > > not ok 2 ":" > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > > vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > > > > > > not ok 3 "," > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > > vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > > > > > > not ok 4 "-" > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > > vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > > > > > > not ok 5 "/" > > > > > > # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5 > > > > > > not ok 4 numbers_list_field_width_val_width > > > > > > # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > > vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518 > > > > > > > > Makes sense. As you can see the error report for the new code is > > > > included here. I'll add the old code's error report if I have to > > > > respin v8. > > > > > > At a bare minimum. can you add in the reply to this email? > > > > Oh, sure: > > > > On Tue, Feb 11, 2025 at 6:54 AM Petr Mladek <pmladek@suse.com> wrote: > > > > > > [...] > > > > > > [ 383.100048] test_scanf: vsscanf("1574 9 64ca 935b 7 142d ff58 0", "%4hx %1hx %4hx %4hx %1hx %4hx %4hx %1hx", ...) expected 2472240330 got 1690959881 > > > [ 383.102843] test_scanf: vsscanf("f12:2:d:2:c166:1:36b:1906", "%3hx:%1hx:%1hx:%1hx:%4hx:%1hx:%3hx:%4hx", ...) expected 131085 got 851970 > > > [ 383.105376] test_scanf: vsscanf("4,b2fe,3,593,6,0,3bde,0", "%1hx,%4hx,%1hx,%3hx,%1hx,%1hx,%4hx,%1hx", ...) expected 93519875 got 242430 > > > [ 383.105659] test_scanf: vsscanf("6-1-2-1-d9e6-f-93e-e567", "%1hx-%1hx-%1hx-%1hx-%4hx-%1hx-%3hx-%4hx", ...) expected 65538 got 131073 > > > [ 383.106127] test_scanf: vsscanf("72d6/35/e88d/1/0/6c8c/7/1", "%4hx/%2hx/%4hx/%1hx/%1hx/%4hx/%1hx/%1hx", ...) expected 125069 got 3901554741 > > > [ 383.106235] test_scanf: vsscanf("c9bea1b8122113e9a168df573", "%4hx%4hx%1hx%4hx%4hx%1hx%4hx%3hx", ...) expected 571539457 got 106936 > > > > [ 383.106398] test_scanf: failed 6 out of 2545 tests > > Is it me who cut something or the above missing this information (total tests)? > If the latter, how are we supposed to answer to the question if the failed test > is from new bunch of cases I hypothetically added or regression of the existing > ones? Without this it seems like I need to go through all failures. OTOH it may > be needed anyway as failing test case needs an investigation. I assume you mean missing from the new output. Yeah, KUnit doesn't do this counting. Instead you get the test name in the failure message: > > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > > > > not ok 1 " " > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 I think maybe you're saying: what if I add a new assertion (rather than a new test case), and I start getting failure reports - how do I know if the reporter is running old or new test code? In an ideal world the message above would give you all the information you need by including the line number from the test. This doesn't quite work out in this case because of the various test helper functions; you end up with a line number in the test helper rather than in the test itself. We could fix that by passing around __FILE__ and __LINE__ (probably by wrapping the test helpers in a macro). What do you think?
On Tue, Feb 11, 2025 at 12:26 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > Is it me who cut something or the above missing this information (total tests)? > > If the latter, how are we supposed to answer to the question if the failed test > > is from new bunch of cases I hypothetically added or regression of the existing > > ones? Without this it seems like I need to go through all failures. OTOH it may > > be needed anyway as failing test case needs an investigation. > > I assume you mean missing from the new output. Yeah, KUnit doesn't do > this counting. Instead you get the test name in the failure message: > > > > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > > > > > not ok 1 " " > > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > I think maybe you're saying: what if I add a new assertion (rather > than a new test case), and I start getting failure reports - how do I > know if the reporter is running old or new test code? > > In an ideal world the message above would give you all the information > you need by including the line number from the test. This doesn't > quite work out in this case because of the various test helper > functions; you end up with a line number in the test helper rather > than in the test itself. We could fix that by passing around __FILE__ > and __LINE__ (probably by wrapping the test helpers in a macro). What > do you think? I gave this a try locally, and it produced this output: > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > lib/scanf_kunit.c:555: vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > not ok 1 " " > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > lib/scanf_kunit.c:555: vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > not ok 2 ":" > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > lib/scanf_kunit.c:555: vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > not ok 3 "," > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > lib/scanf_kunit.c:555: vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > not ok 4 "-" > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > lib/scanf_kunit.c:555: vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > not ok 5 "/" Andy, Petr: what do you think? I've added this (and the original output, as you requested) to the cover letter for when I reroll v8 (not before next week).
On Wed 2025-02-12 11:54:52, Tamir Duberstein wrote: > On Tue, Feb 11, 2025 at 12:26 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > Is it me who cut something or the above missing this information (total tests)? > > > If the latter, how are we supposed to answer to the question if the failed test > > > is from new bunch of cases I hypothetically added or regression of the existing > > > ones? Without this it seems like I need to go through all failures. OTOH it may > > > be needed anyway as failing test case needs an investigation. > > > > I assume you mean missing from the new output. Yeah, KUnit doesn't do > > this counting. Instead you get the test name in the failure message: > > > > > > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > > > > > > not ok 1 " " > > > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > I think maybe you're saying: what if I add a new assertion (rather > > than a new test case), and I start getting failure reports - how do I > > know if the reporter is running old or new test code? > > > > In an ideal world the message above would give you all the information > > you need by including the line number from the test. This doesn't > > quite work out in this case because of the various test helper > > functions; you end up with a line number in the test helper rather > > than in the test itself. We could fix that by passing around __FILE__ > > and __LINE__ (probably by wrapping the test helpers in a macro). What > > do you think? I am not sure how many changes are needed to wrap the test helpers in a macro. > I gave this a try locally, and it produced this output: > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > lib/scanf_kunit.c:555: vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > not ok 1 " " > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > lib/scanf_kunit.c:555: vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > > not ok 2 ":" > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > lib/scanf_kunit.c:555: vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > > not ok 3 "," > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > lib/scanf_kunit.c:555: vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > > not ok 4 "-" > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > lib/scanf_kunit.c:555: vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > > not ok 5 "/" But I really like that the error message shows the exact line of the caller. IMHO, it is very helpful in this module. I like it. IMHO, it also justifies removing the pr_debug() messages (currently 1st patch). > Andy, Petr: what do you think? I've added this (and the original > output, as you requested) to the cover letter for when I reroll v8 > (not before next week). I suggest, to do the switch into macros in the 1st patch. Remove the obsolete pr_debug() lines in 2nd patch. Plus two more patches switching the module to kunit test. I am personally fine with this change. Best Regards, Petr
On Fri, Feb 14, 2025 at 8:33 AM Petr Mladek <pmladek@suse.com> wrote: > > On Wed 2025-02-12 11:54:52, Tamir Duberstein wrote: > > On Tue, Feb 11, 2025 at 12:26 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > > > Is it me who cut something or the above missing this information (total tests)? > > > > If the latter, how are we supposed to answer to the question if the failed test > > > > is from new bunch of cases I hypothetically added or regression of the existing > > > > ones? Without this it seems like I need to go through all failures. OTOH it may > > > > be needed anyway as failing test case needs an investigation. > > > > > > I assume you mean missing from the new output. Yeah, KUnit doesn't do > > > this counting. Instead you get the test name in the failure message: > > > > > > > > > > > > vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > > > > > > > not ok 1 " " > > > > > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 > > > > > > I think maybe you're saying: what if I add a new assertion (rather > > > than a new test case), and I start getting failure reports - how do I > > > know if the reporter is running old or new test code? > > > > > > In an ideal world the message above would give you all the information > > > you need by including the line number from the test. This doesn't > > > quite work out in this case because of the various test helper > > > functions; you end up with a line number in the test helper rather > > > than in the test itself. We could fix that by passing around __FILE__ > > > and __LINE__ (probably by wrapping the test helpers in a macro). What > > > do you think? > > I am not sure how many changes are needed to wrap the test helpers in > a macro. > > > I gave this a try locally, and it produced this output: > > > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > > lib/scanf_kunit.c:555: vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 > > > not ok 1 " " > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > > lib/scanf_kunit.c:555: vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 > > > not ok 2 ":" > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > > lib/scanf_kunit.c:555: vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 > > > not ok 3 "," > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > > lib/scanf_kunit.c:555: vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 > > > not ok 4 "-" > > > # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:94 > > > lib/scanf_kunit.c:555: vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 > > > not ok 5 "/" > > But I really like that the error message shows the exact line of the > caller. IMHO, it is very helpful in this module. I like it. > > IMHO, it also justifies removing the pr_debug() messages (currently 1st patch). > > > Andy, Petr: what do you think? I've added this (and the original > > output, as you requested) to the cover letter for when I reroll v8 > > (not before next week). > > I suggest, to do the switch into macros in the 1st patch. > Remove the obsolete pr_debug() lines in 2nd patch. > Plus two more patches switching the module to kunit test. > > I am personally fine with this change. > > Best Regards, > Petr Thanks Petr. I'll send v8 now, then.
This is one of just 3 remaining "Test Module" kselftests (the others being bitmap and printf), the rest having been converted to KUnit. In addition to the enclosed patch, please consider this an RFC on the removal of the "Test Module" kselftest machinery. I tested this using: $ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 scanf Signed-off-by: Tamir Duberstein <tamird@gmail.com> --- Changes in v7: - Remove redundant debug logs. (Petr Mladek) - Drop Petr's Acked-by. - Use original test assertions as KUNIT_*_EQ_MSG produces hard-to-parse messages. The new failure output is: vsscanf("0 1e 3e43 31f0 0 0 5797 9c70", "%1hx %2hx %4hx %4hx %1hx %1hx %4hx %4hx", ...) expected 837828163 got 1044578334 not ok 1 " " # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 vsscanf("dc2:1c:0:3531:2621:5172:1:7", "%3hx:%2hx:%1hx:%4hx:%4hx:%4hx:%1hx:%1hx", ...) expected 892403712 got 28 not ok 2 ":" # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 vsscanf("e083,8f6e,b,70ca,1,1,aab1,10e4", "%4hx,%4hx,%1hx,%4hx,%1hx,%1hx,%4hx,%4hx", ...) expected 1892286475 got 757614 not ok 3 "," # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 vsscanf("2e72-8435-1-2fc-7cbd-c2f1-7158-2b41", "%4hx-%4hx-%1hx-%3hx-%4hx-%4hx-%4hx-%4hx", ...) expected 50069505 got 99381 not ok 4 "-" # numbers_list_field_width_val_width: ASSERTION FAILED at lib/scanf_kunit.c:92 vsscanf("403/0/17/1/11e7/1/1fe8/34ba", "%3hx/%1hx/%2hx/%1hx/%4hx/%1hx/%4hx/%4hx", ...) expected 65559 got 1507328 not ok 5 "/" # numbers_list_field_width_val_width: pass:0 fail:5 skip:0 total:5 not ok 4 numbers_list_field_width_val_width # numbers_slice: ASSERTION FAILED at lib/scanf_kunit.c:92 vsscanf("3c87eac0f4afa1f9231da52", "%1hx%4hx%4hx%4hx%1hx%4hx%4hx%1hx", ...) expected 1257942031 got 2886715518 - Link to v6: https://lore.kernel.org/r/20250210-scanf-kunit-convert-v6-0-4d583d07f92d@gmail.com Changes in v6: - s/at boot/at runtime/ for consistency with the printf series. - Go back to kmalloc. (Geert Uytterhoeven) - Link to v5: https://lore.kernel.org/r/20250210-scanf-kunit-convert-v5-0-8e64f3a7de99@gmail.com Changes in v5: - Remove extraneous trailing newlines from failure messages. - Replace `pr_debug` with `kunit_printk`. - Use static char arrays instead of kmalloc. - Drop KUnit boilerplate from CONFIG_SCANF_KUNIT_TEST help text. - Drop arch changes. - Link to v4: https://lore.kernel.org/r/20250207-scanf-kunit-convert-v4-0-a23e2afaede8@gmail.com Changes in v4: - Bake `test` into various macros, greatly reducing diff noise. - Revert control flow changes. - Link to v3: https://lore.kernel.org/r/20250204-scanf-kunit-convert-v3-0-386d7c3ee714@gmail.com Changes in v3: - Reduce diff noise in lib/Makefile. (Petr Mladek) - Split `scanf_test` into a few test cases. New output: : =================== scanf (10 subtests) ==================== : [PASSED] numbers_simple : ====================== numbers_list ======================= : [PASSED] delim=" " : [PASSED] delim=":" : [PASSED] delim="," : [PASSED] delim="-" : [PASSED] delim="/" : ================== [PASSED] numbers_list =================== : ============ numbers_list_field_width_typemax ============= : [PASSED] delim=" " : [PASSED] delim=":" : [PASSED] delim="," : [PASSED] delim="-" : [PASSED] delim="/" : ======== [PASSED] numbers_list_field_width_typemax ========= : =========== numbers_list_field_width_val_width ============ : [PASSED] delim=" " : [PASSED] delim=":" : [PASSED] delim="," : [PASSED] delim="-" : [PASSED] delim="/" : ======= [PASSED] numbers_list_field_width_val_width ======== : [PASSED] numbers_slice : [PASSED] numbers_prefix_overflow : [PASSED] test_simple_strtoull : [PASSED] test_simple_strtoll : [PASSED] test_simple_strtoul : [PASSED] test_simple_strtol : ====================== [PASSED] scanf ====================== : ============================================================ : Testing complete. Ran 22 tests: passed: 22 : Elapsed time: 5.517s total, 0.001s configuring, 5.440s building, 0.067s running - Link to v2: https://lore.kernel.org/r/20250203-scanf-kunit-convert-v2-1-277a618d804e@gmail.com Changes in v2: - Rename lib/{test_scanf.c => scanf_kunit.c}. (Andy Shevchenko) - Link to v1: https://lore.kernel.org/r/20250131-scanf-kunit-convert-v1-1-0976524f0eba@gmail.com --- Tamir Duberstein (3): scanf: remove redundant debug logs scanf: convert self-test to KUnit scanf: break kunit into test cases MAINTAINERS | 2 +- lib/Kconfig.debug | 12 +- lib/Makefile | 2 +- lib/{test_scanf.c => scanf_kunit.c} | 270 +++++++++++++++++------------------ tools/testing/selftests/lib/Makefile | 2 +- tools/testing/selftests/lib/config | 1 - tools/testing/selftests/lib/scanf.sh | 4 - 7 files changed, 143 insertions(+), 150 deletions(-) --- base-commit: a64dcfb451e254085a7daee5fe51bf22959d52d3 change-id: 20250131-scanf-kunit-convert-f70dc33bb34c Best regards,