Re: [RFC PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()

From: Daniel Latypov <dlatypov@google.com>
To: Javier Martinez Canillas <javierm@redhat.com>
Cc: "David Gow" <davidgow@google.com>,
	"David Airlie" <airlied@linux.ie>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	dri-devel@lists.freedesktop.org,
	"Maxime Ripard" <maxime@cerno.tech>,
	tzimmermann@suse.de, "José Expósito" <jose.exposito89@gmail.com>,
	"KUnit Development" <kunit-dev@googlegroups.com>
Subject: Re: [RFC PATCH 1/1] drm/format-helper: Add KUnit tests for drm_fb_xrgb8888_to_rgb332()
Date: Thu, 2 Jun 2022 10:45:23 -0700	[thread overview]
Message-ID: <CAGS_qxqpiCim_sy1LDK7PLwVgWf-LKW+uNFTGM=T7ydk-dYcEw@mail.gmail.com> (raw)
In-Reply-To: <fea8f80a-939b-2c73-d94b-5179d1e65be9@redhat.com>

On Thu, Jun 2, 2022 at 10:29 AM Javier Martinez Canillas
<javierm@redhat.com> wrote:
> > Or, without the .kunitconfig:
> > ./tools/testing/kunit/kunit.py run --kconfig_add CONFIG_DRM=y
> > --kconfig_add CONFIG_DRM_FORMAR_HELPER_TEST=y --kconfig_add
> > CONFIG_VIRTIO_UML=y  --kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y
> > 'drm-*'
> >
>
> I wonder if would make sense to have for example an arch/um/.kunitconfig
> with those symbols and maybe others and then the tests could also be run
> with something like:
>
> ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/.kunitconfig \
> --kunitconfig=arch/um/.kunitconfig

Yeah, this came up before.
It'd be nice to have
* --kunitconfig be repeatable (it isn't right now)
* a "uml_pci.config" with the magic needed to set CONFIG_PCI=y on UML

Another example where this would be useful, coverage on UML
CONFIG_DEBUG_KERNEL=y
CONFIG_DEBUG_INFO=y
CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT=y
CONFIG_GCOV=y

I did prototype the changes to support this but never sent anything
out since I had some concerns, namely:
1. we'd be blindly appending them, but that won't always work. Would
users be ok with that?
2. people are already confused about .kunitconfig. It seems like the
most confusing part to new people, especially those new to kernel
development. Would adding this make them even more lost? Perhaps
making the docs clearer on this would a good pre-req.
3. What conventions should there be around these partial configs? I
think the idea should be more generic than just kunit.
4. --kconfig_add now makes this possible, even if in a noisier way
than another --kunitconfig
5. we didn't have a good way of reporting options set to different
values. https://lore.kernel.org/linux-kselftest/20220520224200.3764027-1-dlatypov@google.com/
would help by giving us an easier way to give clearer error messages.

That said, I'm willing to whip up another version based on the patch in #5.
I think we need a docs rewrite for #2, which I can take a stab at.
But I think we'll need some bikeshedding about naming (#3).

Daniel