[PATCH] Documentation: kunit: Add naming guidelines

[PATCH] Documentation: kunit: Add naming guidelines

[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>
---
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

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Brendan Higgins]

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.
>
> [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?

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
>         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

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'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.

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 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
>

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Kees Cook]

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>

-- 
Kees Cook

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Marco Elver]

On Fri, Sep 04, 2020 at 12:22PM +0800, David Gow wrote:
[...]
> 
> This is a good point -- renaming files is definitely a pain. It's
> obviously my hope that KUnit sticks around long enough that it's not
> being added/removed as a dependency too often, particularly for the
> unit tests, so "_kunit" as a name doesn't worry me that much
> otherwise.

Make sense. And I do also hope that once a test is a KUnit test, there
is no need to change it. :-)

[...]
> I'm not personally convinced that "kunit" isn't something people could
> associate with tests, particularly as it becomes more popular, but if
> people really dislike it, we could have"_unittest.c" or similar.
> There's a balancing act between being generic (and not distinguishing
> between unit/integration/etc tests) and being consistent or avoiding
> renames. Take the case where there's a set of unit tests in a
> "-test.c" file, and an integration test is written as well: it
> probably should go in a speparate file, so now you'd either have a
> "-test.c" and a separate "-integration-test.c" (or the other way
> around if the integration test was written first), or the "-test.c"
> file would be renamed.

Makes sense, too. Yeah, if we have both we'd need to distinguish one way
or another. What might be particularly annoying is the case if an
integration test exists first, and it had been named "_kunit.c",
followed by addition of a unit test. I think the only sane thing at that
point would be to do a rename of the integration test; whereas if it had
been named "_test.c", I could live with there simply being a
"_unit_test.c" (or similar) file for the new unit test.

