RE: [PATCH v1 2/7] Documentation: KUnit: Rewrite getting started

[Tim.Bird]

> -----Original Message-----
> From: Harinder Singh <sharinder@xxxxxxxxxx>
> 
> Hello Tim,
> 
> Thanks for the review comments.
> 
> I incorporated your comments in v2 here:
> https://lore.kernel.org/linux-kselftest/20211207054019.1455054-3-sharinder@xxxxxxxxxx/
> 
> Please see my comments below.
> 
> On Sat, Dec 4, 2021 at 12:04 AM <Tim.Bird@xxxxxxxx> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Harinder Singh <sharinder@xxxxxxxxxx>
> > > Sent: Thursday, December 2, 2021 9:25 PM
> > > To: davidgow@xxxxxxxxxx; brendanhiggins@xxxxxxxxxx; shuah@xxxxxxxxxx; corbet@xxxxxxx
> > > Cc: linux-kselftest@xxxxxxxxxxxxxxx; kunit-dev@xxxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Harinder
> > > Singh <sharinder@xxxxxxxxxx>
> > > 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@xxxxxxxxxx>
> > > ---
> > >  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