Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 16 Jun 2020 10:40:22 +0100 (BST)
From:   Alan Maguire <alan.maguire@oracle.com>
To:     David Gow <davidgow@google.com>
cc:     Kees Cook <keescook@chromium.org>,
        Vitor Massaru Iha <vitor@massaru.org>,
        KUnit Development <kunit-dev@googlegroups.com>,
        Shuah Khan <skhan@linuxfoundation.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Brendan Higgins <brendanhiggins@google.com>,
        linux-kernel-mentees@lists.linuxfoundation.org,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: Re: common KUnit Kconfig and file naming (was: Re: [PATCH] lib:
 kunit_test_overflow: add KUnit test of check_*_overflow functions)
In-Reply-To: <CABVgOSkPYBL8Qy9Fsr0owNhh2=tXZmdwCsWhnq+PshJLr06YKA@mail.gmail.com>
Message-ID: <alpine.LRH.2.21.2006160956170.16027@localhost>
References: <20200611215501.213058-1-vitor@massaru.org> <202006121403.CF8D57C@keescook> <CABVgOSnofuJQ_fiCL-8KdKezg3Hnqk3A+X509c4YP_toKeBVBg@mail.gmail.com> <202006141005.BA19A9D3@keescook>
 <CABVgOSkPYBL8Qy9Fsr0owNhh2=tXZmdwCsWhnq+PshJLr06YKA@mail.gmail.com>
User-Agent: Alpine 2.21 (LRH 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 16 Jun 2020, David Gow wrote:

> CONFIG_PM_QOS_KUNIT_TESTOn Mon, Jun 15, 2020 at 1:48 AM Kees Cook
> <keescook@chromium.org> wrote:
> >
> > On Sat, Jun 13, 2020 at 02:51:17PM +0800, David Gow wrote:
> > > Yeah, _KUNIT_TEST was what we've sort-of implicitly decided on for
> > > config names, but the documentation does need to happen.
> >
> > That works for me. It still feels redundant, but all I really want is a
> > standard name. :)
> >
> > > We haven't put as much thought into standardising the filenames much, though.
> >
> > I actually find this to be much more important because it is more
> > end-user-facing (i.e. in module naming, in build logs, in scripts, on
> > filesystem, etc -- CONFIG is basically only present during kernel build).
> > Trying to do any sorting or greping really needs a way to find all the
> > kunit pieces.
> >
> 
> Certainly this is more of an issue now we support building KUnit tests
> as modules, rather than having them always be built-in.
> 
> Having some halfway consistent config-name <-> filename <-> test suite
> name could be useful down the line, too. Unfortunately, not
> necessarily a 1:1 mapping, e.g.:
> - CONFIG_KUNIT_TEST compiles both kunit-test.c and string-stream-test.c
> - kunit-test.c has several test suites within it:
> kunit-try-catch-test, kunit-resource-test & kunit-log-test.
> - CONFIG_EXT4_KUNIT_TESTS currently only builds ext4-inode-test.c, but
> as the plural name suggests, might build others later.
> - CONFIG_SECURITY_APPARMOR_KUNIT_TEST doesn't actually have its own
> source file: the test is built into policy_unpack.c
> - &cetera
> 
> Indeed, this made me quickly look up the names of suites, and there
> are a few inconsistencies there:
> - most have "-test" as a suffix
> - some have "_test" as a suffix
> - some have no suffix
> 
> (I'm inclined to say that these don't need a suffix at all.)
> 

A good convention for module names - which I _think_ is along the lines
of what Kees is suggesting - might be something like

<subsystem>[_<optional_test-area>]_kunit.ko

So for example

kunit_test -> test_kunit.ko
string_stream_test.ko -> test_string_stream_kunit.ko
kunit_example_test -> example_kunit.ko
ext4_inode_test.ko -> ext4_inode_kunit.ko

For the kunit selftests, "selftest_" might be a better name
than "test_", as the latter might encourage people to reintroduce
a redundant "test" into their module name.  

> Within test suites, we're also largely prefixing all of the tests with
> a suite name (even if it's not actually the specified suite name). For
> example, CONFIG_PM_QOS_KUNIT_TEST builds
> drivers/base/power/qos-test.c which contains a suite called
> "qos-kunit-test", with tests prefixed "freq_qos_test_". Some of this
> clearly comes down to wanting to namespace things a bit more
> ("qos-test" as a name could refer to a few things, I imagine), but
> specifying how to do so consistently could help.
> 

Could we add some definitions to help standardize this?
For example, adding a "subsystem" field to "struct kunit_suite"?

So for the ext4 tests the "subsystem" would be "ext4" and the
name "inode" would specify the test area within that subsystem.
For the KUnit selftests, the subsystem would be "test"/"selftest".
Logging could utilize the subsystem definition to allow test
writers to use less redundant test names too.  For example
the suite name logged could be constructed from the
subsystem + area values associated with the kunit_suite,
and individual test names could be shown as the
suite area + test_name.

Thanks!

Alan