linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
From: Brendan Higgins via Linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Petr Mladek <pmladek@suse.com>,
	Arpitha Raghunandan <98.arpi@gmail.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	linux-kernel@vger.kernel.org, rostedt@goodmis.org,
	sergey.senozhatsky@gmail.com, linux-kselftest@vger.kernel.org,
	linux-kernel-mentees@lists.linuxfoundation.org,
	kunit-dev@googlegroups.com
Subject: Re: [Linux-kernel-mentees] [PATCH] lib: Convert test_printf.c to KUnit
Date: Mon, 12 Oct 2020 13:46:25 -0700	[thread overview]
Message-ID: <20201012204625.GA56597@google.com> (raw)
In-Reply-To: <20200821122849.GS1891694@smile.fi.intel.com>

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/
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

  reply	other threads:[~2020-10-12 20:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-17  4:30 [Linux-kernel-mentees] [PATCH] lib: Convert test_printf.c to KUnit Arpitha Raghunandan
2020-08-17  7:06 ` Rasmus Villemoes
2020-08-21  4:54   ` Arpitha Raghunandan
2020-08-21 11:37   ` Petr Mladek
2020-08-21 12:19     ` Rasmus Villemoes
2020-10-12 20:13       ` Brendan Higgins via Linux-kernel-mentees
2020-10-12 20:48         ` Brendan Higgins via Linux-kernel-mentees
2020-08-21 12:28     ` Andy Shevchenko
2020-10-12 20:46       ` Brendan Higgins via Linux-kernel-mentees [this message]
2020-10-13  8:53         ` Arpitha Raghunandan
2020-10-13  9:55         ` Rasmus Villemoes

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=20201012204625.GA56597@google.com \
    --to=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=98.arpi@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=brendanhiggins@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    /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).