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:94lib/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.