On 7/29/24 02:15, Muhammad Usama Anjum wrote:
On 7/27/24 11:10 PM, Yury Norov wrote:
On Fri, Jul 26, 2024 at 01:26:48PM -0600, Shuah Khan wrote:
On 7/26/24 05:06, Muhammad Usama Anjum wrote:
In this series, test_bitmap is being converted to kunit test. Multiple
patches will make the review process smooth.
- Patch-1: Convert the tests in lib/test_bitmap.c to kunit
- Patch-2: Rename the lib/test_bitmap.c to lib/bitmap_kunit.c and other
configuration options
- Patch-3: Remove the bitmap.sh selftest
Muhammad Usama Anjum (3):
bitmap: convert test_bitmap to KUnit test
bitmap: Rename module
selftests: lib: remove test_bitmap
MAINTAINERS | 2 +-
lib/Kconfig.debug | 15 +-
lib/Makefile | 2 +-
lib/{test_bitmap.c => bitmap_kunit.c} | 624 ++++++++++++--------------
tools/testing/selftests/lib/Makefile | 2 +-
tools/testing/selftests/lib/bitmap.sh | 3 -
tools/testing/selftests/lib/config | 1 -
7 files changed, 295 insertions(+), 354 deletions(-)
rename lib/{test_bitmap.c => bitmap_kunit.c} (70%)
delete mode 100755 tools/testing/selftests/lib/bitmap.sh
Can you tell me how this conversion helps?
It is removing the ability to run bitmap tests during boot.
It doesn't make sense to blindly convert all test under lib
to kunit - Nack on this change or any change that takes away
the ability to run tests and makes them dependent on kunit.
Hi Muhammad,
In addition to Shuah's and John's reasoning. This patch wipes the
test history (git blame will point on you for most of the test),
When files are renamed, their history isn't lost. We just need to use
--follow option with git log to get complete history[1].
breaks boot-time testing support, messes with config names and
usability, and drops kselftest support for ... exactly, what?
AFAIU the kselftest wasn't detected the test results that's why I started
thinking on which could be best way to detect if any failure happens in
this test. Triggering the test from kselftest doesn't grantee the test it
would pass every time until we check results. For this kind of in-kernel
testing, kunit is best suites. Please find earlier discussion [2].
KUnit isn't idea for cases where people would want to check a subsystem
on a running kernel - KUnit covers some use-cases and kselftest covers
others.
What happens if we are debugging a problem that requires us to debug on
a running system? Please don't go converting kselftest into kunit without
understanding how these are intended to be used.
Yes kselftest results need to be looked at. Write a parser which can
be improved. What you are doing is reducing the coverage and talking
away the ability to debug and test on running system.
Fix what needs to be fixed instead of deleting tests.
KUNIT engine here doesn't improve on readability, neither shorten
the test length, to my taste.
If you'd like to contribute to bitmaps testing - I'm all for that.
This is the very core and performance-sensitive piece of kernel,
and any extra-coverage is always welcome.
+1 on this. Add new tests and look at the reports.
But I think the best way would be either adding new cases to the
existing test, or writing a new test, KUNIT-based, if you like.
+1
As I mentioned in my earlier message, I am going to nack all patches
that convert existing selftests to kunit such as this one.
thanks,
-- Shuah