[...]
> > 1. Clear, intuitive, descriptive filenames ("[...] something that says
> > more strongly that this is a test [...]").
> >
> > 2. Avoid renames if any of the following changes: test framework, test
> > type or scope. I worry the most about this point, because it affects
> > our workflows. We need to avoid unnecessary patch conflicts, keep
> > cherry-picks simple, etc.
> >
> > 3. Strive for consistently named tests, regardless of type (because
> > it's hard to get right).
> >
> > 4. Want to distinguish KUnit tests from non-KUnit tests. (Also
> > consider that tooling can assist with this.)
> >
> 
> I think that these are somewhat in conflict with each other, which is
> what makes this complicated. Particularly, it's going to be difficult
> to both avoid renames if the test framework changes and to distinguish
> between KUnit and non-KUnit tests by filename.
> 
> I personally think that of these requirements, 2 is probably the one
> that would cause people the most real-world pain. I'm not sure how
> often test type or scope changes enough to be worth the rename, and I
> hope KUnit survives long enough and is useful enough that test
> framework changes are kept to a minimum, but this has already
> irritated enough people porting tests to KUnit to be a noticeable
> issue. One possibility is to focus on module names, which are probably
> more important and can be renamed without changing the filename,
> though that's pretty ugly.
> 
> I actually think "_kunit.c" probably is descriptive/intuitive enough
> to meet (1) -- or at least will be once KUnit is more widely used --
> but it does conflict a bit with 2.
> 
> It'd be nice to have consistently named tests, but we're not there at
> the moment, so fixing it will require a lot of renaming things. It's
> looking increasingly unlikely that we'll be able to do that for
> everything, so making this a recommendation for new test suites is
> probably the best we're likely to get.
> 
> > These are the 2 options under closer consideration:
> >
> > A. Original choice of "*-test.c": Satisfies 1,2,3. It seems to fail 4,
> > per Kees's original concern.
> >
> 
> Kees also brings up that using hyphens instead of underscores causes
> some inconsistency with module names, which is a bit of a pain.
> 
> > B. "*_kunit.c": Satisfies 4, maybe 3.
> >   - Fails 1, because !strstr("_kunit.c", "test") and the resulting
> > indirection. It hints at "unit test", but this may be a problem for
> > (2).
> >   - Fails 2, because if the test for some reason decides to stop using
> > KUnit (or a unit test morphs into an integration test), the file needs
> > to be renamed.
> >
> > And based on all this, why not:
> >
> > C. "*-ktest.c" (or "*_ktest.c"):
> >   - Satisfies 1, because it's descriptive and clearly says it's a
> > test; the 'k' can suggest it's an "[in-]kernel test" vs. some other
> > hybrid test that requires a userspace component.
> >   - Satisfies 2, because neither test framework or test type need to
> > be encoded in the filename.
> >   - Satisfies 3, because every test (that wants to use KUnit) can just
> > use this without thinking too much about it.
> >   - Satisfies 4, because "git grep -- '[-_]ktest\.[co]'" returns nothing.
> >
> 
> My concern with this is that we're introducing new jargon either way:
> does having "test" in the name outweigh the potential confusion from
> having "ktest" be in the filename only for "KUnit tests". So my
> feeling is that this would've been really useful if we'd named KUnit
> KTest (which, ironically, I think Brendan had considered) instead, but
> as-is is probably more confusing.

Make sense, too.

> At the risk of just chickening out at calling this "too hard", I'm
> leaning towards a variant of (A) here, and going for _test, but making
> it a weaker recommendation:
> - Specifying that the module name should end in _test, rather than the
> source filename. Module names are easier to change without causing
> merge conflicts (though they're a pain to change for the user).
> - Only applies to new test suites, and another suffix may be used if
> it conflicts with an existing non-kunit test (if it conflicts with a
> kunit test, they should be disambiguated in the suite name).
> - Test types (unit, integration, some subsystem-specific thing, etc)
> may be disambiguated in the suite name, at the discretion of the test
> author. (e.g., "driver_integration" as a suite name, with
> "driver_integration_test" as the module name, and either
> "driver_integration_test.c" or "integration_test.c" as recommended
> test filenames, depending on if "driver" is in its own directory.)
> 
> This should satisfy 1 & 2, and go some way towards satisfying 3. We
> can try to come up with some other technical solution to 4 if we need
> to.
> 
> Unless the objections are particularly earth-shattering, I'll do a new
> version of the patch that matches this next week. The other option is
> to drop the filename stuff from the document altogether, and sort it
> out in another patch, so we at least get some of the consistency in
> suite and Kconfig names.

Thanks for the detailed answer. Your plan sounds good to me. I'm fine
either way, as long as requirement (2) is somehow addressed, and we do
not end up with unnecessary renames.

Thanks,
-- Marco

Re: [PATCH] Documentation: kunit: Add naming guidelines

[David Gow]

On Tue, Sep 1, 2020 at 8:23 PM Marco Elver <elver@google.com> wrote:
>
> On Tue, 1 Sep 2020 at 07:31, David Gow <davidgow@google.com> wrote:
> > On Tue, Sep 1, 2020 at 7:47 AM Kees Cook <keescook@chromium.org> wrote:
> > > On Fri, Aug 28, 2020 at 12:17:05AM +0800, David Gow wrote:
> > > > On Thu, Aug 27, 2020 at 9:14 PM Marco Elver <elver@google.com> wrote:
> [...]
> >
> > I guess there are two audiences to cater for:
> > 1. Test authors, who may wish to have both unit-style and
> > integration-style tests, and want to distinguish them. They probably
> > have the best idea of where the line should be drawn for their
> > subsystems, and may have some existing style/nomenclature.
> > 2. People running "all tests", who want to broadly understand how the
> > whole suite of tests will behave (e.g., how long they'll take to run,
> > are they possibly nondeterministic, are there weird hardware/software
> > dependencies). This is where some more standardisation probably makes
> > sense.
> >
> > I'm not 100% the file/module name is the best place to make these
> > distinctions (given that it's the Kconfig entries that are at least
> > our current way of finding and running tests).
>
> I agree -- as you note, it's very hard to make this distinction. Since
> we're still discussing the best convention to use, one point I want to
> make is that encoding a dependency ("kunit") or type of test (unit,
> integration, etc.) in the name hurts scalability of our workflows.
> Because as soon as the dependency changes, or the type, any
> rename/move is very destructive to our workflow, because it
> immediately causes conflict with any in-flight patches. Whereas
> encoding this either in a comment, or via Kconfig would be less
> destructive.
>

This is a good point -- renaming files is definitely a pain. It's
obviously my hope that KUnit sticks around long enough that it's not
being added/removed as a dependency too often, particularly for the
unit tests, so "_kunit" as a name doesn't worry me that much
otherwise.

> > An off-the-wall idea
> > would be to have a flags field in the test suite structure to note
> > things like "large/long-running test" or "nondeterministic", and have
> > either a KUnit option to bypass them, note them in the output, or even
> > something terrifying like parsing it out of a compiled module.
>
> As a side-node, in the other very large codebase I have worked on, we
> have such markers ("size = ..."):
> https://docs.bazel.build/versions/master/be/common-definitions.html#common-attributes-tests
> However, there is also incentive to get this distinction right,
> because the test will be killed by the CI system if it exceeds the
> specified size (ran too long, OOM). Not sure we have this incentive
> yet.
>

KUnit does have test timeouts, but they're generous (30 seconds), and
not configurable per-test or per-suite. Fixing that's probably a
separate feature request which, as you point out, probably isn't worth
doing until we're actually seeing a problem. Maybe it's something
that'll become more apparent as KUnit is integrated into KernelCI and
the like, so we see more (and more varied) test runs/configs.

> [...]
> > > > I guess the interesting thing to note is that we've to date not really
> > > > made a distinction between KUnit the framework and the suite of all
> > > > KUnit tests. Maybe having a separate file/module naming scheme could
> > > > be a way of making that distinction, though it'd really only appear
> > > > when loading tests as modules -- there'd be no indication in e.g.,
> > > > suite names or test results. The more obvious solution to me (at
> > > > least, based on the current proposal) would be to have "integration"
> > > > or similar be part of the suite name (and hence the filename, so
> > > > _integration_kunit.c or similar), though even I admit that that's much
> > > > uglier. Maybe the idea of having the subsystem/suite distinction be
> > > > represented in the code could pave the way to having different suites
> > > > support different suffixes like that.
> > >
> > > Heh, yeah, let's not call them "_integration_kunit.c" ;) _behavior.c?
> > > _integration.c?
>
> If possible, I'd still prefer generic filenames, because it's easy to
> get wrong as we noted. Changes will cause conflicts.
>
> > I think we'd really like something that says more strongly that this
> > is a test (which is I suspect one of the reasons why _kunit.c has its
> > detractors: it doesn't have the word "test" in it).
>
> ^ Agreed.
>

I'm not personally convinced that "kunit" isn't something people could
associate with tests, particularly as it becomes more popular, but if
people really dislike it, we could have"_unittest.c" or similar.
There's a balancing act between being generic (and not distinguishing
between unit/integration/etc tests) and being consistent or avoiding
renames. Take the case where there's a set of unit tests in a
"-test.c" file, and an integration test is written as well: it
probably should go in a speparate file, so now you'd either have a
"-test.c" and a separate "-integration-test.c" (or the other way
around if the integration test was written first), or the "-test.c"
file would be renamed.

> > The other thing to
> > consider is that if there are multiple tests for the same thing (e.g.,
> > a unit test suite and an integration test suite), they'd still need
> > separate, non-conflicting suite names if we wanted them to be able to
> > be present in the same kernel.
> >
> > Maybe the right thing to do is to say that, for now, the _kunit.c
> > naming guideline only applies to "unit-style" tests.
> [...]
> >
> > So, putting together the various bits of feedback, how about something
> > like this:
> > Test filenames/modules should end in _kunit.c, unless they are either
> > a) not unit-style tests -- i.e, test something other than correctness
> > (e.g., performance), are non-deterministic, take a long time to run (>
> > ~1--2 seconds), or are testing across multiple subsystems -- OR
> > b) are ports of existing tests, which may keep their existing filename
> > (at least for now), so as not to break existing workflows.
> >
> > This is a bit weaker than the existing guidelines, and will probably
> > need tightening up once we have a better idea of what non-unit tests
> > should be and/or the existing, inconsistently named tests are
> > sufficiently outnumbered by the _kunit ones that people are used to it
> > and the perceived ugliness fades away. What (if any) tooling we need
> > around enumerating tests may end up influencing/being influenced by
> > this a bit, too.
> >
> > Thoughts?
>
> That could work, but it all still feels a little unsatisfying. Here's
> what I think the requirements for all this are:
>
> 1. Clear, intuitive, descriptive filenames ("[...] something that says
> more strongly that this is a test [...]").
>
> 2. Avoid renames if any of the following changes: test framework, test
> type or scope. I worry the most about this point, because it affects
> our workflows. We need to avoid unnecessary patch conflicts, keep
> cherry-picks simple, etc.
>
> 3. Strive for consistently named tests, regardless of type (because
> it's hard to get right).
>
> 4. Want to distinguish KUnit tests from non-KUnit tests. (Also
> consider that tooling can assist with this.)
>

I think that these are somewhat in conflict with each other, which is
what makes this complicated. Particularly, it's going to be difficult
to both avoid renames if the test framework changes and to distinguish
between KUnit and non-KUnit tests by filename.

I personally think that of these requirements, 2 is probably the one
that would cause people the most real-world pain. I'm not sure how
often test type or scope changes enough to be worth the rename, and I
hope KUnit survives long enough and is useful enough that test
framework changes are kept to a minimum, but this has already
irritated enough people porting tests to KUnit to be a noticeable
issue. One possibility is to focus on module names, which are probably
more important and can be renamed without changing the filename,
though that's pretty ugly.

I actually think "_kunit.c" probably is descriptive/intuitive enough
to meet (1) -- or at least will be once KUnit is more widely used --
but it does conflict a bit with 2.

It'd be nice to have consistently named tests, but we're not there at
the moment, so fixing it will require a lot of renaming things. It's
looking increasingly unlikely that we'll be able to do that for
everything, so making this a recommendation for new test suites is
probably the best we're likely to get.

> These are the 2 options under closer consideration:
>
> A. Original choice of "*-test.c": Satisfies 1,2,3. It seems to fail 4,
> per Kees's original concern.
>

Kees also brings up that using hyphens instead of underscores causes
some inconsistency with module names, which is a bit of a pain.

> B. "*_kunit.c": Satisfies 4, maybe 3.
>   - Fails 1, because !strstr("_kunit.c", "test") and the resulting
> indirection. It hints at "unit test", but this may be a problem for
> (2).
>   - Fails 2, because if the test for some reason decides to stop using
> KUnit (or a unit test morphs into an integration test), the file needs
> to be renamed.
>
> And based on all this, why not:
>
> C. "*-ktest.c" (or "*_ktest.c"):
>   - Satisfies 1, because it's descriptive and clearly says it's a
> test; the 'k' can suggest it's an "[in-]kernel test" vs. some other
> hybrid test that requires a userspace component.
>   - Satisfies 2, because neither test framework or test type need to
> be encoded in the filename.
>   - Satisfies 3, because every test (that wants to use KUnit) can just
> use this without thinking too much about it.
>   - Satisfies 4, because "git grep -- '[-_]ktest\.[co]'" returns nothing.
>

My concern with this is that we're introducing new jargon either way:
does having "test" in the name outweigh the potential confusion from
having "ktest" be in the filename only for "KUnit tests". So my
feeling is that this would've been really useful if we'd named KUnit
KTest (which, ironically, I think Brendan had considered) instead, but
as-is is probably more confusing.

> Thanks,
> -- Marco

At the risk of just chickening out at calling this "too hard", I'm
leaning towards a variant of (A) here, and going for _test, but making
it a weaker recommendation:
- Specifying that the module name should end in _test, rather than the
source filename. Module names are easier to change without causing
merge conflicts (though they're a pain to change for the user).
- Only applies to new test suites, and another suffix may be used if
it conflicts with an existing non-kunit test (if it conflicts with a
kunit test, they should be disambiguated in the suite name).
- Test types (unit, integration, some subsystem-specific thing, etc)
may be disambiguated in the suite name, at the discretion of the test
author. (e.g., "driver_integration" as a suite name, with
"driver_integration_test" as the module name, and either
"driver_integration_test.c" or "integration_test.c" as recommended
test filenames, depending on if "driver" is in its own directory.)

This should satisfy 1 & 2, and go some way towards satisfying 3. We
can try to come up with some other technical solution to 4 if we need
to.

Unless the objections are particularly earth-shattering, I'll do a new
version of the patch that matches this next week. The other option is
to drop the filename stuff from the document altogether, and sort it
out in another patch, so we at least get some of the consistency in
suite and Kconfig names.

Cheers,
-- David

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Marco Elver]

On Tue, 1 Sep 2020 at 07:31, David Gow <davidgow@google.com> wrote:
> On Tue, Sep 1, 2020 at 7:47 AM Kees Cook <keescook@chromium.org> wrote:
> > On Fri, Aug 28, 2020 at 12:17:05AM +0800, David Gow wrote:
> > > On Thu, Aug 27, 2020 at 9:14 PM Marco Elver <elver@google.com> wrote:
[...]
>
> I guess there are two audiences to cater for:
> 1. Test authors, who may wish to have both unit-style and
> integration-style tests, and want to distinguish them. They probably
> have the best idea of where the line should be drawn for their
> subsystems, and may have some existing style/nomenclature.
> 2. People running "all tests", who want to broadly understand how the
> whole suite of tests will behave (e.g., how long they'll take to run,
> are they possibly nondeterministic, are there weird hardware/software
> dependencies). This is where some more standardisation probably makes
> sense.
>
> I'm not 100% the file/module name is the best place to make these
> distinctions (given that it's the Kconfig entries that are at least
> our current way of finding and running tests).

I agree -- as you note, it's very hard to make this distinction. Since
we're still discussing the best convention to use, one point I want to
make is that encoding a dependency ("kunit") or type of test (unit,
integration, etc.) in the name hurts scalability of our workflows.
Because as soon as the dependency changes, or the type, any
rename/move is very destructive to our workflow, because it
immediately causes conflict with any in-flight patches. Whereas
encoding this either in a comment, or via Kconfig would be less
destructive.

> An off-the-wall idea
> would be to have a flags field in the test suite structure to note
> things like "large/long-running test" or "nondeterministic", and have
> either a KUnit option to bypass them, note them in the output, or even
> something terrifying like parsing it out of a compiled module.

As a side-node, in the other very large codebase I have worked on, we
have such markers ("size = ..."):
https://docs.bazel.build/versions/master/be/common-definitions.html#common-attributes-tests
However, there is also incentive to get this distinction right,
because the test will be killed by the CI system if it exceeds the
specified size (ran too long, OOM). Not sure we have this incentive
yet.

[...]
> > > I guess the interesting thing to note is that we've to date not really
> > > made a distinction between KUnit the framework and the suite of all
> > > KUnit tests. Maybe having a separate file/module naming scheme could
> > > be a way of making that distinction, though it'd really only appear
> > > when loading tests as modules -- there'd be no indication in e.g.,
> > > suite names or test results. The more obvious solution to me (at
> > > least, based on the current proposal) would be to have "integration"
> > > or similar be part of the suite name (and hence the filename, so
> > > _integration_kunit.c or similar), though even I admit that that's much
> > > uglier. Maybe the idea of having the subsystem/suite distinction be
> > > represented in the code could pave the way to having different suites
> > > support different suffixes like that.
> >
> > Heh, yeah, let's not call them "_integration_kunit.c" ;) _behavior.c?
> > _integration.c?

If possible, I'd still prefer generic filenames, because it's easy to
get wrong as we noted. Changes will cause conflicts.

> I think we'd really like something that says more strongly that this
> is a test (which is I suspect one of the reasons why _kunit.c has its
> detractors: it doesn't have the word "test" in it).

^ Agreed.

> The other thing to
> consider is that if there are multiple tests for the same thing (e.g.,
> a unit test suite and an integration test suite), they'd still need
> separate, non-conflicting suite names if we wanted them to be able to
> be present in the same kernel.
>
> Maybe the right thing to do is to say that, for now, the _kunit.c
> naming guideline only applies to "unit-style" tests.
[...]
>
> So, putting together the various bits of feedback, how about something
> like this:
> Test filenames/modules should end in _kunit.c, unless they are either
> a) not unit-style tests -- i.e, test something other than correctness
> (e.g., performance), are non-deterministic, take a long time to run (>
> ~1--2 seconds), or are testing across multiple subsystems -- OR
> b) are ports of existing tests, which may keep their existing filename
> (at least for now), so as not to break existing workflows.
>
> This is a bit weaker than the existing guidelines, and will probably
> need tightening up once we have a better idea of what non-unit tests
> should be and/or the existing, inconsistently named tests are
> sufficiently outnumbered by the _kunit ones that people are used to it
> and the perceived ugliness fades away. What (if any) tooling we need
> around enumerating tests may end up influencing/being influenced by
> this a bit, too.
>
> Thoughts?

That could work, but it all still feels a little unsatisfying. Here's
what I think the requirements for all this are:

1. Clear, intuitive, descriptive filenames ("[...] something that says
more strongly that this is a test [...]").

2. Avoid renames if any of the following changes: test framework, test
type or scope. I worry the most about this point, because it affects
our workflows. We need to avoid unnecessary patch conflicts, keep
cherry-picks simple, etc.

3. Strive for consistently named tests, regardless of type (because
it's hard to get right).

4. Want to distinguish KUnit tests from non-KUnit tests. (Also
consider that tooling can assist with this.)

These are the 2 options under closer consideration:

A. Original choice of "*-test.c": Satisfies 1,2,3. It seems to fail 4,
per Kees's original concern.

B. "*_kunit.c": Satisfies 4, maybe 3.
  - Fails 1, because !strstr("_kunit.c", "test") and the resulting
indirection. It hints at "unit test", but this may be a problem for
(2).
  - Fails 2, because if the test for some reason decides to stop using
KUnit (or a unit test morphs into an integration test), the file needs
to be renamed.

And based on all this, why not:

C. "*-ktest.c" (or "*_ktest.c"):
  - Satisfies 1, because it's descriptive and clearly says it's a
test; the 'k' can suggest it's an "[in-]kernel test" vs. some other
hybrid test that requires a userspace component.
  - Satisfies 2, because neither test framework or test type need to
be encoded in the filename.
  - Satisfies 3, because every test (that wants to use KUnit) can just
use this without thinking too much about it.
  - Satisfies 4, because "git grep -- '[-_]ktest\.[co]'" returns nothing.

Thanks,
-- Marco

Re: [PATCH] Documentation: kunit: Add naming guidelines

[David Gow]

On Tue, Sep 1, 2020 at 7:47 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Aug 28, 2020 at 12:17:05AM +0800, David Gow wrote:
> > On Thu, Aug 27, 2020 at 9:14 PM Marco Elver <elver@google.com> wrote:
> > > Just an idea: Maybe the names are also an opportunity to distinguish
> > > real _unit_ style tests and then the rarer integration-style tests. I
> > > personally prefer using the more generic *-test.c, at least for the
> > > integration-style tests I've been working on (KUnit is still incredibly
> > > valuable for integration-style tests, because otherwise I'd have to roll
> > > my own poor-man's version of KUnit, so thank you!). Using *_kunit.c for
> > > such tests is unintuitive, because the word "unit" hints at "unit tests"
> > > -- and having descriptive (and not misleading) filenames is still
> > > important. So I hope you won't mind if *-test.c are still used where
> > > appropriate.
>
> This is a good point, and I guess not one that has really been examined.
> I'm not sure what to think of some of the lib/ tests. test_user_copy
> seems to be a "unit" test -- it's validating the function family vs
> all kinds of arguments and conditions. But test_overflow is less unit
> and more integration, which includes "do all of these allocators end up
> acting the same way?" etc
>
> I'm not really sure what to make of that -- I don't really have a formal
> testing background.
>

I'm not sure we need a super-precise definition here (maybe just
because I don't have one), and really just need to work out what
distinctions are actually useful. For example, I'm not sure there's
any real advantage to treating test_user_copy and test_overflow
differently. The KCSAN tests which spawn lots of threads and are both
slower and less deterministic seem more obviously different, though.

I guess there are two audiences to cater for:
1. Test authors, who may wish to have both unit-style and
integration-style tests, and want to distinguish them. They probably
have the best idea of where the line should be drawn for their
subsystems, and may have some existing style/nomenclature.
2. People running "all tests", who want to broadly understand how the
whole suite of tests will behave (e.g., how long they'll take to run,
are they possibly nondeterministic, are there weird hardware/software
dependencies). This is where some more standardisation probably makes
sense.

I'm not 100% the file/module name is the best place to make these
distinctions (given that it's the Kconfig entries that are at least
our current way of finding and running tests). An off-the-wall idea
would be to have a flags field in the test suite structure to note
things like "large/long-running test" or "nondeterministic", and have
either a KUnit option to bypass them, note them in the output, or even
something terrifying like parsing it out of a compiled module.

> > As Brendan alluded to in the talk, the popularity of KUnit for these
> > integration-style tests came as something of a surprise (more due to
> > our lack of imagination than anything else, I suspect). It's great
> > that it's working, though: I don't think anyone wants the world filled
> > with more single-use test "frameworks" than is necessary!
> >
> > I guess the interesting thing to note is that we've to date not really
> > made a distinction between KUnit the framework and the suite of all
> > KUnit tests. Maybe having a separate file/module naming scheme could
> > be a way of making that distinction, though it'd really only appear
> > when loading tests as modules -- there'd be no indication in e.g.,
> > suite names or test results. The more obvious solution to me (at
> > least, based on the current proposal) would be to have "integration"
> > or similar be part of the suite name (and hence the filename, so
> > _integration_kunit.c or similar), though even I admit that that's much
> > uglier. Maybe the idea of having the subsystem/suite distinction be
> > represented in the code could pave the way to having different suites
> > support different suffixes like that.
>
> Heh, yeah, let's not call them "_integration_kunit.c" ;) _behavior.c?
> _integration.c?
>

I think we'd really like something that says more strongly that this
is a test (which is I suspect one of the reasons why _kunit.c has its
detractors: it doesn't have the word "test" in it). The other thing to
consider is that if there are multiple tests for the same thing (e.g.,
a unit test suite and an integration test suite), they'd still need
separate, non-conflicting suite names if we wanted them to be able to
be present in the same kernel.

Maybe the right thing to do is to say that, for now, the _kunit.c
naming guideline only applies to "unit-style" tests.

> > Do you know of any cases where something has/would have both
> > unit-style tests and integration-style tests built with KUnit where
> > the distinction needs to be clear?
>
> This is probably the right question. :)
>
I had a quick look, and we don't appear to have anything quite like
this yet, at least with KUnit.


So, putting together the various bits of feedback, how about something
like this:
Test filenames/modules should end in _kunit.c, unless they are either
a) not unit-style tests -- i.e, test something other than correctness
(e.g., performance), are non-deterministic, take a long time to run (>
~1--2 seconds), or are testing across multiple subsystems -- OR
b) are ports of existing tests, which may keep their existing filename
(at least for now), so as not to break existing workflows.

This is a bit weaker than the existing guidelines, and will probably
need tightening up once we have a better idea of what non-unit tests
should be and/or the existing, inconsistently named tests are
sufficiently outnumbered by the _kunit ones that people are used to it
and the perceived ugliness fades away. What (if any) tooling we need
around enumerating tests may end up influencing/being influenced by
this a bit, too.

Thoughts?
-- David

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Kees Cook]

On Fri, Aug 28, 2020 at 12:17:05AM +0800, David Gow wrote:
> On Thu, Aug 27, 2020 at 9:14 PM Marco Elver <elver@google.com> wrote:
> > Just an idea: Maybe the names are also an opportunity to distinguish
> > real _unit_ style tests and then the rarer integration-style tests. I
> > personally prefer using the more generic *-test.c, at least for the
> > integration-style tests I've been working on (KUnit is still incredibly
> > valuable for integration-style tests, because otherwise I'd have to roll
> > my own poor-man's version of KUnit, so thank you!). Using *_kunit.c for
> > such tests is unintuitive, because the word "unit" hints at "unit tests"
> > -- and having descriptive (and not misleading) filenames is still
> > important. So I hope you won't mind if *-test.c are still used where
> > appropriate.

This is a good point, and I guess not one that has really been examined.
I'm not sure what to think of some of the lib/ tests. test_user_copy
seems to be a "unit" test -- it's validating the function family vs
all kinds of arguments and conditions. But test_overflow is less unit
and more integration, which includes "do all of these allocators end up
acting the same way?" etc

I'm not really sure what to make of that -- I don't really have a formal
testing background.

> As Brendan alluded to in the talk, the popularity of KUnit for these
> integration-style tests came as something of a surprise (more due to
> our lack of imagination than anything else, I suspect). It's great
> that it's working, though: I don't think anyone wants the world filled
> with more single-use test "frameworks" than is necessary!
> 
> I guess the interesting thing to note is that we've to date not really
> made a distinction between KUnit the framework and the suite of all
> KUnit tests. Maybe having a separate file/module naming scheme could
> be a way of making that distinction, though it'd really only appear
> when loading tests as modules -- there'd be no indication in e.g.,
> suite names or test results. The more obvious solution to me (at
> least, based on the current proposal) would be to have "integration"
> or similar be part of the suite name (and hence the filename, so
> _integration_kunit.c or similar), though even I admit that that's much
> uglier. Maybe the idea of having the subsystem/suite distinction be
> represented in the code could pave the way to having different suites
> support different suffixes like that.

Heh, yeah, let's not call them "_integration_kunit.c" ;) _behavior.c?
_integration.c?

> Do you know of any cases where something has/would have both
> unit-style tests and integration-style tests built with KUnit where
> the distinction needs to be clear?

This is probably the right question. :)

-- 
Kees Cook

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Brendan Higgins]

On Thu, Aug 27, 2020 at 11:28 AM Marco Elver <elver@google.com> wrote:
>
> On Thu, 27 Aug 2020 at 18:17, David Gow <davidgow@google.com> wrote:
> [...]
> > > First of all, thanks for the talk yesterday! I only looked at this
> > > because somebody pasted the LKML link. :-)
> >
> > No worries! Clearly this document needed linking -- even I was
> > starting to suspect the reason no-one was complaining about this was
> > that no-one had read it. :-)
> [...]
> > >
> > > While I guess this ship has sailed, and *_kunit.c is the naming
> > > convention now, I hope this is still just a recommendation and names of
> > > the form *-test.c are not banned!
> >
> > The ship hasn't technically sailed until this patch is actually
> > accepted. Thus far, we hadn't had any real complaints about the
> > _kunit.c idea, though that may have been due to this email not
> > reaching enough people. If you haven't read the discussion in
> > https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
> > it's worthwhile: the _kunit.c name is discussed, and Kees lays out
> > some more detailed rationale for it as well.
>
> Thanks, I can see the rationale. AFAIK the main concern was "it does
> not distinguish it from other tests".

That was my understanding as well.

> > > $> git grep 'KUNIT.*-test.o'
> > >         drivers/base/power/Makefile:obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o
> > >         drivers/base/test/Makefile:obj-$(CONFIG_KUNIT_DRIVER_PE_TEST) += property-entry-test.o
> > >         fs/ext4/Makefile:obj-$(CONFIG_EXT4_KUNIT_TESTS)         += ext4-inode-test.o
> > >         kernel/Makefile:obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
> > >         lib/Makefile:obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> > >         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=          kunit-test.o
> > >         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=          string-stream-test.o
> > >         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=  kunit-example-test.o
> > >
> > > $> git grep 'KUNIT.*_kunit.o'
> > > # Returns nothing
> > >
> >
> > This was definitely something we noted. Part of the goal of having
> > _kunit.c as a filename suffix (and, perhaps more importantly, the
> > _kunit module name suffix) was to have a way of both distinguishing
> > KUnit tests from non-KUnit ones (of which there are quite a few
> > already with -test names), and to have a way of quickly determining
> > what modules contain KUnit tests. That really only works if everyone
> > is using it, so the plan was to push this as much as possible. This'd
> > probably include renaming most of the existing test files to match,
> > which is a bit of a pain (particularly when converting non-KUnit tests
> > to KUnit and similar), but is a one-time thing.
>
> Feel free to ignore the below, but here might be one concern:
>
> I think the problem of distinguishing KUnit tests from non-KUnit tests
> is a technical problem (in fact, the Kconfig entries have all the info
> we need), but a solution that sacrifices readability might cause
> unnecessary friction.
>
> The main issue I foresee is that the _kunit.c name is opaque as to
> what the file does, but merely indicates one of its dependencies. Most
> of us clearly know that KUnit is a test framework, but it's a level of
> indirection nevertheless. (But _kunit_test.c is also bad, because it's
> unnecessarily long.) For a dozen tests, that's probably still fine.
> But now imagine 100s of tests, people will quickly wonder "what's this
> _kunit thing?". And if KUnit tests are eventually the dominant tests,
> does it still matter?
>
> I worry that because of the difficulty of enforcing the name, choosing
> something unintuitive will also achieve the opposite result:
> proliferation of even more diverse names. A generic convention like
> "*-test.c" will be close enough to what's intuitive for most people,
> and we might actually have a chance of getting everyone to stick to
> it.
>
> The problem of identifying all KUnit tests can be solved with a script.

Fair point. However, converting all non-kselftests to kselftest or
KUnit will likely take time, and kselftest will likely be with us
forever. In short, other tests are likely to co-exist with KUnit for a
long time if not in perpetuity; thus, I think Kees' point stands in
this case.

> > > Just an idea: Maybe the names are also an opportunity to distinguish
> > > real _unit_ style tests and then the rarer integration-style tests. I
> > > personally prefer using the more generic *-test.c, at least for the
> > > integration-style tests I've been working on (KUnit is still incredibly
> > > valuable for integration-style tests, because otherwise I'd have to roll
> > > my own poor-man's version of KUnit, so thank you!). Using *_kunit.c for
> > > such tests is unintuitive, because the word "unit" hints at "unit tests"
> > > -- and having descriptive (and not misleading) filenames is still
> > > important. So I hope you won't mind if *-test.c are still used where
> > > appropriate.
> >
> > As Brendan alluded to in the talk, the popularity of KUnit for these
> > integration-style tests came as something of a surprise (more due to
> > our lack of imagination than anything else, I suspect). It's great
> > that it's working, though: I don't think anyone wants the world filled
> > with more single-use test "frameworks" than is necessary!
> >
> > I guess the interesting thing to note is that we've to date not really
> > made a distinction between KUnit the framework and the suite of all
> > KUnit tests. Maybe having a separate file/module naming scheme could
> > be a way of making that distinction, though it'd really only appear
> > when loading tests as modules -- there'd be no indication in e.g.,
> > suite names or test results. The more obvious solution to me (at
> > least, based on the current proposal) would be to have "integration"
> > or similar be part of the suite name (and hence the filename, so
> > _integration_kunit.c or similar), though even I admit that that's much
> > uglier.

Very good point.

> Yeah, that's not great either.  Again, in the end it's probably
> entirely up to you, but it'd be good if the filenames are descriptive
> and readable (vs. a puzzle).

Yeah, I have no good answers either.

Dumb idea: name integration tests *_kint.c. Probably worse and more
confusing than either just *_kunit.c or *_integration_{test|kunit}.c.

As misleading as it is to have the label "unit" on an integration
test, I tend to think that asking devs to label their tests properly
as either unit tests or integration tests is unrealistic.

Then again, does it really matter for anyone other than the maintainer
and devs who contribute to the test?

> > Maybe the idea of having the subsystem/suite distinction be
> > represented in the code could pave the way to having different suites
> > support different suffixes like that.

I don't think I follow.

> > Do you know of any cases where something has/would have both
> > unit-style tests and integration-style tests built with KUnit where
> > the distinction needs to be clear?
>
> None I know of, so probably not a big deal.
>
> > Brendan, Kees: do you have any thoughts?

Marco and Kees both make good points. I don't think there is a right
or wrong decision here; I think we just need to pick.

Personally, I like *-test.c, but that's probably just because that was
my original intention: I am not really sure that the costs/benefits
really make it superior to *-kunit.c.

David, it's your patch, so it's up to you; however, if you want to
pick *-test.c, and blame any consequences on me, feel free.

Cheers

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Marco Elver]

On Thu, 27 Aug 2020 at 18:17, David Gow <davidgow@google.com> wrote:
[...]
> > First of all, thanks for the talk yesterday! I only looked at this
> > because somebody pasted the LKML link. :-)
>
> No worries! Clearly this document needed linking -- even I was
> starting to suspect the reason no-one was complaining about this was
> that no-one had read it. :-)
[...]
> >
> > While I guess this ship has sailed, and *_kunit.c is the naming
> > convention now, I hope this is still just a recommendation and names of
> > the form *-test.c are not banned!
>
> The ship hasn't technically sailed until this patch is actually
> accepted. Thus far, we hadn't had any real complaints about the
> _kunit.c idea, though that may have been due to this email not
> reaching enough people. If you haven't read the discussion in
> https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
> it's worthwhile: the _kunit.c name is discussed, and Kees lays out
> some more detailed rationale for it as well.

Thanks, I can see the rationale. AFAIK the main concern was "it does
not distinguish it from other tests".

> > $> git grep 'KUNIT.*-test.o'
> >         drivers/base/power/Makefile:obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o
> >         drivers/base/test/Makefile:obj-$(CONFIG_KUNIT_DRIVER_PE_TEST) += property-entry-test.o
> >         fs/ext4/Makefile:obj-$(CONFIG_EXT4_KUNIT_TESTS)         += ext4-inode-test.o
> >         kernel/Makefile:obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
> >         lib/Makefile:obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
> >         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=          kunit-test.o
> >         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=          string-stream-test.o
> >         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=  kunit-example-test.o
> >
> > $> git grep 'KUNIT.*_kunit.o'
> > # Returns nothing
> >
>
> This was definitely something we noted. Part of the goal of having
> _kunit.c as a filename suffix (and, perhaps more importantly, the
> _kunit module name suffix) was to have a way of both distinguishing
> KUnit tests from non-KUnit ones (of which there are quite a few
> already with -test names), and to have a way of quickly determining
> what modules contain KUnit tests. That really only works if everyone
> is using it, so the plan was to push this as much as possible. This'd
> probably include renaming most of the existing test files to match,
> which is a bit of a pain (particularly when converting non-KUnit tests
> to KUnit and similar), but is a one-time thing.

