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:   Mon, 12 Oct 2020 13:46:25 -0700
From:   Brendan Higgins <brendanhiggins@google.com>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Petr Mladek <pmladek@suse.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Arpitha Raghunandan <98.arpi@gmail.com>,
        skhan@linuxfoundation.org, rostedt@goodmis.org,
        sergey.senozhatsky@gmail.com, kunit-dev@googlegroups.com,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [PATCH] lib: Convert test_printf.c to KUnit
Message-ID: <20201012204625.GA56597@google.com>
References: <20200817043028.76502-1-98.arpi@gmail.com>
 <f408efbd-10f7-f1dd-9baa-0f1233cafffc@rasmusvillemoes.dk>
 <20200821113710.GA26290@alley>
 <20200821122849.GS1891694@smile.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200821122849.GS1891694@smile.fi.intel.com>
Precedence: bulk

On Fri, Aug 21, 2020 at 03:28:49PM +0300, Andy Shevchenko wrote:
> On Fri, Aug 21, 2020 at 01:37:10PM +0200, Petr Mladek wrote:
> > On Mon 2020-08-17 09:06:32, Rasmus Villemoes wrote:
> > > On 17/08/2020 06.30, Arpitha Raghunandan wrote:
> > > > Converts test lib/test_printf.c to KUnit.
> > > > More information about KUnit can be found at
> > > > https://www.kernel.org/doc/html/latest/dev-tools/kunit/index.html.
> > > > KUnit provides a common framework for unit tests in the kernel.
> > > 
> > > So I can continue to build a kernel with some appropriate CONFIG set to
> > > y, boot it under virt-me, run dmesg and see if I broke printf? That's
> > > what I do now, and I don't want to have to start using some enterprisy
> > > framework.
> > 
> > I had the same concern. I have tried it.

Sorry you feel that way. Do you have any suggestions on how we can make
it seem less enterprisy? Seems like there are people here who are not a
fan of the output format, so of which we can fix here, some of which is
part of KTAP[1].

> Which raises an obvious question: did the people who convert this test this
> themselves? Looks like a janitor work in the area without understanding the
> area good enough.

Looks to me like Arpitha ran it, but you are right, we don't have a lot
of familiarity with this area; we were treating it as "janitor work" as
you say.

Our intention was just to take some existing tests and as non-invasively
as possible, get them to report using a common format, and maybe even
get some of the tests to follow a common pattern.

> Probably I will NAK all those patches from now on, until it will be good commit
> messages and cover of risen aspects, including reference to before and after
> outcome for passed and failed test cases.

Fair enough, hopefully we can address these issues in the next revision.

One issue though, with the "before and after outcome" you are
referencing; are you referring to the issue that Petr pointed out in how
they are inconsistent:

? ?+ original code: vsnprintf(buf, 6, "%pi4|%pI4", ...) wrote '127.0', expected '127-0'
? ?+ kunit code: vsnprintf(buf, 20, "%pi4|%pI4", ...) wrote '127.000.000.001|127', expected '127-000.000.001|127' ?

(I think Rasmus addressed this.) Or are your referring to something
else?

> Brendan, I guess the ball now on your side to prove this is good activity.

And I see that we are off to a great start! :-)

In all seriousness, I am really sorry about this. I kind of bungled this
up trying to go after too many of these conversions at once.

Arpitha, can you get this follow up patch out?

[1] https://lore.kernel.org/linux-kselftest/CY4PR13MB1175B804E31E502221BC8163FD830@CY4PR13MB1175.namprd13.prod.outlook.com/