As discussed in [1], KUnit tests have hitherto not had a particularly consistent naming scheme. This adds documentation outlining how tests and test suites should be named, including how those names should be used in Kconfig entries and filenames.
[1]: https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
Signed-off-by: David Gow davidgow@google.com --- This is a first draft of some naming guidelines for KUnit tests. Note that I haven't edited it for spelling/grammar/style yet: I wanted to get some feedback on the actual naming conventions first.
The issues which came most to the forefront while writing it were: - Do we want to make subsystems a more explicit thing (make the KUnit framework recognise them, make suites KTAP subtests of them, etc) - I'm leaning towards no, mainly because it doesn't seem necessary, and it makes the subsystem-with-only-one-suite case ugly.
- Do we want to support (or encourage) Kconfig options and/or modules at the subsystem level rather than the suite level? - This could be nice: it'd avoid the proliferation of a large number of tiny config options and modules, and would encourage the test for <module> to be <module>_kunit, without other stuff in-between.
- As test names are also function names, it may actually make sense to decorate them with "test" or "kunit" or the like. - If we're testing a function "foo", "test_foo" seems like as good a name for the function as any. Sure, many cases may could have better names like "foo_invalid_context" or something, but that won't make sense for everything. - Alternatively, do we split up the test name and the name of the function implementing the test?
Thoughts?
Documentation/dev-tools/kunit/index.rst | 1 + Documentation/dev-tools/kunit/style.rst | 139 ++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 Documentation/dev-tools/kunit/style.rst
diff --git a/Documentation/dev-tools/kunit/index.rst b/Documentation/dev-tools/kunit/index.rst index e93606ecfb01..117c88856fb3 100644 --- a/Documentation/dev-tools/kunit/index.rst +++ b/Documentation/dev-tools/kunit/index.rst @@ -11,6 +11,7 @@ KUnit - Unit Testing for the Linux Kernel usage kunit-tool api/index + style faq
What is KUnit? diff --git a/Documentation/dev-tools/kunit/style.rst b/Documentation/dev-tools/kunit/style.rst new file mode 100644 index 000000000000..9363b5607262 --- /dev/null +++ b/Documentation/dev-tools/kunit/style.rst @@ -0,0 +1,139 @@ +.. SPDX-License-Identifier: GPL-2.0 + +=========================== +Test Style and Nomenclature +=========================== + +Subsystems, Suites, and Tests +============================= + +In order to make tests as easy to find as possible, they're grouped into suites +and subsystems. A test suite is a group of tests which test a related area of +the kernel, and a subsystem is a set of test suites which test different parts +of the same kernel subsystem or driver. + +Subsystems +---------- + +Every test suite must belong to a subsystem. A subsystem is a collection of one +or more KUnit test suites which test the same driver or part of the kernel. A +rule of thumb is that a test subsystem should match a single kernel module. If +the code being tested can't be compiled as a module, in many cases the subsystem +should correspond to a directory in the source tree or an entry in the +MAINTAINERS file. If unsure, follow the conventions set by tests in similar +areas. + +Test subsystems should be named after the code being tested, either after the +module (wherever possible), or after the directory or files being tested. Test +subsystems should be named to avoid ambiguity where necessary. + +If a test subsystem name has multiple components, they should be separated by +underscores. Do not include "test" or "kunit" directly in the subsystem name +unless you are actually testing other tests or the kunit framework itself. + +Example subsystems could be: + +* ``ext4`` +* ``apparmor`` +* ``kasan`` + +.. note:: + The KUnit API and tools do not explicitly know about subsystems. They're + simply a way of categorising test suites and naming modules which + provides a simple, consistent way for humans to find and run tests. This + may change in the future, though. + +Suites +------ + +KUnit tests are grouped into test suites, which cover a specific area of +functionality being tested. Test suites can have shared initialisation and +shutdown code which is run for all tests in the suite. +Not all subsystems will need to be split into multiple test suites (e.g. simple drivers). + +Test suites are named after the subsystem they are part of. If a subsystem +contains several suites, the specific area under test should be appended to the +subsystem name, separated by an underscore. + +The full test suite name (including the subsystem name) should be specified as +the ``.name`` member of the ``kunit_suite`` struct, and forms the base for the +module name (see below). + +Example test suites could include: + +* ``ext4_inode`` +* ``kunit_try_catch`` +* ``apparmor_property_entry`` +* ``kasan`` + +Tests +----- + +Individual tests consist of a single function which tests a constrained +codepath, property, or function. In the test output, individual tests' results +will show up as subtests of the suite's results. + +Tests should be named after what they're testing. This is often the name of the +function being tested, with a description of the input or codepath being tested. +As tests are C functions, they should be named and written in accordance with +the kernel coding style. + +.. note:: + As tests are themselves functions, their names cannot conflict with + other C identifiers in the kernel. This may require some creative + naming. It's a good idea to make your test functions `static` to avoid + polluting the global namespace. + +Should it be necessary to refer to a test outside the context of its test suite, +the *fully-qualified* name of a test should be the suite name followed by the +test name, separated by a colon (i.e. ``suite:test``). + +Test Kconfig Entries +==================== + +Every test suite should be tied to a Kconfig entry. + +This Kconfig entry must: + +* be named ``CONFIG_<name>_KUNIT_TEST``: where <name> is the name of the test + suite. +* be listed either alongside the config entries for the driver/subsystem being + tested, or be under [Kernel Hacking]→[Kernel Testing and Coverage] +* depend on ``CONFIG_KUNIT`` +* be visible only if ``CONFIG_KUNIT_ALL_TESTS`` is not enabled. +* have a default value of ``CONFIG_KUNIT_ALL_TESTS``. +* have a brief description of KUnit in the help text +* include "If unsure, say N" in the help text + +Unless there's a specific reason not to (e.g. the test is unable to be built as +a module), Kconfig entries for tests should be tristate. + +An example Kconfig entry: + +.. code-block:: none + + config FOO_KUNIT_TEST + tristate "KUnit test for foo" if !KUNIT_ALL_TESTS + depends on KUNIT + default KUNIT_ALL_TESTS + help + This builds unit tests for foo. + + For more information on KUnit and unit tests in general, please refer + to the KUnit documentation in Documentation/dev-tools/kunit + + If unsure, say N + + +Test Filenames +============== + +Where possible, test suites should be placed in a separate source file in the +same directory as the code being tested. + +This file should be named ``<suite>_kunit.c``. It may make sense to strip +excessive namespacing from the source filename (e.g., ``firmware_kunit.c`` instead of +``<drivername>_firmware.c``), but please ensure the module name does contain the +full suite name. + +
Hi--
On 6/19/20 10:49 PM, David Gow wrote:
Documentation/dev-tools/kunit/index.rst | 1 + Documentation/dev-tools/kunit/style.rst | 139 ++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 Documentation/dev-tools/kunit/style.rst
diff --git a/Documentation/dev-tools/kunit/index.rst b/Documentation/dev-tools/kunit/index.rst index e93606ecfb01..117c88856fb3 100644 --- a/Documentation/dev-tools/kunit/index.rst +++ b/Documentation/dev-tools/kunit/index.rst @@ -11,6 +11,7 @@ KUnit - Unit Testing for the Linux Kernel usage kunit-tool api/index
style
Use tab for indentation, not spaces.
faq
thanks.
I imagine +Theodore Ts'o might have some thoughts on this.
+Bird, Timothy - Figured you might be interested since I think this might pertain to the KTAP discussion.
On Fri, Jun 19, 2020 at 10:50 PM David Gow davidgow@google.com wrote:
As discussed in [1], KUnit tests have hitherto not had a particularly consistent naming scheme. This adds documentation outlining how tests and test suites should be named, including how those names should be used in Kconfig entries and filenames.
Signed-off-by: David Gow davidgow@google.com
This is a first draft of some naming guidelines for KUnit tests. Note that I haven't edited it for spelling/grammar/style yet: I wanted to get some feedback on the actual naming conventions first.
The issues which came most to the forefront while writing it were:
Do we want to make subsystems a more explicit thing (make the KUnit framework recognise them, make suites KTAP subtests of them, etc)
- I'm leaning towards no, mainly because it doesn't seem necessary, and it makes the subsystem-with-only-one-suite case ugly.
Do we want to support (or encourage) Kconfig options and/or modules at the subsystem level rather than the suite level?
- This could be nice: it'd avoid the proliferation of a large number of tiny config options and modules, and would encourage the test for <module> to be <module>_kunit, without other stuff in-between.
As test names are also function names, it may actually make sense to decorate them with "test" or "kunit" or the like.
- If we're testing a function "foo", "test_foo" seems like as good a name for the function as any. Sure, many cases may could have better names like "foo_invalid_context" or something, but that won't make sense for everything.
- Alternatively, do we split up the test name and the name of the function implementing the test?
Thoughts?
Overall it looks pretty good. I would like to see some examples fleshed out a bit more or at least say how things like subsystem names are used, but otherwise this looks good to me.
Documentation/dev-tools/kunit/index.rst | 1 + Documentation/dev-tools/kunit/style.rst | 139 ++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 Documentation/dev-tools/kunit/style.rst
diff --git a/Documentation/dev-tools/kunit/index.rst b/Documentation/dev-tools/kunit/index.rst index e93606ecfb01..117c88856fb3 100644 --- a/Documentation/dev-tools/kunit/index.rst +++ b/Documentation/dev-tools/kunit/index.rst @@ -11,6 +11,7 @@ KUnit - Unit Testing for the Linux Kernel usage kunit-tool api/index
style faqWhat is KUnit? diff --git a/Documentation/dev-tools/kunit/style.rst b/Documentation/dev-tools/kunit/style.rst new file mode 100644 index 000000000000..9363b5607262 --- /dev/null +++ b/Documentation/dev-tools/kunit/style.rst @@ -0,0 +1,139 @@ +.. SPDX-License-Identifier: GPL-2.0
+=========================== +Test Style and Nomenclature +===========================
+Subsystems, Suites, and Tests +=============================
+In order to make tests as easy to find as possible, they're grouped into suites +and subsystems. A test suite is a group of tests which test a related area of +the kernel, and a subsystem is a set of test suites which test different parts +of the same kernel subsystem or driver.
+Subsystems +----------
+Every test suite must belong to a subsystem. A subsystem is a collection of one +or more KUnit test suites which test the same driver or part of the kernel. A +rule of thumb is that a test subsystem should match a single kernel module. If +the code being tested can't be compiled as a module, in many cases the subsystem +should correspond to a directory in the source tree or an entry in the +MAINTAINERS file. If unsure, follow the conventions set by tests in similar +areas.
+Test subsystems should be named after the code being tested, either after the +module (wherever possible), or after the directory or files being tested. Test +subsystems should be named to avoid ambiguity where necessary.
+If a test subsystem name has multiple components, they should be separated by +underscores. Do not include "test" or "kunit" directly in the subsystem name
nit: Embolden "Do not".
+unless you are actually testing other tests or the kunit framework itself.
+Example subsystems could be:
+* ``ext4`` +* ``apparmor`` +* ``kasan``
Maybe add some examples that exercise the "multiple components ... separated by underscores". Some negative examples might also be good since we currently violate this rule.
+.. note::
The KUnit API and tools do not explicitly know about subsystems. They'resimply a way of categorising test suites and naming modules whichprovides a simple, consistent way for humans to find and run tests. Thismay change in the future, though.
I think we should have some way to enshrine this in KUnit, if not via code, I think we should at least say how the convention is used.
+Suites +------
+KUnit tests are grouped into test suites, which cover a specific area of +functionality being tested. Test suites can have shared initialisation and +shutdown code which is run for all tests in the suite. +Not all subsystems will need to be split into multiple test suites (e.g. simple drivers).
+Test suites are named after the subsystem they are part of. If a subsystem +contains several suites, the specific area under test should be appended to the +subsystem name, separated by an underscore.
+The full test suite name (including the subsystem name) should be specified as +the ``.name`` member of the ``kunit_suite`` struct, and forms the base for the +module name (see below).
+Example test suites could include:
+* ``ext4_inode`` +* ``kunit_try_catch`` +* ``apparmor_property_entry`` +* ``kasan``
+Tests
nit: "Test Cases".
+-----
+Individual tests consist of a single function which tests a constrained +codepath, property, or function. In the test output, individual tests' results +will show up as subtests of the suite's results.
+Tests should be named after what they're testing. This is often the name of the +function being tested, with a description of the input or codepath being tested. +As tests are C functions, they should be named and written in accordance with +the kernel coding style.
Can you add an example?
+.. note::
As tests are themselves functions, their names cannot conflict withother C identifiers in the kernel. This may require some creativenaming. It's a good idea to make your test functions `static` to avoidpolluting the global namespace.+Should it be necessary to refer to a test outside the context of its test suite, +the *fully-qualified* name of a test should be the suite name followed by the +test name, separated by a colon (i.e. ``suite:test``).
+Test Kconfig Entries +====================
+Every test suite should be tied to a Kconfig entry.
+This Kconfig entry must:
+* be named ``CONFIG_<name>_KUNIT_TEST``: where <name> is the name of the test
- suite.
+* be listed either alongside the config entries for the driver/subsystem being
- tested, or be under [Kernel Hacking]→[Kernel Testing and Coverage]
+* depend on ``CONFIG_KUNIT`` +* be visible only if ``CONFIG_KUNIT_ALL_TESTS`` is not enabled. +* have a default value of ``CONFIG_KUNIT_ALL_TESTS``. +* have a brief description of KUnit in the help text +* include "If unsure, say N" in the help text
+Unless there's a specific reason not to (e.g. the test is unable to be built as +a module), Kconfig entries for tests should be tristate.
+An example Kconfig entry:
+.. code-block:: none
config FOO_KUNIT_TESTtristate "KUnit test for foo" if !KUNIT_ALL_TESTSdepends on KUNITdefault KUNIT_ALL_TESTShelpThis builds unit tests for foo.For more information on KUnit and unit tests in general, please referto the KUnit documentation in Documentation/dev-tools/kunitIf unsure, say N+Test Filenames +==============
+Where possible, test suites should be placed in a separate source file in the +same directory as the code being tested.
+This file should be named ``<suite>_kunit.c``. It may make sense to strip +excessive namespacing from the source filename (e.g., ``firmware_kunit.c`` instead of +``<drivername>_firmware.c``), but please ensure the module name does contain the +full suite name.
-- 2.27.0.111.gc72c7da667-goog
On Fri, Jun 19, 2020 at 10:49:44PM -0700, David Gow wrote:
As discussed in [1], KUnit tests have hitherto not had a particularly consistent naming scheme. This adds documentation outlining how tests and test suites should be named, including how those names should be used in Kconfig entries and filenames.
Thanks for preparing this!
[...] +Test Kconfig Entries +====================
+Every test suite should be tied to a Kconfig entry.
+This Kconfig entry must:
+* be named ``CONFIG_<name>_KUNIT_TEST``: where <name> is the name of the test
- suite.
+* be listed either alongside the config entries for the driver/subsystem being
- tested, or be under [Kernel Hacking]→[Kernel Testing and Coverage]
+* depend on ``CONFIG_KUNIT`` +* be visible only if ``CONFIG_KUNIT_ALL_TESTS`` is not enabled. +* have a default value of ``CONFIG_KUNIT_ALL_TESTS``. +* have a brief description of KUnit in the help text +* include "If unsure, say N" in the help text
Is this last one needed? It seems redundant -- I'm not sure any of the normal Kconfig help needs this. *shrug*
Everything else looks good!
Reviewed-by: Kees Cook keescook@chromium.org
linux-kselftest-mirror@lists.linaro.org
-
Brendan Higgins -
David Gow -
Kees Cook -
Randy Dunlap