[PATCH 0/3] bitmap: convert self-test to KUnit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is one of just 3 remaining "Test Module" kselftests (the others
being printf 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 bitmap.

I've already sent out a conversion series for each of printf[0] and scanf[1].

There was a previous attempt[2] to do this in July 2024. Please bear
with me as I try to understand and address the objections from that
time. I've spoken with Muhammad Usama Anjum, the author of that series,
and received their approval to "take over" this work. Here we go...

On 7/26/24 11:45 PM, John Hubbard wrote:

This changes the situation from "works for Linus' tab completion
case", to "causes a tab completion problem"! :)

I think a tests/ subdir is how we eventually decided to do this [1],
right?

So:

    lib/tests/bitmap_kunit.c

[1] https://lore.kernel.org/20240724201354.make.730-kees@xxxxxxxxxx

This is true and unfortunate, but not trivial to fix because new
kallsyms tests were placed in lib/tests in commit 84b4a51fce4c
("selftests: add new kallsyms selftests")  *after* the KUnit filename
best practices were adopted.

I propose that the KUnit maintainers blaze this trail using
`string_kunit.c` which currently still lives in lib/ despite the KUnit
docs giving it as an example at lib/tests/.

On 7/27/24 12:24 AM, Shuah Khan wrote:

This change will take away the ability to run bitmap tests during
boot on a non-kunit kernel.

Nack on this change. I wan to see all tests that are being removed
from lib because they have been converted - also it doesn't make
sense to convert some tests like this one that add the ability test
during boot.

This point was also discussed in another thread[3] in which:

On 7/27/24 12:35 AM, Shuah Khan wrote:

Please make sure you aren't taking away the ability to run these tests during
boot. 

It doesn't make sense to convert every single test especially when it
is intended to be run during boot without dependencies - not as a kunit test
but a regression test during boot.

bitmap is one example - pay attention to the config help test - bitmap
one clearly states it runs regression testing during boot. Any test that
says that isn't a candidate for conversion.

I am going to nack any such conversions.

The crux of the argument seems to be that the config help text is taken
to describe the author's intent with the fragment "at boot". I think
this may be a case of confirmation bias: I see at least the following
KUnit tests with "at boot" in their help text:
- CPUMASK_KUNIT_TEST
- BITFIELD_KUNIT
- CHECKSUM_KUNIT
- UTIL_MACROS_KUNIT

It seems to me that the inference being made is that any test that runs
"at boot" is intended to be run by both developers and users, but I find
no evidence that bitmap in particular would ever provide additional
value when run by users.

There's further discussion about KUnit not being "ideal for cases where
people would want to check a subsystem on a running kernel", but I find
no evidence that bitmap in particular is actually testing the running
kernel; it is a unit test of the bitmap functions, which is also stated
in the config help text.

David Gow made many of the same points in his final reply[4], which was
never replied to.

Link: https://lore.kernel.org/all/20250207-printf-kunit-convert-v2-0-057b23860823@xxxxxxxxx/T/#u [0]
Link: https://lore.kernel.org/all/20250207-scanf-kunit-convert-v4-0-a23e2afaede8@xxxxxxxxx/T/#u [1]
Link: https://lore.kernel.org/all/20240726110658.2281070-1-usama.anjum@xxxxxxxxxxxxx/T/#u [2]
Link: https://lore.kernel.org/all/327831fb-47ab-4555-8f0b-19a8dbcaacd7@xxxxxxxxxxxxx/T/#u [3]
Link: https://lore.kernel.org/all/CABVgOSmMoPD3JfzVd4VTkzGL2fZCo8LfwzaVSzeFimPrhgLa5w@xxxxxxxxxxxxxx/ [4]

Thanks for your attention.

Signed-off-by: Tamir Duberstein <tamird@xxxxxxxxx>
---
Tamir Duberstein (3):
      bitmap: remove _check_eq_u32_array
      bitmap: convert self-test to KUnit
      bitmap: break kunit into test cases

 MAINTAINERS                           |   2 +-
 arch/m68k/configs/amiga_defconfig     |   1 -
 arch/m68k/configs/apollo_defconfig    |   1 -
 arch/m68k/configs/atari_defconfig     |   1 -
 arch/m68k/configs/bvme6000_defconfig  |   1 -
 arch/m68k/configs/hp300_defconfig     |   1 -
 arch/m68k/configs/mac_defconfig       |   1 -
 arch/m68k/configs/multi_defconfig     |   1 -
 arch/m68k/configs/mvme147_defconfig   |   1 -
 arch/m68k/configs/mvme16x_defconfig   |   1 -
 arch/m68k/configs/q40_defconfig       |   1 -
 arch/m68k/configs/sun3_defconfig      |   1 -
 arch/m68k/configs/sun3x_defconfig     |   1 -
 arch/powerpc/configs/ppc64_defconfig  |   1 -
 lib/Kconfig.debug                     |  24 +-
 lib/Makefile                          |   2 +-
 lib/{test_bitmap.c => bitmap_kunit.c} | 454 +++++++++++++---------------------
 tools/testing/selftests/lib/bitmap.sh |   3 -
 tools/testing/selftests/lib/config    |   1 -
 19 files changed, 195 insertions(+), 304 deletions(-)
---
base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
change-id: 20250207-bitmap-kunit-convert-92d3147b2eee

Best regards,
-- 
Tamir Duberstein <tamird@xxxxxxxxx>





[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux