linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Gow <davidgow@google.com>
To: Brendan Higgins <brendanhiggins@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Kees Cook <keescook@chromium.org>,
	Alan Maguire <alan.maguire@oracle.com>
Cc: kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	David Gow <davidgow@google.com>
Subject: [PATCH] Documentation: kunit: Add naming guidelines
Date: Fri, 19 Jun 2020 22:49:44 -0700	[thread overview]
Message-ID: <20200620054944.167330-1-davidgow@google.com> (raw)

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.
+
+
-- 
2.27.0.111.gc72c7da667-goog


             reply	other threads:[~2020-06-20  5:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-20  5:49 David Gow [this message]
2020-06-22  3:55 ` [PATCH] Documentation: kunit: Add naming guidelines Randy Dunlap
2020-06-22 21:33 ` Brendan Higgins
2020-06-22 21:41 ` Kees Cook
2020-07-02  7:14 David Gow
2020-07-31 22:02 ` Brendan Higgins
2020-08-27 13:14 ` Marco Elver
2020-08-27 16:17   ` David Gow
2020-08-27 18:28     ` Marco Elver
2020-08-27 19:34       ` Brendan Higgins
2020-08-31 23:47     ` Kees Cook
2020-09-01  5:31       ` David Gow
2020-09-01 12:23         ` Marco Elver
2020-09-04  4:22           ` David Gow
2020-09-07  8:57             ` Marco Elver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200620054944.167330-1-davidgow@google.com \
    --to=davidgow@google.com \
    --cc=alan.maguire@oracle.com \
    --cc=brendanhiggins@google.com \
    --cc=corbet@lwn.net \
    --cc=keescook@chromium.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).