On Thu, 6 Feb 2025 at 23:42, Tamir Duberstein tamird@gmail.com wrote:
On Thu, Feb 6, 2025 at 4:27 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On Tue, 4 Feb 2025 at 20:36, Tamir Duberstein tamird@gmail.com 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@g... [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@rasmusvill... https://lore.kernel.org/lkml/876cc862-56f1-7330-f988-0248bec2fbad@rasmusvill... https://lore.kernel.org/lkml/0ab618c7-8c5c-00ae-8e08-0c1b99f3bf5c@rasmusvill...
I think the previous attempt was close to something acceptable (around https://lore.kernel.org/lkml/57976ff4-7845-d721-ced1-1fe439000a9b@rasmusvill...), 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:95vsnprintf(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'
This failure message looks good to me. The ones in the current patch are very verbose, and while the memory comparisons could be useful for the overflow/buffer size tests, for simple string comparisons, having the string in a readable format is best.
Rasmus: the KUnit framework has since added a summary line to the output by default, which should also make this less of a regression from the existing format: # printf: pass:28 fail:0 skip:0 total:28
Cheers, -- David