On Mon, May 30, 2022 at 9:29 AM José Expósito jose.exposito89@gmail.com wrote:
I just cc'ed kunit-dev@googlegroups.com. For anyone joining the conversation, here is the link to the patch and the cover letter with some questions:
https://lore.kernel.org/dri-devel/20220530102017.471865-1-jose.exposito89@gm...
Thanks for the link. A few initial notes: a) normally, `select`ing other kconfigs is discouraged. It's not explicitly called out in https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-kconf... but this was the conclusion after some debate on the mailing lists earlier. b) I see `dst` is allocated with kzalloc but not freed. Should we use `kunit_kzalloc()` instead so it does get automatically freed?
drivers/gpu/drm/Kconfig | 12 ++ drivers/gpu/drm/Makefile | 3 + drivers/gpu/drm/drm_format_helper_test.c | 166 +++++++++++++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 drivers/gpu/drm/drm_format_helper_test.c
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index e88c497fa010..d92be6faef15 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -76,6 +76,18 @@ config DRM_KMS_HELPER help CRTC helpers for KMS drivers.
+config DRM_FORMAR_HELPER_TEST
- bool "drm_format_helper tests" if !KUNIT_ALL_TESTS
- depends on DRM && KUNIT=y
- select DRM_KMS_HELPER
From above, a)
Specifically here, it'd be encouraged to instead do depends on DRM && KUNIT=y && DRM_KMS_HELPER
Ideally, using a .kunitconfig file would make it so having to select DRM_KMS_HELPER manually isn't that annoying.
AFAIK, kunit test cases are supposed to have a .kunitconfig too to enable the kunit tests easily.
Maxime
A .kuniconfig example is present in the cover letter. My understanding from the docs:
https://docs.kernel.org/dev-tools/kunit/run_wrapper.html#create-a-kunitconfi...
The bit of the documentation you're looking for is https://www.kernel.org/doc/html/latest/dev-tools/kunit/running_tips.html#def... You can create a drivers/gpu/drm/.kunitconfig file and run with $ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm --arch=x86_86
The contents of that file would be just like CONFIG_KUNIT=y CONFIG_DRM=y CONFIG_DRM_KMS_HELPER=y # if no `select` CONFIG_DRM_FORMAR_HELPER_TEST=y
Re "kunit test cases are supposed to have a .kunitconfig too" As I noted in the docs: This is a relatively new feature (5.12+) so we don’t have any conventions yet about on what files should be checked in versus just kept around locally. It’s up to you and your maintainer to decide if a config is useful enough to submit (and therefore have to maintain).
So it's up to whatever people think works best/is worth it. I think in this case, it makes sense to add the file.
Is that, like the .config file, the .kunitconfig file is not meant to be included in git, but I'm sure someone else will clarify this point.
That bit of the docs needs to be rewritten. You're recommended to check in a drivers/gpu/drm/.kunitconfig file into git.
Context: `kunit.py` used to use the "<root>/.kunitconfig" file. Anytime you wanted to run more tests, you'd append to that file. So we agreed that no one should check in that file specifically.
Now kunit.py * uses "<build-dir>/.kunitconfig", by default: ".kunit/.kunitconfig" * has the --kunitconfig flag so you can use different files so it's no longer as relevant.
Hope that helps, Daniel