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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Andi,

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.

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.

All the best,
Karolina



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux