linux-kernel - Re: [PATCH v2] Documentation: kunit: Add naming guidelines

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 9 Sep 2020 10:36:53 +0200
From:   Marco Elver <elver@...gle.com>
To:     David Gow <davidgow@...gle.com>
Cc:     Brendan Higgins <brendanhiggins@...gle.com>,
        Jonathan Corbet <corbet@....net>,
        Kees Cook <keescook@...omium.org>,
        Alan Maguire <alan.maguire@...cle.com>,
        Randy Dunlap <rd.dunlab@...il.com>,
        Theodore Ts'o <tytso@....edu>, Tim Bird <Tim.Bird@...y.com>,
        kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] Documentation: kunit: Add naming guidelines

On Tue, Sep 08, 2020 at 10:16PM -0700, David Gow wrote:
> As discussed in [1], KUnit tests have hitherto not had a particularly
> consistent naming scheme. This adds documentation outlining how tests
> and test suites should be named, including how those names should be
> used in Kconfig entries and filenames.
> 
> [1]:
> https://lore.kernel.org/linux-kselftest/202006141005.BA19A9D3@keescook/t/#u
> 
> Signed-off-by: David Gow <davidgow@...gle.com>
> Reviewed-by: Kees Cook <keescook@...omium.org>
> Reviewed-by: Brendan Higgins <brendanhiggins@...gle.com>
> ---
> 
> This is v2 of the KUnit test nomenclature guidelines. The guidelines have
> changed a bit in response to the discussion on the v1 thread which came
> about after plumbers. The major change is that the filename suffix is
> now "_test", with "_kunit" permitted where it conflicts. There are also
> some other exceptions carved out around existing tests, and very
> non-unit-like tests.
> 
> Changelog:
> 
> v2:
> - Rewrote the filename section to use "_test" as a suffix, and focus on
>   module names, not filenames.
> - Add a motivating introduction, which also calls out existing tests and
>   tests which cause problems when run automatically (long running,
>   flaky tests) as reasons to avoid the guidelines.
> - Talk about including the type of test in the suite name, but only if
>   theres an actual confict. (And update the example for this).
> 
> v1:
> https://lore.kernel.org/linux-kselftest/20200702071416.1780522-1-davidgow@google.com/
> - Fixed a bit of space/tab confusion in the index (Thanks, Randy)
> - Added some more examples (and some test case examples).
> - Added some examples of what not to call subsystems and suites.
> - No longer explicitly require "If unsure, put N" in Kconfig entries.
> - Minor formatting changes
> 
> RFC:
> https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-davidgow@google.com/T/#u
> - Initial version
> 
> 
> The result is a little bit weaker than the previous versions, but
> hopefully will let us get the areas we agree on down.
> 
> -- David
> 
> 
>  Documentation/dev-tools/kunit/index.rst |   1 +
>  Documentation/dev-tools/kunit/style.rst | 207 ++++++++++++++++++++++++
>  2 files changed, 208 insertions(+)
>  create mode 100644 Documentation/dev-tools/kunit/style.rst
> 
> diff --git a/Documentation/dev-tools/kunit/index.rst b/Documentation/dev-tools/kunit/index.rst
> index e93606ecfb01..c234a3ab3c34 100644
> --- a/Documentation/dev-tools/kunit/index.rst
> +++ b/Documentation/dev-tools/kunit/index.rst
> @@ -11,6 +11,7 @@ KUnit - Unit Testing for the Linux Kernel
>  	usage
>  	kunit-tool
>  	api/index
> +	style
>  	faq
>  
>  What is KUnit?
> diff --git a/Documentation/dev-tools/kunit/style.rst b/Documentation/dev-tools/kunit/style.rst
> new file mode 100644
> index 000000000000..c001ea1cd87d
> --- /dev/null
> +++ b/Documentation/dev-tools/kunit/style.rst
> @@ -0,0 +1,207 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +===========================
> +Test Style and Nomenclature
> +===========================
> +
> +To make finding, writing, and using KUnit tests as simple as possible, it's
> +strongly encouraged that they are named and written according to the guidelines
> +below. While it's possible to write KUnit tests which do not follow these rules,
> +they may break some tooling, may conflict with other tests, and may not be run
> +automatically by testing systems.
> +
> +It's recommended that you only deviate from these guidelines when:
> +
> +1. Porting tests to KUnit which are already known with an existing name, or
> +2. Writing tests which would cause serious problems if automatically run (e.g.,
> +   nonderministically producing false positives or negatives, or taking an

s/nonderministically/nondeterministically/ (or non-deterministically?)

> +   extremely long time to run).
[...]
> +Test File and Module Names
> +==========================
> +
> +KUnit tests can often be compiled as a module. These modules should be named
> +after the test suite, followed by ``_test``. If this is likely to conflict with
> +non-KUnit tests, the suffic ``_kunit`` can also be used.

s/suffic/suffix/

> +The easiest way of achieving this is to name the file containing the test suite
> +``<suite>_test.c`` (or, as above, ``<suite>_kunit.c``). This file should be
> +placed next to the code under test.
> +
> +If the suite name contains some or all of the name of the test's parent
> +directory, it may make sense to modify the source filename to reduce redundancy.
> +For example, a ``foo_firmware`` suite could be in the ``foo/firmware_test.c``
> +file.

Reviewed-by: Marco Elver <elver@...gle.com>

Thank you!