[v3] Documentation: KUnit: Update filename best practices

Commit Message

Kees Cook July 24, 2024, 8:13 p.m. UTC

Based on feedback from Linus[1] and follow-up discussions, change the
suggested file naming for KUnit tests.

Link: https://lore.kernel.org/lkml/CAHk-=wgim6pNiGTBMhP8Kd3tsB7_JTAuvNJ=XYd3wPvvk=[e-mail]/ [1]
Reviewed-by: John Hubbard <[e-mail]>
Signed-off-by: Kees Cook <[e-mail]>
---
 v3: additional clarification
 v2: https://lore.kernel.org/all/[e-mail]/
Cc: David Gow <[e-mail]>
Cc: Brendan Higgins <[e-mail]>
Cc: Rae Moar <[e-mail]>
Cc: John Hubbard <[e-mail]>
Cc: Jonathan Corbet <[e-mail]>
Cc: Linus Torvalds <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
---
 Documentation/dev-tools/kunit/style.rst | 29 +++++++++++++++++--------
 1 file changed, 20 insertions(+), 9 deletions(-)

Comments

David Gow July 26, 2024, 7:14 a.m. UTC | #1

On Thu, 25 Jul 2024 at 04:14, Kees Cook <[e-mail]> wrote:
>
> Based on feedback from Linus[1] and follow-up discussions, change the
> suggested file naming for KUnit tests.
>
> Link: https://lore.kernel.org/lkml/CAHk-=wgim6pNiGTBMhP8Kd3tsB7_JTAuvNJ=XYd3wPvvk=[e-mail]/ [1]
> Reviewed-by: John Hubbard <[e-mail]>
> Signed-off-by: Kees Cook <[e-mail]>
> ---

Looks good to me. (And it seems like sphinx is okay with it, too,
which is always nice.)

We normally take these changes in via the kunit tree, but if you want
to try to push it through along with the naming changes, that's fine
by me.

Reviewed-by: David Gow <[e-mail]>

Thanks,
-- David

Kees Cook Aug. 23, 2024, 12:18 a.m. UTC | #2

On Fri, Jul 26, 2024 at 03:14:36PM +0800, David Gow wrote:
> On Thu, 25 Jul 2024 at 04:14, Kees Cook <[e-mail]> wrote:
> >
> > Based on feedback from Linus[1] and follow-up discussions, change the
> > suggested file naming for KUnit tests.
> >
> > Link: https://lore.kernel.org/lkml/CAHk-=wgim6pNiGTBMhP8Kd3tsB7_JTAuvNJ=XYd3wPvvk=[e-mail]/ [1]
> > Reviewed-by: John Hubbard <[e-mail]>
> > Signed-off-by: Kees Cook <[e-mail]>
> > ---
> 
> Looks good to me. (And it seems like sphinx is okay with it, too,
> which is always nice.)
> 
> We normally take these changes in via the kunit tree, but if you want
> to try to push it through along with the naming changes, that's fine
> by me.
> 
> Reviewed-by: David Gow <[e-mail]>

Hi! Just a quick check: I haven't seen this land in -next yet. Are you
still planning to take this?

Shuah Khan Aug. 27, 2024, 12:34 p.m. UTC | #3

On 8/22/24 18:18, Kees Cook wrote:
> On Fri, Jul 26, 2024 at 03:14:36PM +0800, David Gow wrote:
>> On Thu, 25 Jul 2024 at 04:14, Kees Cook <[e-mail]> wrote:
>>>
>>> Based on feedback from Linus[1] and follow-up discussions, change the
>>> suggested file naming for KUnit tests.
>>>
>>> Link: https://lore.kernel.org/lkml/CAHk-=wgim6pNiGTBMhP8Kd3tsB7_JTAuvNJ=XYd3wPvvk=[e-mail]/ [1]
>>> Reviewed-by: John Hubbard <[e-mail]>
>>> Signed-off-by: Kees Cook <[e-mail]>
>>> ---
>>
>> Looks good to me. (And it seems like sphinx is okay with it, too,
>> which is always nice.)
>>
>> We normally take these changes in via the kunit tree, but if you want
>> to try to push it through along with the naming changes, that's fine
>> by me.
>>
>> Reviewed-by: David Gow <[e-mail]>
> 
> Hi! Just a quick check: I haven't seen this land in -next yet. Are you
> still planning to take this?
> 

I applied it yesterday to kunit next.

thanks,
-- Shuah

Patch

diff --git a/Documentation/dev-tools/kunit/style.rst b/Documentation/dev-tools/kunit/style.rst
index b6d0d7359f00..eac81a714a29 100644
--- a/Documentation/dev-tools/kunit/style.rst
+++ b/Documentation/dev-tools/kunit/style.rst
@@ -188,15 +188,26 @@  For example, a Kconfig entry might look like:
 Test File and Module Names
 ==========================
 
-KUnit tests can often be compiled as a module. These modules should be named
-after the test suite, followed by ``_test``. If this is likely to conflict with
-non-KUnit tests, the suffix ``_kunit`` can also be used.
+KUnit tests are often compiled as a separate module. To avoid conflicting
+with regular modules, KUnit modules should be named after the test suite,
+followed by ``_kunit`` (e.g. if "foobar" is the core module, then
+"foobar_kunit" is the KUnit test module).
 
-The easiest way of achieving this is to name the file containing the test suite
-``<suite>_test.c`` (or, as above, ``<suite>_kunit.c``). This file should be
-placed next to the code under test.
+Test source files, whether compiled as a separate module or an
+``#include`` in another source file, are best kept in a ``tests/``
+subdirectory to not conflict with other source files (e.g. for
+tab-completion).
+
+Note that the ``_test`` suffix has also been used in some existing
+tests. The ``_kunit`` suffix is preferred, as it makes the distinction
+between KUnit and non-KUnit tests clearer.
+
+So for the common case, name the file containing the test suite
+``tests/<suite>_kunit.c``. The ``tests`` directory should be placed at
+the same level as the code under test. For example, tests for
+``lib/string.c`` live in ``lib/tests/string_kunit.c``.
 
 If the suite name contains some or all of the name of the test's parent
-directory, it may make sense to modify the source filename to reduce redundancy.
-For example, a ``foo_firmware`` suite could be in the ``foo/firmware_test.c``
-file.
+directory, it may make sense to modify the source filename to reduce
+redundancy. For example, a ``foo_firmware`` suite could be in the
+``foo/tests/firmware_kunit.c`` file.