All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Brendan Higgins <brendan.higgins@linux.dev>,
	David Gow <davidgow@google.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>,
	"kunit-dev@googlegroups.com" <kunit-dev@googlegroups.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Maxime Ripard <maxime@cerno.tech>,
	Jonathan Cameron <jic23@kernel.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>
Subject: Re: [PATCH v5 1/8] drivers: kunit: Generic helpers for test device creation
Date: Thu, 23 Mar 2023 11:25:57 +0100	[thread overview]
Message-ID: <ZBwpNcd/A15/Azjr@kroah.com> (raw)
In-Reply-To: <3c09bda1-330d-6d49-ade5-aab567b3a0c4@gmail.com>

On Thu, Mar 23, 2023 at 11:20:33AM +0200, Matti Vaittinen wrote:
> On 3/23/23 10:58, Greg Kroah-Hartman wrote:
> > On Thu, Mar 23, 2023 at 07:17:40AM +0000, Vaittinen, Matti wrote:
> > > On 3/22/23 20:57, Greg Kroah-Hartman wrote:
> > > > On Wed, Mar 22, 2023 at 03:48:00PM +0200, Matti Vaittinen wrote:
> > > > > Hi Greg,
> > > > > 
> > > > > Thanks for looking at this.
> > > > > 
> > > > > On 3/22/23 14:07, Greg Kroah-Hartman wrote:
> > > > > > On Wed, Mar 22, 2023 at 11:05:55AM +0200, Matti Vaittinen wrote:
> 
> > > I am very conservative what comes to adding unit tests due to the huge
> > > inertia they add to any further development. I usually only add tests to
> > > APIs which I know won't require changing (I don't know such in-kernel
> > > APIs)
> > 
> > So anything that is changing doesn't get a test?
> 
> No. I think you misread me. I didn't say I don't like adding tests to code
> which changes. I said, I don't like adding tests to APIs which change.

Then you should not be writing any in-kernel tests as all of our APIs
change all the time.

>  If you only test
> > things that don't change then no tests fail, and so, why have the test
> > at all?
> 
> Because implementation cascading into functions below an API may change even
> if the API stays unchanged.

Then it needs to be fixed.

> > On the contrary, tests should be used to verify things that are changing
> > all the time, to ensure that we don't break things.
> 
> This is only true when your test code stays valid. Problem with excessive
> amount of tests is that more we have callers for an API, harder changing
> that API becomes. I've seen a point where people stop fixing "unimportant"
> things just because the amount of work fixing all impacted UT-cases would
> take. I know that many things went wrong before that project ended up to the
> point - but what I picked up with me is that carelessly added UTs do really
> hinder further development.

Again, in-kernel apis change at any moment.  I just changed one that was
over 15 years old.  Don't get stuck into thinking that you can only
write tests for stuff that is "stable" as nothing in the kernel is
"stable" and can change at any point in time.  You fix up all the
in-kernel users of the api, and the tests, and all is good.  That's how
kernel development works.

>  That's why we need
> > them, not to just validate that old code still is going ok.
> > 
> > The driver core is changing, and so, I would love to see tests for it to
> > ensure that I don't break anything over time.  That should NOT slow down
> > development but rather, speed it up as it ensures that things still work
> > properly.
> 
> I agree that there are cases where UTs are very handy and can add confidence
> that things work as intended. Still, my strong opinion is that people should
> consider what parts of code are really worth testing - and how to do the
> tests so that the amount of maintenance required by the tests stays low.
> It's definitely _not fun_ to do refactoring for minor improvement when 400+
> unit-test cases break. It's a point when many developers start seeing fixing
> this minor culprit much less important... And when people stop fixing minor
> things ... major things start to be just around the corner.

If people stop fixing minor things then the kernel development process
is dead.  Based on all the changes that go into it right now, we are far
from having that problem.

So write valid tests, if we get to the point where we have too much of a
problem fixing up the tests than the real users of apis, then we can
revisit it.  But for now, that's not an issue.

And again, remember, and api can, and will, change at any moment in
time, you can never know what will be "stable" as we do not have such a
thing.

thanks,

greg k-h

  reply	other threads:[~2023-03-23 10:28 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-22  9:05 [PATCH v5 0/8] Support ROHM BU27034 ALS sensor Matti Vaittinen
2023-03-22  9:05 ` Matti Vaittinen
2023-03-22  9:05 ` [PATCH v5 1/8] drivers: kunit: Generic helpers for test device creation Matti Vaittinen
2023-03-22 12:04   ` Greg Kroah-Hartman
2023-03-22 12:07   ` Greg Kroah-Hartman
2023-03-22 13:48     ` Matti Vaittinen
2023-03-22 18:57       ` Greg Kroah-Hartman
2023-03-23  7:17         ` Vaittinen, Matti
2023-03-23  8:58           ` Greg Kroah-Hartman
2023-03-23  9:20             ` Matti Vaittinen
2023-03-23 10:25               ` Greg Kroah-Hartman [this message]
2023-03-23 10:43                 ` Matti Vaittinen
2023-03-23 10:01             ` Matti Vaittinen
2023-03-23 10:27               ` Greg Kroah-Hartman
2023-03-23 11:00                 ` Matti Vaittinen
2023-03-23 10:12         ` Maxime Ripard
2023-03-23 10:21           ` Greg Kroah-Hartman
2023-03-23 12:16             ` Matti Vaittinen
2023-03-23 12:29               ` Maxime Ripard
2023-03-23 13:02                 ` Matti Vaittinen
2023-03-23 16:36                   ` Maxime Ripard
2023-03-24  6:11                     ` Matti Vaittinen
2023-03-24  6:34                       ` David Gow
2023-03-24  6:51                         ` Matti Vaittinen
2023-03-24  9:52                           ` David Gow
2023-03-24 10:05                             ` Matti Vaittinen
2023-03-24 10:17                               ` Matti Vaittinen
2023-03-25  4:35                                 ` David Gow
2023-03-25  7:26                                   ` Matti Vaittinen
2023-03-24 12:46                         ` Maxime Ripard
2023-03-24 12:31                       ` Maxime Ripard
2023-03-25  5:40                         ` David Gow
2023-03-29 19:43                           ` Maxime Ripard
2023-03-25 17:50                         ` Jonathan Cameron
2023-03-26 17:16                           ` Lars-Peter Clausen
2023-04-01 15:30                             ` Jonathan Cameron
2023-03-29 19:46                           ` Maxime Ripard
2023-04-01 15:36                             ` Jonathan Cameron
2023-03-24 12:36             ` Maxime Ripard
2023-03-24 12:43               ` Greg Kroah-Hartman
2023-03-24 13:02                 ` Maxime Ripard
2023-03-24 13:42                   ` Greg Kroah-Hartman
2023-03-22 12:08   ` Greg Kroah-Hartman
2023-03-23  7:30   ` David Gow
2023-03-23  8:35     ` Matti Vaittinen
2023-03-23  9:02     ` Greg Kroah-Hartman
2023-03-23 10:07     ` Maxime Ripard
2023-03-22  9:06 ` [PATCH v5 2/8] drm/tests: helpers: Use generic helpers Matti Vaittinen
2023-03-22  9:06   ` Matti Vaittinen
2023-03-22  9:06 ` [PATCH v5 3/8] dt-bindings: iio: light: Support ROHM BU27034 Matti Vaittinen
2023-03-22  9:06 ` [PATCH v5 4/8] iio: light: Add gain-time-scale helpers Matti Vaittinen
2023-03-22  9:07 ` [PATCH v5 5/8] iio: test: test " Matti Vaittinen
2023-03-24  6:29   ` Matti Vaittinen
2023-03-22  9:07 ` [PATCH v5 6/8] MAINTAINERS: Add IIO " Matti Vaittinen
2023-03-22  9:07 ` [PATCH v5 7/8] iio: light: ROHM BU27034 Ambient Light Sensor Matti Vaittinen
2023-03-26 16:19   ` Jonathan Cameron
2023-03-27  7:16     ` Vaittinen, Matti
2023-03-22  9:08 ` [PATCH v5 8/8] MAINTAINERS: Add ROHM BU27034 Matti Vaittinen
2023-03-22 10:01 ` [PATCH v5 0/8] Support ROHM BU27034 ALS sensor Andy Shevchenko
2023-03-22 10:01   ` Andy Shevchenko
2023-03-22 10:34   ` Javier Martinez Canillas
2023-03-22 10:34     ` Javier Martinez Canillas
2023-03-22 10:59     ` Matti Vaittinen
2023-03-22 10:59       ` Matti Vaittinen
2023-03-22 11:02       ` Andy Shevchenko
2023-03-22 11:02         ` Andy Shevchenko
2023-03-23  9:28       ` Maxime Ripard
2023-03-23  9:28         ` Maxime Ripard

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=ZBwpNcd/A15/Azjr@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Matti.Vaittinen@fi.rohmeurope.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=brendan.higgins@linux.dev \
    --cc=davidgow@google.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=jic23@kernel.org \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maxime@cerno.tech \
    --cc=mazziesaccount@gmail.com \
    --cc=rafael@kernel.org \
    --cc=sboyd@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.