Re: [PATCH v5 1/8] drivers: kunit: Generic helpers for test device creation

[Matti Vaittinen <mazziesaccount@gmail.com>]

This is a low priority babbling - feel free to skip if busy.

On 3/23/23 12:25, Greg Kroah-Hartman wrote:
> 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 agree. This is why I initially wrote:

 >>>> APIs which I know won't require changing (I don't know such in-kernel
 >>>> APIs)

> 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.

I don't. But I don't either think that UTs come with no cost. Thus I do 
only write tests when I see a _real need_ for one. If the APIs would be 
guaranteed not to change, then I would understand writing the tests for 
each and every "thing" without much of thinking if "the thing" is worth 
the test.

>  You fix up all the
> in-kernel users of the api, and the tests, and all is good.  That's how
> kernel development works.

Sure. This is how it works and how I think it should work. But I also 
have seen how this 'UT work overhead' has made people to decide not to 
touch things. Not in kernel but in other project. This is a real thing 
which can happen - many engineers like me are lazy bastards :)

>>   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.

And I am so happy for that. Kernel/drivers are still fun to work with. 
My personal preference is to keep it that way :)

> 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.

The beginning of your sentence hits the point. Write valid tests. I just 
encourage people to occasionally ask if the test they write is really a 
valid one. :)

> 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.

We agree on this.

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~