Feel free to ignore the below, but here might be one concern:

I think the problem of distinguishing KUnit tests from non-KUnit tests
is a technical problem (in fact, the Kconfig entries have all the info
we need), but a solution that sacrifices readability might cause
unnecessary friction.

The main issue I foresee is that the _kunit.c name is opaque as to
what the file does, but merely indicates one of its dependencies. Most
of us clearly know that KUnit is a test framework, but it's a level of
indirection nevertheless. (But _kunit_test.c is also bad, because it's
unnecessarily long.) For a dozen tests, that's probably still fine.
But now imagine 100s of tests, people will quickly wonder "what's this
_kunit thing?". And if KUnit tests are eventually the dominant tests,
does it still matter?

I worry that because of the difficulty of enforcing the name, choosing
something unintuitive will also achieve the opposite result:
proliferation of even more diverse names. A generic convention like
"*-test.c" will be close enough to what's intuitive for most people,
and we might actually have a chance of getting everyone to stick to
it.

The problem of identifying all KUnit tests can be solved with a script.

> > Just an idea: Maybe the names are also an opportunity to distinguish
> > real _unit_ style tests and then the rarer integration-style tests. I
> > personally prefer using the more generic *-test.c, at least for the
> > integration-style tests I've been working on (KUnit is still incredibly
> > valuable for integration-style tests, because otherwise I'd have to roll
> > my own poor-man's version of KUnit, so thank you!). Using *_kunit.c for
> > such tests is unintuitive, because the word "unit" hints at "unit tests"
> > -- and having descriptive (and not misleading) filenames is still
> > important. So I hope you won't mind if *-test.c are still used where
> > appropriate.
>
> As Brendan alluded to in the talk, the popularity of KUnit for these
> integration-style tests came as something of a surprise (more due to
> our lack of imagination than anything else, I suspect). It's great
> that it's working, though: I don't think anyone wants the world filled
> with more single-use test "frameworks" than is necessary!
>
> I guess the interesting thing to note is that we've to date not really
> made a distinction between KUnit the framework and the suite of all
> KUnit tests. Maybe having a separate file/module naming scheme could
> be a way of making that distinction, though it'd really only appear
> when loading tests as modules -- there'd be no indication in e.g.,
> suite names or test results. The more obvious solution to me (at
> least, based on the current proposal) would be to have "integration"
> or similar be part of the suite name (and hence the filename, so
> _integration_kunit.c or similar), though even I admit that that's much
> uglier.

Yeah, that's not great either.  Again, in the end it's probably
entirely up to you, but it'd be good if the filenames are descriptive
and readable (vs. a puzzle).

> Maybe the idea of having the subsystem/suite distinction be
> represented in the code could pave the way to having different suites
> support different suffixes like that.

> Do you know of any cases where something has/would have both
> unit-style tests and integration-style tests built with KUnit where
> the distinction needs to be clear?

None I know of, so probably not a big deal.

> Brendan, Kees: do you have any thoughts?
>

> Cheers,
> -- David

Thanks,
-- Marco

Re: [PATCH] Documentation: kunit: Add naming guidelines

[David Gow]

On Thu, Aug 27, 2020 at 9:14 PM Marco Elver <elver@google.com> wrote:
>
> On Thu, Jul 02, 2020 at 12:14AM -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>
> > ---
> ...
> > +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.
>
> First of all, thanks for the talk yesterday! I only looked at this
> because somebody pasted the LKML link. :-)

No worries! Clearly this document needed linking -- even I was
starting to suspect the reason no-one was complaining about this was
that no-one had read it. :-)

> The example about excessive namespacing seems confusing. Was it supposed
> to be
>
>         [...] firmware_kunit.c`` instead of ``<drivername>_firmware_kunit.c [...]
>
> ?

Whoops, yes. I'll fix that.

>
> While I guess this ship has sailed, and *_kunit.c is the naming
> convention now, I hope this is still just a recommendation and names of
> the form *-test.c are not banned!

The ship hasn't technically sailed until this patch is actually
accepted. Thus far, we hadn't had any real complaints about the
_kunit.c idea, though that may have been due to this email not
reaching enough people. If you haven't read the discussion in
https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
it's worthwhile: the _kunit.c name is discussed, and Kees lays out
some more detailed rationale for it as well.

> $> git grep 'KUNIT.*-test.o'
>         drivers/base/power/Makefile:obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o
>         drivers/base/test/Makefile:obj-$(CONFIG_KUNIT_DRIVER_PE_TEST) += property-entry-test.o
>         fs/ext4/Makefile:obj-$(CONFIG_EXT4_KUNIT_TESTS)         += ext4-inode-test.o
>         kernel/Makefile:obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
>         lib/Makefile:obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
>         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=          kunit-test.o
>         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=          string-stream-test.o
>         lib/kunit/Makefile:obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=  kunit-example-test.o
>
> $> git grep 'KUNIT.*_kunit.o'
> # Returns nothing
>

This was definitely something we noted. Part of the goal of having
_kunit.c as a filename suffix (and, perhaps more importantly, the
_kunit module name suffix) was to have a way of both distinguishing
KUnit tests from non-KUnit ones (of which there are quite a few
already with -test names), and to have a way of quickly determining
what modules contain KUnit tests. That really only works if everyone
is using it, so the plan was to push this as much as possible. This'd
probably include renaming most of the existing test files to match,
which is a bit of a pain (particularly when converting non-KUnit tests
to KUnit and similar), but is a one-time thing.

>
> Just an idea: Maybe the names are also an opportunity to distinguish
> real _unit_ style tests and then the rarer integration-style tests. I
> personally prefer using the more generic *-test.c, at least for the
> integration-style tests I've been working on (KUnit is still incredibly
> valuable for integration-style tests, because otherwise I'd have to roll
> my own poor-man's version of KUnit, so thank you!). Using *_kunit.c for
> such tests is unintuitive, because the word "unit" hints at "unit tests"
> -- and having descriptive (and not misleading) filenames is still
> important. So I hope you won't mind if *-test.c are still used where
> appropriate.

As Brendan alluded to in the talk, the popularity of KUnit for these
integration-style tests came as something of a surprise (more due to
our lack of imagination than anything else, I suspect). It's great
that it's working, though: I don't think anyone wants the world filled
with more single-use test "frameworks" than is necessary!

I guess the interesting thing to note is that we've to date not really
made a distinction between KUnit the framework and the suite of all
KUnit tests. Maybe having a separate file/module naming scheme could
be a way of making that distinction, though it'd really only appear
when loading tests as modules -- there'd be no indication in e.g.,
suite names or test results. The more obvious solution to me (at
least, based on the current proposal) would be to have "integration"
or similar be part of the suite name (and hence the filename, so
_integration_kunit.c or similar), though even I admit that that's much
uglier. Maybe the idea of having the subsystem/suite distinction be
represented in the code could pave the way to having different suites
support different suffixes like that.

Do you know of any cases where something has/would have both
unit-style tests and integration-style tests built with KUnit where
the distinction needs to be clear?

Brendan, Kees: do you have any thoughts?


> Thanks,
> -- Marco


Cheers,
-- David

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Marco Elver]

On Thu, Jul 02, 2020 at 12:14AM -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>
> ---
...
> +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.

First of all, thanks for the talk yesterday! I only looked at this
because somebody pasted the LKML link. :-)

The example about excessive namespacing seems confusing. Was it supposed
to be 

	[...] firmware_kunit.c`` instead of ``<drivername>_firmware_kunit.c [...]

?


While I guess this ship has sailed, and *_kunit.c is the naming
convention now, I hope this is still just a recommendation and names of
the form *-test.c are not banned!

$> git grep 'KUNIT.*-test.o'
	drivers/base/power/Makefile:obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o
	drivers/base/test/Makefile:obj-$(CONFIG_KUNIT_DRIVER_PE_TEST) += property-entry-test.o
	fs/ext4/Makefile:obj-$(CONFIG_EXT4_KUNIT_TESTS)		+= ext4-inode-test.o
	kernel/Makefile:obj-$(CONFIG_SYSCTL_KUNIT_TEST) += sysctl-test.o
	lib/Makefile:obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
	lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=		kunit-test.o
	lib/kunit/Makefile:obj-$(CONFIG_KUNIT_TEST) +=		string-stream-test.o
	lib/kunit/Makefile:obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=	kunit-example-test.o

$> git grep 'KUNIT.*_kunit.o'
# Returns nothing


Just an idea: Maybe the names are also an opportunity to distinguish
real _unit_ style tests and then the rarer integration-style tests. I
personally prefer using the more generic *-test.c, at least for the
integration-style tests I've been working on (KUnit is still incredibly
valuable for integration-style tests, because otherwise I'd have to roll
my own poor-man's version of KUnit, so thank you!). Using *_kunit.c for
such tests is unintuitive, because the word "unit" hints at "unit tests"
-- and having descriptive (and not misleading) filenames is still
important. So I hope you won't mind if *-test.c are still used where
appropriate.

Thanks,
-- Marco

Re: [PATCH] Documentation: kunit: Add naming guidelines

[Brendan Higgins]

On Thu, Jul 2, 2020 at 12:14 AM 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.
>
> [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>

Sorry for taking so long on this; nevertheless, looks great!

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

[PATCH] Documentation: kunit: Add naming guidelines

[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>
---
This is a follow-up v1 to the RFC patch here:
https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@google.com/T/#u

There weren't any fundamental objections to the naming guidelines
themselves, so nothing's changed on that front.

Otherwise, changes since the RFC:
- 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.

Cheers,
-- David

 Documentation/dev-tools/kunit/index.rst |   1 +
 Documentation/dev-tools/kunit/style.rst | 181 ++++++++++++++++++++++++
 2 files changed, 182 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..8cad2627924c
--- /dev/null
+++ b/Documentation/dev-tools/kunit/style.rst
@@ -0,0 +1,181 @@
+.. 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``
+  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.
+
+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_unit_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 ``unit_test``.
+
+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 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.212.ge8ba1cc988-goog