On Fri, Feb 14, 2025 at 8:33 AM Petr Mladek <pmla...@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 <tam...@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.
