On Thu, Feb 06 2025, Tamir Duberstein <tamird@xxxxxxxxx> wrote: > On Thu, Feb 6, 2025 at 4:27 AM Rasmus Villemoes > <linux@xxxxxxxxxxxxxxxxxx> wrote: >> >> On Tue, 4 Feb 2025 at 20:36, Tamir Duberstein <tamird@xxxxxxxxx> 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@xxxxxxxxx/T/#u [0] >> > >> >> Sorry, but NAK, not in this form. >> >> Please read the previous threads to understand what is wrong with this >> mechanical approach. Not only is it wrong, it also actively makes the >> test suite much less useful. >> >> https://lore.kernel.org/lkml/f408efbd-10f7-f1dd-9baa-0f1233cafffc@xxxxxxxxxxxxxxxxxx/ >> https://lore.kernel.org/lkml/876cc862-56f1-7330-f988-0248bec2fbad@xxxxxxxxxxxxxxxxxx/ >> https://lore.kernel.org/lkml/0ab618c7-8c5c-00ae-8e08-0c1b99f3bf5c@xxxxxxxxxxxxxxxxxx/ >> >> I think the previous attempt was close to something acceptable (around >> https://lore.kernel.org/lkml/57976ff4-7845-d721-ced1-1fe439000a9b@xxxxxxxxxxxxxxxxxx/), >> but I don't know what happened to it. >> >> Rasmus > > Thanks Rasmus, I wasn't aware of that prior effort. I've gone through > and adopted your comments - the result is a first patch that is much > smaller (104 insertions(+), 104 deletions(-)) and failure messages > that are quite close to what is emitted now. I've taken care to keep > all the control flow the same, as you requested. The previous > discussion concluded with a promise to send another patch which didn't > happen. May I send a v2 with these changes, or are there more > fundamental objections? I'll also cc Arpitha and Brendan. The new > failure output: > > # ip4: EXPECTATION FAILED at lib/printf_kunit.c:95 > vsnprintf(buf, 256, "%piS|%pIS", ...) wrote > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' > # ip4: EXPECTATION FAILED at lib/printf_kunit.c:95 > vsnprintf(buf, 19, "%piS|%pIS", ...) wrote '127.000.000.001|12', > expected '127-000.000.001|12' > # ip4: EXPECTATION FAILED at lib/printf_kunit.c:131 > kvasprintf(..., "%piS|%pIS", ...) returned > '127.000.000.001|127.0.0.1', expected '127-000.000.001|127.0.0.1' That certainly addresses one of my main objections; I really don't want to see "memcmp(..., ...) == 1, expected memcmp(..., ...) == 0". And you said you've kept the control flow/early returns the same, so that should also be ok. I'll have to see the actual code, of course. In general, I find reading code using those KUNIT macros quite hard, because I'm not familiar with those macros and when I try to look up what they do they turn out to be defined in terms of other KUNIT macros 10 levels deep. But that still leaves a few points. First, I really like that "388 test cases passed" tally or some other free-form summary (so that I can see that I properly hooked up, compiled, and ran a new testcase inside test_number(), so any kind of aggregation on those top-level test_* is too coarse). The other thing I want to know is if this would make it harder for me to finish up that "deterministic random testing" thing I wrote [*], but never got around to actually get it upstream. It seems like something that a framework like kunit could usefully provide out-of-the-box (which is why I attempted to get it into kselftest), but as long as I'd still be able to add in something like that, and get a "xxx failed, random seed used was 0xabcdef" line printed, and run the test again setting the seed explicitly to that 0xabcdef value, I'm good. [*] https://lore.kernel.org/lkml/20201025214842.5924-4-linux@xxxxxxxxxxxxxxxxxx/ Rasmus
