| Message ID | 20200909051631.2960347-1-davidgow@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Documentation: kunit: Add naming guidelines | expand |
On Tue, Sep 08, 2020 at 10:16PM -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. > > [1]: > https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u > > Signed-off-by: David Gow <davidgow@google.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > Reviewed-by: Brendan Higgins <brendanhiggins@google.com> > --- > > This is v2 of the KUnit test nomenclature guidelines. The guidelines have > changed a bit in response to the discussion on the v1 thread which came > about after plumbers. The major change is that the filename suffix is > now "_test", with "_kunit" permitted where it conflicts. There are also > some other exceptions carved out around existing tests, and very > non-unit-like tests. > > Changelog: > > v2: > - Rewrote the filename section to use "_test" as a suffix, and focus on > module names, not filenames. > - Add a motivating introduction, which also calls out existing tests and > tests which cause problems when run automatically (long running, > flaky tests) as reasons to avoid the guidelines. > - Talk about including the type of test in the suite name, but only if > theres an actual confict. (And update the example for this). > > v1: > https://lore.kernel.org/linux-kselftest/20200702071416.1780522-1-davidgow@google.com/ > - Fixed a bit of space/tab confusion in the index (Thanks, Randy) > - Added some more examples (and some test case examples). > - Added some examples of what not to call subsystems and suites. > - No longer explicitly require "If unsure, put N" in Kconfig entries. > - Minor formatting changes > > RFC: > https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@google.com/T/#u > - Initial version > > > The result is a little bit weaker than the previous versions, but > hopefully will let us get the areas we agree on down. > > -- David > > > Documentation/dev-tools/kunit/index.rst | 1 + > Documentation/dev-tools/kunit/style.rst | 207 ++++++++++++++++++++++++ > 2 files changed, 208 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..c234a3ab3c34 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..c001ea1cd87d > --- /dev/null > +++ b/Documentation/dev-tools/kunit/style.rst > @@ -0,0 +1,207 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +=========================== > +Test Style and Nomenclature > +=========================== > + > +To make finding, writing, and using KUnit tests as simple as possible, it's > +strongly encouraged that they are named and written according to the guidelines > +below. While it's possible to write KUnit tests which do not follow these rules, > +they may break some tooling, may conflict with other tests, and may not be run > +automatically by testing systems. > + > +It's recommended that you only deviate from these guidelines when: > + > +1. Porting tests to KUnit which are already known with an existing name, or > +2. Writing tests which would cause serious problems if automatically run (e.g., > + nonderministically producing false positives or negatives, or taking an s/nonderministically/nondeterministically/ (or non-deterministically?) > + extremely long time to run). [...] > +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 suffic ``_kunit`` can also be used. s/suffic/suffix/ > +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. > + > +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. Reviewed-by: Marco Elver <elver@google.com> Thank you!
> -----Original Message----- > From: David Gow > > 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> > Reviewed-by: Kees Cook <keescook@chromium.org> > Reviewed-by: Brendan Higgins <brendanhiggins@google.com> > --- > > This is v2 of the KUnit test nomenclature guidelines. The guidelines have > changed a bit in response to the discussion on the v1 thread which came > about after plumbers. The major change is that the filename suffix is > now "_test", with "_kunit" permitted where it conflicts. There are also > some other exceptions carved out around existing tests, and very > non-unit-like tests. > > Changelog: > > v2: > - Rewrote the filename section to use "_test" as a suffix, and focus on > module names, not filenames. > - Add a motivating introduction, which also calls out existing tests and > tests which cause problems when run automatically (long running, > flaky tests) as reasons to avoid the guidelines. > - Talk about including the type of test in the suite name, but only if > theres an actual confict. (And update the example for this). > > v1: > https://lore.kernel.org/linux-kselftest/20200702071416.1780522-1-davidgow@google.com/ > - Fixed a bit of space/tab confusion in the index (Thanks, Randy) > - Added some more examples (and some test case examples). > - Added some examples of what not to call subsystems and suites. > - No longer explicitly require "If unsure, put N" in Kconfig entries. > - Minor formatting changes > > RFC: > https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@google.com/T/#u > - Initial version > > > The result is a little bit weaker than the previous versions, but > hopefully will let us get the areas we agree on down. > > -- David > > > Documentation/dev-tools/kunit/index.rst | 1 + > Documentation/dev-tools/kunit/style.rst | 207 ++++++++++++++++++++++++ > 2 files changed, 208 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..c234a3ab3c34 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..c001ea1cd87d > --- /dev/null > +++ b/Documentation/dev-tools/kunit/style.rst > @@ -0,0 +1,207 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +=========================== > +Test Style and Nomenclature > +=========================== > + > +To make finding, writing, and using KUnit tests as simple as possible, it's > +strongly encouraged that they are named and written according to the guidelines > +below. While it's possible to write KUnit tests which do not follow these rules, > +they may break some tooling, may conflict with other tests, and may not be run > +automatically by testing systems. > + > +It's recommended that you only deviate from these guidelines when: > + > +1. Porting tests to KUnit which are already known with an existing name, or > +2. Writing tests which would cause serious problems if automatically run (e.g., > + nonderministically producing false positives or negatives, or taking an > + extremely long time to run). > + > +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`` > + Matches the module and filesystem name. > +``apparmor`` > + Matches the module name and LSM name. > +``kasan`` > + Common name for the tool, prominent part of the path ``mm/kasan`` > +``snd_hda_codec_hdmi`` > + Has several components (``snd``, ``hda``, ``codec``, ``hdmi``) separated by > + underscores. Matches the module name. > + > +Avoid names like these: > + > +``linear-ranges`` > + Names should use underscores, not dashes, to separate words. Prefer > + ``linear_ranges``. > +``qos-kunit-test`` > + As well as using underscores, this name should not have "kunit-test" as a > + suffix, and ``qos`` is ambiguous as a subsystem name. ``power_qos`` would be a > + better name. > +``pc_parallel_port`` > + The corresponding module name is ``parport_pc``, so this subsystem should also > + be named ``parport_pc``. > + > +.. 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. > + > +In the event that there are multiple types of test using KUnit within a > +subsystem (e.g., both unit tests and integration tests), they should be put into > +separate suites, with the type of test as the last element in the suite name. > +Unless these tests are actually present, avoid using ``_test``, ``_unittest`` or > +similar in the suite name. > + > +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`` > + Part of the ``ext4`` subsystem, testing the ``inode`` area. > +``kunit_try_catch`` > + Part of the ``kunit`` implementation itself, testing the ``try_catch`` area. > +``apparmor_property_entry`` > + Part of the ``apparmor`` subsystem, testing the ``property_entry`` area. > +``kasan`` > + The ``kasan`` subsystem has only one suite, so the suite name is the same as > + the subsystem name. > + > +Avoid names like: > + > +``ext4_ext4_inode`` > + There's no reason to state the subsystem twice. > +``property_entry`` > + The suite name is ambiguous without the subsystem name. > +``kasan_integration_test`` > + Because there is only one suite in the ``kasan`` subsystem, the suite should > + just be called ``kasan``. There's no need to redundantly add > + ``integration_test``. Should a separate test suite with, for example, unit > + tests be added, then that suite could be named ``kasan_unittest`` or similar. > + > +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. > + > +.. 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. > + > +Example test names include: > + > +``unpack_u32_with_null_name`` > + Tests the ``unpack_u32`` function when a NULL name is passed in. > +``test_list_splice`` > + Tests the ``list_splice`` macro. It has the prefix ``test_`` to avoid a > + name conflict with the macro itself. > + > + > +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 > + > +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 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 suffic ``_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. > + > +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. > + > + > -- > 2.28.0.526.ge36021eeef-goog Looks good! I know this was a lot of hard work to get consensus here. This will help a lot for test frameworks to have consistent and persistent, fully-qualified names for testcases. Good job! Reviewed-by: Tim Bird <tim.bird@sony.com> -- Tim
diff --git a/Documentation/dev-tools/kunit/index.rst b/Documentation/dev-tools/kunit/index.rst index e93606ecfb01..c234a3ab3c34 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..c001ea1cd87d --- /dev/null +++ b/Documentation/dev-tools/kunit/style.rst @@ -0,0 +1,207 @@ +.. SPDX-License-Identifier: GPL-2.0 + +=========================== +Test Style and Nomenclature +=========================== + +To make finding, writing, and using KUnit tests as simple as possible, it's +strongly encouraged that they are named and written according to the guidelines +below. While it's possible to write KUnit tests which do not follow these rules, +they may break some tooling, may conflict with other tests, and may not be run +automatically by testing systems. + +It's recommended that you only deviate from these guidelines when: + +1. Porting tests to KUnit which are already known with an existing name, or +2. Writing tests which would cause serious problems if automatically run (e.g., + nonderministically producing false positives or negatives, or taking an + extremely long time to run). + +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`` + Matches the module and filesystem name. +``apparmor`` + Matches the module name and LSM name. +``kasan`` + Common name for the tool, prominent part of the path ``mm/kasan`` +``snd_hda_codec_hdmi`` + Has several components (``snd``, ``hda``, ``codec``, ``hdmi``) separated by + underscores. Matches the module name. + +Avoid names like these: + +``linear-ranges`` + Names should use underscores, not dashes, to separate words. Prefer + ``linear_ranges``. +``qos-kunit-test`` + As well as using underscores, this name should not have "kunit-test" as a + suffix, and ``qos`` is ambiguous as a subsystem name. ``power_qos`` would be a + better name. +``pc_parallel_port`` + The corresponding module name is ``parport_pc``, so this subsystem should also + be named ``parport_pc``. + +.. 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. + +In the event that there are multiple types of test using KUnit within a +subsystem (e.g., both unit tests and integration tests), they should be put into +separate suites, with the type of test as the last element in the suite name. +Unless these tests are actually present, avoid using ``_test``, ``_unittest`` or +similar in the suite name. + +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`` + Part of the ``ext4`` subsystem, testing the ``inode`` area. +``kunit_try_catch`` + Part of the ``kunit`` implementation itself, testing the ``try_catch`` area. +``apparmor_property_entry`` + Part of the ``apparmor`` subsystem, testing the ``property_entry`` area. +``kasan`` + The ``kasan`` subsystem has only one suite, so the suite name is the same as + the subsystem name. + +Avoid names like: + +``ext4_ext4_inode`` + There's no reason to state the subsystem twice. +``property_entry`` + The suite name is ambiguous without the subsystem name. +``kasan_integration_test`` + Because there is only one suite in the ``kasan`` subsystem, the suite should + just be called ``kasan``. There's no need to redundantly add + ``integration_test``. Should a separate test suite with, for example, unit + tests be added, then that suite could be named ``kasan_unittest`` or similar. + +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. + +.. 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. + +Example test names include: + +``unpack_u32_with_null_name`` + Tests the ``unpack_u32`` function when a NULL name is passed in. +``test_list_splice`` + Tests the ``list_splice`` macro. It has the prefix ``test_`` to avoid a + name conflict with the macro itself. + + +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 + +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 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 suffic ``_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. + +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. + +