Re: [Linux-kernel-mentees] common KUnit Kconfig and file naming (was: Re: [PATCH] lib: kunit_test_overflow: add KUnit test of check_*_overflow functions)
From: Alan Maguire <hidden>
Date: 2020-06-16 09:40:50
Also in:
linux-kselftest, lkml
On Tue, 16 Jun 2020, David Gow wrote:
CONFIG_PM_QOS_KUNIT_TESTOn Mon, Jun 15, 2020 at 1:48 AM Kees Cook [off-list ref] wrote:quoted
On Sat, Jun 13, 2020 at 02:51:17PM +0800, David Gow wrote:quoted
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. :)quoted
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 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees