Re: [PATCH v8 0/8] Improve test coverage of TTM

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Karolina,

On Wed, Dec 06, 2023 at 12:28:14PM +0100, Karolina Stolarek wrote:
> On 5.12.2023 16:39, Andi Shyti wrote:
...
> > Hi Karolina and Christian,
...
> > > Karolina Stolarek (8): drm/ttm/tests: Add tests for ttm_resource and
> > > ttm_sys_man drm/ttm/tests: Add tests for ttm_tt drm/ttm/tests:
> > >  Add tests for ttm_bo functions drm/ttm/tests: Fix argument in
> > > ttm_tt_kunit_init() drm/ttm/tests: Use an init function from the
> > > helpers lib drm/ttm/tests: Test simple BO creation and validation
> > > drm/ttm/tests: Add tests with mock resource managers drm/ttm/tests:
> > > Add test cases dependent on fence signaling
> > 
> > I see that all these files (and previous patches, as well) don't have
> > a consistent prefix system, like kunit_ttm_*. But they all start with
> > ttm_*, thread_*, drm_*, etc, which makes it a bit difficult to follow
> > and grep through.
> 
> I see what you mean. As for ttm_* part, I decided to keep it as it is
> based on what was in DRM KUnit tests and helpers lib. Almost all
> functions and subtests start with drm_*, and as I'm working on TTM
> tests, ttm_* prefix seemed like a natural choice. With thread_*, I could
> add ttm_kunit_* in front of it, but not sure what would be the benefit
> of doing this.

I do not agree here, ttm_* are related to ttm and not "testing
ttm" and we need to be consistent.

I'd be more inclined to a kunit_ttm_* or ttm_kunit_*.

Anyway, let's hear also what Christian thinks.

> To be honest, I haven't considered using kunit_* prefix here. In the
> code context, it's obvious these are kunit tests, and repeating that
> information would make already long function/struct names even longer.
> 
> I had a quick glance at other KUnit tests in the kernel and this seems
> to be the case, no kunit_* prefixes are used.
> 
> > Can we please maintain a consistent prefix system?
> > 
> > I know it looks bad to come out with it after this series (and previous
> > work too) has been out for several months already. I need to
> > say my "mea culpa" as well as I've been in the review loop, as well.
> 
> After this series, I plan to send a small one to wrap up my work in this
> field. How about adding a TODO to clean these up? I mean, that's
> just how I see it, I'd like to hear Christian's thoughts on this.

nah... a TODO note would be too much... your promise is enough
for me.

Andi