From: Yury Norov <yury.norov@gmail.com>
To: Shuah Khan <skhan@linuxfoundation.org>
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>,
Andrew Morton <akpm@linux-foundation.org>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Shuah Khan <shuah@kernel.org>,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
kees@kernel.org, David Gow <davidgow@google.com>,
John Hubbard <jhubbard@nvidia.com>,
kernel@collabora.com
Subject: Re: [PATCH 0/3] bitmap: Convert test_bitmap to kunit test
Date: Sat, 27 Jul 2024 11:10:28 -0700 [thread overview]
Message-ID: <ZqU4FANdHOvpGc5t@yury-ThinkPad> (raw)
In-Reply-To: <27b91030-b01f-44e4-82f7-93b3e11e8d74@linuxfoundation.org>
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),
breaks boot-time testing support, messes with config names and
usability, and drops kselftest support for ... exactly, what?
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.
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.
Thanks,
Yury
next prev parent reply other threads:[~2024-07-27 18:10 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-26 11:06 [PATCH 0/3] bitmap: Convert test_bitmap to kunit test Muhammad Usama Anjum
2024-07-26 11:06 ` [PATCH 1/3] bitmap: convert test_bitmap to KUnit test Muhammad Usama Anjum
2024-07-26 11:06 ` [PATCH 2/3] bitmap: Rename module Muhammad Usama Anjum
2024-07-26 18:45 ` John Hubbard
2024-07-29 7:57 ` Muhammad Usama Anjum
2024-07-26 19:24 ` Shuah Khan
2024-07-29 8:02 ` Muhammad Usama Anjum
2024-07-27 17:35 ` Yury Norov
2024-07-29 8:07 ` Muhammad Usama Anjum
2024-07-29 14:09 ` Randy Dunlap
2024-07-30 7:51 ` Muhammad Usama Anjum
2024-07-30 13:53 ` Randy Dunlap
2024-07-30 10:10 ` David Gow
2024-07-30 15:55 ` Shuah Khan
2024-07-30 18:17 ` Shuah Khan
2024-07-30 18:44 ` John Hubbard
2024-07-30 17:49 ` Yury Norov
2024-07-26 11:06 ` [PATCH 3/3] selftests: lib: remove test_bitmap Muhammad Usama Anjum
2024-07-26 19:22 ` Shuah Khan
2024-07-26 19:26 ` [PATCH 0/3] bitmap: Convert test_bitmap to kunit test Shuah Khan
2024-07-27 18:10 ` Yury Norov [this message]
2024-07-29 8:15 ` Muhammad Usama Anjum
2024-07-30 15:39 ` Shuah Khan
2024-07-31 3:05 ` David Gow
2024-07-29 8:29 ` Muhammad Usama Anjum
2024-07-30 15:49 ` Shuah Khan
2024-07-31 3:06 ` David Gow
2024-07-31 16:26 ` Shuah Khan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZqU4FANdHOvpGc5t@yury-ThinkPad \
--to=yury.norov@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=davidgow@google.com \
--cc=jhubbard@nvidia.com \
--cc=kees@kernel.org \
--cc=kernel@collabora.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=shuah@kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=usama.anjum@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox