linux-kselftest.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: <Tim.Bird@sony.com>
To: <sharinder@google.com>
Cc: <davidgow@google.com>, <brendanhiggins@google.com>,
	<shuah@kernel.org>, <corbet@lwn.net>,
	<linux-kselftest@vger.kernel.org>, <kunit-dev@googlegroups.com>,
	<linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v1 2/7] Documentation: KUnit: Rewrite getting started
Date: Tue, 7 Dec 2021 18:50:29 +0000	[thread overview]
Message-ID: <BYAPR13MB2503E49BA2D4D59CF2A12F64FD6E9@BYAPR13MB2503.namprd13.prod.outlook.com> (raw)
In-Reply-To: <CAHLZCaEqr-0OipzKhDRXgtqrYqxNT6rNWpk5orFY0m5mYFdb2Q@mail.gmail.com>



> -----Original Message-----
> From: Harinder Singh <sharinder@google.com>
> 
> Hello Tim,
> 
> Thanks for the review comments.
> 
> I incorporated your comments in v2 here:
> https://lore.kernel.org/linux-kselftest/20211207054019.1455054-3-sharinder@google.com/
> 
> Please see my comments below.
> 
> On Sat, Dec 4, 2021 at 12:04 AM <Tim.Bird@sony.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Harinder Singh <sharinder@google.com>
> > > Sent: Thursday, December 2, 2021 9:25 PM
> > > To: davidgow@google.com; brendanhiggins@google.com; shuah@kernel.org; corbet@lwn.net
> > > Cc: linux-kselftest@vger.kernel.org; kunit-dev@googlegroups.com; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Harinder
> > > Singh <sharinder@google.com>
> > > Subject: [PATCH v1 2/7] Documentation: KUnit: Rewrite getting started
> > >
> > > Clarify the purpose of kunit_tool and fixed consistency issues
> > >
> > > Signed-off-by: Harinder Singh <sharinder@google.com>
> > > ---
> > >  Documentation/dev-tools/kunit/start.rst | 192 ++++++++++++------------
> > >  1 file changed, 98 insertions(+), 94 deletions(-)
> > >
> > > diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> > > index 1e00f9226f74..04b6b6a37488 100644
> > > --- a/Documentation/dev-tools/kunit/start.rst
> > > +++ b/Documentation/dev-tools/kunit/start.rst
> > > @@ -4,132 +4,131 @@
> > >  Getting Started
> > >  ===============
> > >
> > > -Installing dependencies
> > > +Installing Dependencies
> > >  =======================
> > > -KUnit has the same dependencies as the Linux kernel. As long as you can build
> > > -the kernel, you can run KUnit.
> > > +KUnit has the same dependencies as the Linux kernel. As long as you can
> > > +build the kernel, you can run KUnit.
> > >
> > > -Running tests with the KUnit Wrapper
> > > -====================================
> > > -Included with KUnit is a simple Python wrapper which runs tests under User Mode
> > > -Linux, and formats the test results.
> > > -
> > > -The wrapper can be run with:
> > > +Running tests with kunit_tool
> > > +=============================
> > > +kunit_tool is a Python script, which configures and build a kernel, runs
> >
> > build -> builds
> 
> Done
> 
> > > +tests, and formats the test results. From the kernel repository, you
> > > +can run kunit_tool:
> > >
> > >  .. code-block:: bash
> > >
> > >       ./tools/testing/kunit/kunit.py run
> > >
> > > -For more information on this wrapper (also called kunit_tool) check out the
> > > -Documentation/dev-tools/kunit/kunit-tool.rst page.
> > > +For more information on this wrapper, see:
> > > +Documentation/dev-tools/kunit/kunit-tool.rst.
> > > +
> > > +Creating a ``.kunitconfig``
> > > +---------------------------
> > > +If you want to run a specific set of tests (rather than those listed in
> > > +the KUnit ``defconfig``), you can provide Kconfig options in the
> > > +``.kunitconfig`` file.
> >
> > I know you didn't change this sentence, but it never made sense to me.
> > If we're in here changing the format, can we rewrite this to be more clear?
> >
> > What is the purpose of .kunitconfig?
> >
> > Here's an alternative wording (which I'm not sure is correct):
> >
> > By default, KUnit provides a ``defconfig`` which runs all of the unit
> > tests.  However, you can control which set of unit tests to run by creating
> > a ``.kunitconfig`` file with kernel config options that enable only a specific
> > set of tests and their dependencies.
> 
> Rewrote the paragraph.
> 
> > > This file contains the regular
> > > +Kernel config with the specific test targets. The
> >
> > What does "This file contains the regular Kernel config" mean?
> > Does it have all the entries from a standard .config file?
> 
> Rewrote the paragraph.
> 
> > My kunit default.config looks like this:
> > CONFIG_KUNIT=y
> > CONFIG_KUNIT_EXAMPLE_TEST=y
> > CONFIG_KUNIT_ALL_TESTS=y
> >
> > I think it would be better to say something like:
> > "This file contains the default configuration for KUnit, which is to run an example
> > test and all unit tests"
> 
> I am not sure what you mean. This part is not talking about the
> default.config. We reworded the section in the next version. If it is
> not clear please elaborate.

I read the section in the v2 patch, and it is much improved.

Thanks,
 -- Tim


  reply	other threads:[~2021-12-07 18:50 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-03  4:24 [PATCH v1 0/7] Documentation: KUnit: Rework KUnit documentation Harinder Singh
2021-12-03  4:24 ` [PATCH v1 1/7] Documentation: KUnit: Rewrite main page Harinder Singh
2021-12-03 17:32   ` Tim.Bird
2021-12-07  5:46     ` Harinder Singh
2021-12-03  4:24 ` [PATCH v1 2/7] Documentation: KUnit: Rewrite getting started Harinder Singh
2021-12-03 18:34   ` Tim.Bird
2021-12-07  5:53     ` Harinder Singh
2021-12-07 18:50       ` Tim.Bird [this message]
2021-12-03  4:24 ` [PATCH v1 3/7] Documentation: KUnit: Added KUnit Architecture Harinder Singh
2021-12-03  4:24 ` [PATCH v1 4/7] Documentation: kunit: Reorganize documentation related to running tests Harinder Singh
2021-12-03  4:24 ` [PATCH v1 5/7] Documentation: KUnit: Rework writing page to focus on writing tests Harinder Singh
2021-12-03  4:24 ` [PATCH v1 6/7] Documentation: KUnit: Restyle Test Style and Nomenclature page Harinder Singh
2021-12-03  4:24 ` [PATCH v1 7/7] Documentation: KUnit: Restyled Frequently Asked Questions Harinder Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR13MB2503E49BA2D4D59CF2A12F64FD6E9@BYAPR13MB2503.namprd13.prod.outlook.com \
    --to=tim.bird@sony.com \
    --cc=brendanhiggins@google.com \
    --cc=corbet@lwn.net \
    --cc=davidgow@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=sharinder@google.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).