Re: [PATCH v3 0/3] KUnit tests for drm_format_helper

From: David Gow <davidgow@google.com>
To: "José Expósito" <jose.exposito89@gmail.com>,
	"Maíra Canal" <maira.canal@usp.br>,
	"Isabella Basso" <isabbasso@riseup.net>,
	magalilemes00@gmail.com, tales.aparecida@gmail.com
Cc: Javier Martinez Canillas <javierm@redhat.com>,
	Daniel Latypov <dlatypov@google.com>,
	tzimmermann@suse.de, maarten.lankhorst@linux.intel.com,
	mripard@kernel.org, David Airlie <airlied@linux.ie>,
	Daniel Vetter <daniel@ffwll.ch>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	dri-devel@lists.freedesktop.org,
	KUnit Development <kunit-dev@googlegroups.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 0/3] KUnit tests for drm_format_helper
Date: Thu, 16 Jun 2022 22:44:36 +0800	[thread overview]
Message-ID: <CABVgOSn3zAGsphdEpuevhTo1xoYqHWNF4qty=gR22LcRiUkz0A@mail.gmail.com> (raw)
In-Reply-To: <20220613171738.111013-1-jose.exposito89@gmail.com>

On Tue, Jun 14, 2022 at 1:17 AM José Expósito <jose.exposito89@gmail.com> wrote:
>
> Hello everyone,
>
> Here is the v3 of the series, including the documentation, previously
> sent as a standalone patch [1], and changes suggested during review.
>
> Thanks a lot,
> José Expósito
>

[+Maíra, Isabella, Tales, Magali for other drm,amdgpu,KUnit work.]

These seem pretty good to me, but I'd echo Javier's comments about
consistency with other DRM tests.

In particular, we now have three concurrently developed DRM-related
test suites, each doing things slightly differently:
- This series is putting tests in drm/kunit, and providing a
.kunitconfig in that directory,
- The selftest ports here[1] are putting tests in drm/tests, and
provide a separate Kconfig file, as well as a .kunitconfig
- And the AMDGPU tests[2] are doing something totally different, with
their own tests in drm/amd/display/amdgpu_dm/tests, which get compiled
directly into the amdgpu module (and, at present, can't be run at all
via kunit_tool)

Certainly the general DRM tests should be in the same place, and use
the same Kconfig entries, etc. A mix of the separate Kconfig file from
[1] (if there's enough benefit to having the ability to turn on and
off suites individually, which seems plausible) and the documentation
from this series seems good to me.

There's some basic guidelines around test nomenclature in
Documentation/dev-tools/kunit/style.rst[3], though all of these
patches seem pretty consistent with that. Either 'kunit' or 'tests'
would work as a directory name: given the AMDGPU patches are using
'tests', maybe that's easier to stick with.

Cheers,
-- David

[1]: https://lore.kernel.org/linux-kselftest/20220615135824.15522-1-maira.canal@usp.br/
[2]: https://lore.kernel.org/dri-devel/20220608010709.272962-1-maira.canal@usp.br/
[3]: https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html