From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.6 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29E69C433DF for ; Mon, 22 Jun 2020 21:33:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E78E62076E for ; Mon, 22 Jun 2020 21:33:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="VmiajD07" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730679AbgFVVdS (ORCPT ); Mon, 22 Jun 2020 17:33:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38488 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730678AbgFVVdR (ORCPT ); Mon, 22 Jun 2020 17:33:17 -0400 Received: from mail-pf1-x443.google.com (mail-pf1-x443.google.com [IPv6:2607:f8b0:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 26AC3C061573 for ; Mon, 22 Jun 2020 14:33:17 -0700 (PDT) Received: by mail-pf1-x443.google.com with SMTP id j12so6782658pfn.10 for ; Mon, 22 Jun 2020 14:33:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=48FKT2aNWMCF7CnKbQrKlzmt9BG2SQkryuha/wRgHcU=; b=VmiajD07pDP5KDQbZ3QiMj0D7fvgmF7J9dUgGw4/Z6unPCcSGSusWg6ohdomzmxmI3 eRuyd0vbRPT2EQpDxIjpTwFqfF64NxhmvV7bKkwPP02I4FE3uNvwp35LX7rFGtb2rwoZ BsYJA45tDwD7hi7TNIYukJGtXsBlIZ1+YHBF0YSeGSrsjrcRooOwW6ZONbujKIy6Le9S 8eeqzyEMHc6H9gkOrZHFsHvdJJEHwspka5F3arDEeTuKWE/7J1+/n1d0PuR9lKVMvulB 6RvrFt5ukQdhht2wH8IXYfYMIFJ8mNhIbmy6pyoICNqqEnuDSRbLAw79a0GxaHeGVOex zNKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=48FKT2aNWMCF7CnKbQrKlzmt9BG2SQkryuha/wRgHcU=; b=slr6tsDg/ukKHYOLowHuOG9g959mkTcROLMmzvNXG7dfb4ihKSvn09S8zdORL4Z/Wi fz6UJusSwidkgxY1L9XD2qE7xfwLYNu5KHRnNcgX7WzvbVNLSLZMIuad5o9BFp0qBwm5 0AFjuUwFYJPGB5MrfFADOUBGsLSE+IXY7IG4FZiLvGOCtnhFcTpP0To+V7ALuc+TxWFz li0D+glpbvOxuWGDQu3p42kzr1P5ZtiAW+EZqdid0RofYoV29w+Tneb0s8kEc9JbLqcN 6objaeaVbj4uPBnMvlP3ewiKUFsPGn7xY4X34vod02gOTuHu6w4boXhsYY/3SFpgHOhB Ba4w== X-Gm-Message-State: AOAM531GXfhcKD1YwfqlFvxSolWwTte0ZaubE4Q2RVqjAw9FjgTMLPGG kJgBSJOKuRk42b2tyKVFz4fiShrpCFtInh6424vR5Q== X-Google-Smtp-Source: ABdhPJwhDlpWz0+PUiWIW28s6ahSX1TxyWf94XwDdj1zzNw6tcGSacG5OaH+oulNadzX/yULBfSnKEFDPP63CcyRSVM= X-Received: by 2002:a63:6e0e:: with SMTP id j14mr1129571pgc.384.1592861596328; Mon, 22 Jun 2020 14:33:16 -0700 (PDT) MIME-Version: 1.0 References: <20200620054944.167330-1-davidgow@google.com> In-Reply-To: <20200620054944.167330-1-davidgow@google.com> From: Brendan Higgins Date: Mon, 22 Jun 2020 14:33:05 -0700 Message-ID: Subject: Re: [PATCH] Documentation: kunit: Add naming guidelines To: David Gow , "Theodore Ts'o" , "Bird, Timothy" Cc: Jonathan Corbet , Kees Cook , Alan Maguire , KUnit Development , "open list:KERNEL SELFTEST FRAMEWORK" , "open list:DOCUMENTATION" , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org 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 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 > --- > 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 > to be _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 > + > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > +Test Style and Nomenclature > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > + > +Subsystems, Suites, and Tests > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > + > +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 ar= ea 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 kern= el. A > +rule of thumb is that a test subsystem should match a single kernel modu= le. If > +the code being tested can't be compiled as a module, in many cases the s= ubsystem > +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 simi= lar > +areas. > + > +Test subsystems should be named after the code being tested, either afte= r 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 separat= ed 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 itsel= f. > + > +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 whic= h > + provides a simple, consistent way for humans to find and run tes= ts. 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 a= nd > +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 subsyst= em > +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 specif= ied as > +the ``.name`` member of the ``kunit_suite`` struct, and forms the base f= or 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' r= esults > +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 w= ith > + other C identifiers in the kernel. This may require some creativ= e > + 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 tes= t suite, > +the *fully-qualified* name of a test should be the suite name followed b= y the > +test name, separated by a colon (i.e. ``suite:test``). > + > +Test Kconfig Entries > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > + > +Every test suite should be tied to a Kconfig entry. > + > +This Kconfig entry must: > + > +* be named ``CONFIG__KUNIT_TEST``: where 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]=E2=86=92[Kernel Testing and Cover= age] > +* 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 b= uilt 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 gene= ral, please refer > + to the KUnit documentation in Documentation/dev-tool= s/kunit > + > + If unsure, say N > + > + > +Test Filenames > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > + > +Where possible, test suites should be placed in a separate source file i= n the > +same directory as the code being tested. > + > +This file should be named ``_kunit.c``. It may make sense to stri= p > +excessive namespacing from the source filename (e.g., ``firmware_kunit.c= `` instead of > +``_firmware.c``), but please ensure the module name does con= tain the > +full suite name. > + > + > -- > 2.27.0.111.gc72c7da667-goog >