Skip to contentSkip to conversation

Documentation: KUnit: Update filename best practices

PATCH Superseded Kees Cook 3 participants 5 messages Jul 17, 2024 – Jul 18, 2024

Subsystems (4)

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Kees Cook

author
#0 lore raw

PATCH 1/1 Documentation: KUnit: Update filename best practices

Based on feedback from Linus[1], change the suggested file naming for KUnit tests.

---
Cc: David Gow <[email protected]>
Cc: Brendan Higgins <[email protected]>
Cc: Rae Moar <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

---
 Documentation/dev-tools/kunit/style.rst | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
Documentation/dev-tools/kunit/style.rst +13 -8
index b6d0d7359f00..761dee3f89ca 100644
@@ -188,15 +188,20 @@ 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.
-
-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.
+Whether a KUnit test is compiled as a separate module or via an
+``#include`` in a core kernel source file, the files should be named
+after the test suite, followed by ``_test``, and live in a ``tests``
+subdirectory to avoid conflicting with regular modules or the core kernel
+source file names (e.g. for tab-completion). If this would conflict with
+non-KUnit tests, the suffix ``_kunit`` can be used instead.
+
+So for the common case, name the file containing the test suite
+``tests/<suite>_test.c`` (or, if needed, ``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_test.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``
+For example, a ``foo_firmware`` suite could be in the ``tests/foo/firmware_test.c``
 file.
Signature · 1 line
2.34.1
#1 lore raw
Quoted 26 lines · Kees Cook

Based on feedback from Linus[1], change the suggested file naming for KUnit tests.

---
Cc: David Gow <[email protected]>
Cc: Brendan Higgins <[email protected]>
Cc: Rae Moar <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

---
  Documentation/dev-tools/kunit/style.rst | 21 +++++++++++++--------
  1 file changed, 13 insertions(+), 8 deletions(-)
Documentation/dev-tools/kunit/style.rst +0 -0
index b6d0d7359f00..761dee3f89ca 100644
@@ -188,15 +188,20 @@ For example, a Kconfig entry might look like:
  Test File and Module Names
  ==========================
 

Documentation/dev-tools/kunit/style.rst For example, a Kconfig entry might look like: · line 193

Quoted 10 lines
Documentation/dev-tools/kunit/style.rst +3 -7
-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.
-
-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.
+Whether a KUnit test is compiled as a separate module or via an
+``#include`` in a core kernel source file, the files should be named
+after the test suite, followed by ``_test``, and live in a ``tests``

I read the previous discussion in the other thread and thought about it. And ran some kunit tests on baremetal. Delightful! I love this approach.

However! It is rather distinct and not just any old test module. Kunit has clear conventions and behavior.

As such, I have quickly become convinced that distinct naming is required here. So I'd like to suggest going with the the suffix:

_kunit

...unconditionally. After all, sometimes you'll end up with that anyway, so clearly, the _test suffix isn't strictly required.

And given that we are putting these in tests/ , a _test suffix is redundant.

Yes?

Quoted 5 lines
Documentation/dev-tools/kunit/style.rst +5 -0
+subdirectory to avoid conflicting with regular modules or the core kernel
+source file names (e.g. for tab-completion). If this would conflict with
+non-KUnit tests, the suffix ``_kunit`` can be used instead.
+
+So for the common case, name the file containing the test suite

Documentation/dev-tools/kunit/style.rst For example, a Kconfig entry might look like: · line 207

Quoted 10 lines
Documentation/dev-tools/kunit/style.rst +5 -1
+``tests/<suite>_test.c`` (or, if needed, ``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_test.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``
+For example, a ``foo_firmware`` suite could be in the ``tests/foo/firmware_test.c``
  file.

thanks,

Signature · 2 lines
John Hubbard
NVIDIA
author
#2 lore raw
Quoted 48 lines · John Hubbard
Quoted 25 lines · Kees Cook

Based on feedback from Linus[1], change the suggested file naming for KUnit tests.

---
Cc: David Gow <[email protected]>
Cc: Brendan Higgins <[email protected]>
Cc: Rae Moar <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

---
  Documentation/dev-tools/kunit/style.rst | 21 +++++++++++++--------
  1 file changed, 13 insertions(+), 8 deletions(-)
Documentation/dev-tools/kunit/style.rst +0 -0
index b6d0d7359f00..761dee3f89ca 100644
@@ -188,15 +188,20 @@ For example, a Kconfig entry might look like:
  Test File and Module Names
  ==========================

Documentation/dev-tools/kunit/style.rst For example, a Kconfig entry might look like: · line 192

Quoted 10 lines
Documentation/dev-tools/kunit/style.rst +3 -7
-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.
-
-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.
+Whether a KUnit test is compiled as a separate module or via an
+``#include`` in a core kernel source file, the files should be named
+after the test suite, followed by ``_test``, and live in a ``tests``

I read the previous discussion in the other thread and thought about it. And ran some kunit tests on baremetal. Delightful! I love this approach.

However! It is rather distinct and not just any old test module. Kunit has clear conventions and behavior.

As such, I have quickly become convinced that distinct naming is required here. So I'd like to suggest going with the the suffix:

_kunit

Quoted 7 lines

...unconditionally. After all, sometimes you'll end up with that anyway, so clearly, the _test suffix isn't strictly required.

And given that we are putting these in tests/ , a _test suffix is redundant.

Yes?

I would agree. David, what do you think? I realize drm already does tests/*_test.c, but it does seem like better information density to use the tests/*_kunit.c pattern by default?

Signature · 1 line
Kees Cook
#3 lore raw
Quoted 42 lines · Kees Cook
Quoted 40 lines · John Hubbard
Quoted 25 lines · Kees Cook

Based on feedback from Linus[1], change the suggested file naming for KUnit tests.

---
Cc: David Gow <[email protected]>
Cc: Brendan Higgins <[email protected]>
Cc: Rae Moar <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]

---
  Documentation/dev-tools/kunit/style.rst | 21 +++++++++++++--------
  1 file changed, 13 insertions(+), 8 deletions(-)
Documentation/dev-tools/kunit/style.rst +0 -0
index b6d0d7359f00..761dee3f89ca 100644
@@ -188,15 +188,20 @@ For example, a Kconfig entry might look like:
  Test File and Module Names
  ==========================

Documentation/dev-tools/kunit/style.rst For example, a Kconfig entry might look like: · line 192

Quoted 10 lines
Documentation/dev-tools/kunit/style.rst +3 -7
-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.
-
-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.
+Whether a KUnit test is compiled as a separate module or via an
+``#include`` in a core kernel source file, the files should be named
+after the test suite, followed by ``_test``, and live in a ``tests``

I read the previous discussion in the other thread and thought about it. And ran some kunit tests on baremetal. Delightful! I love this approach.

Quoted 2 lines
Quoted 2 lines

However! It is rather distinct and not just any old test module. Kunit has clear conventions and behavior.

I fear the conventions and behaviour aren't _quite_ as clear as would be ideal (for both good and bad reasons). In particular:
- Tests which don't use KUnit nevertheless are starting to have some KUnit-like behaviour (e.g., KTAP-formatted output); and
- There exist some non-unit tests (e.g., slower integration-y tests) which nevertheless use the KUnit framework

When we last discussed the naming scheme, one proposal was to use _kunit only for tests which were fully "unit" tests (excluding the last batch), though now that we have the 'speed' attribute and similar, I don't think that's as useful as it once was.

Quoted 8 lines
Quoted 8 lines

As such, I have quickly become convinced that distinct naming is required here. So I'd like to suggest going with the the suffix:

_kunit

...unconditionally. After all, sometimes you'll end up with that anyway, so clearly, the _test suffix isn't strictly required.

Quoted 8 lines
Quoted 4 lines

And given that we are putting these in tests/ , a _test suffix is redundant.

Yes?

I would agree. David, what do you think? I realize drm already does tests/*_test.c, but it does seem like better information density to use the tests/*_kunit.c pattern by default?

I personally don't mind either way. Other than the argument that KUnit is simply an implementation detail (and that renaming things makes porting tests to KUnit slightly more annoying), I don't think there's anything wrong with using _kunit, and it would give users an easier way to know if they can use KUnit tooling with it.

Let's do _kunit by default, then.

-- David

#4 lore raw
In reply to Kees Cook
Quoted 6 lines · Kees Cook

Based on feedback from Linus[1], change the suggested file naming for KUnit tests.

Quoted 10 lines

---
Cc: David Gow <[email protected]>
Cc: Brendan Higgins <[email protected]>
Cc: Rae Moar <[email protected]>
Cc: Jonathan Corbet <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---

Looks good to me. Maybe we could make it clearer that the suffix is important for the module name, so if the module is made of multiple source files, it will need to have the _test (or, now, _kunit) suffix added in the Makefile.

Having the extra suffix on the module name shouldn't annoy modprobe as much, as it doesn't need the file extension. So, e.g., if we have foo_bar.ko and tests/foo_bar_kunit.ko:
- insmod doesn't have tab completion issues, as insmod foo[TAB] will complete the filename to foo_bar.ko.
- modprobe also is fine, as modprobe foo[TAB] will complete the module name partially to foo_bar (and adding _k[TAB] will complete to foo_bar_kunit).

(It could still be annoying with fancy shells which show menus, or something, but they'll be equally annoyed by all of the other options, as far as I can tell.)

So:
- s/_test/_kunit for the default.
- Explicitly mention the module name, in addition to the filename. Maybe also "if the module is made of multiple source files, specify the module name (with the _kunit suffix) in the Makefile" or similar.

Cheers,
-- David

Quoted 39 lines
 Documentation/dev-tools/kunit/style.rst | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
Documentation/dev-tools/kunit/style.rst +13 -9
index b6d0d7359f00..761dee3f89ca 100644
@@ -188,15 +188,20 @@ 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.
-
-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.
+Whether a KUnit test is compiled as a separate module or via an
+``#include`` in a core kernel source file, the files should be named
+after the test suite, followed by ``_test``, and live in a ``tests``
+subdirectory to avoid conflicting with regular modules or the core kernel
+source file names (e.g. for tab-completion). If this would conflict with
+non-KUnit tests, the suffix ``_kunit`` can be used instead.
+
+So for the common case, name the file containing the test suite
+``tests/<suite>_test.c`` (or, if needed, ``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_test.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``
+For example, a ``foo_firmware`` suite could be in the ``tests/foo/firmware_test.c``
 file.
--

2.34.1