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:
+/**
- test_kunit_helper_alloc_device - Allocate a mock device for a KUnit test
- @test: The test context object
- This allocates a fake struct &device to create a mock for a KUnit
- test. The device will also be bound to a fake driver. It will thus be
- able to leverage the usual infrastructure and most notably the
- device-managed resources just like a "real" device.
What specific "usual infrastructure" are you wanting to access here?
And again, if you want a fake device, make a virtual one, by just calling device_create().
Or are you wanting to do "more" with that device pointer than device_create() can give you?
Personally, I was (am) only interested in devm_ unwinding. I guess the device_create(), device_add(), device_remove()... (didn't study this sequence in details so sorry if there is errors) could've been sufficient for me. I haven't looked how much of the code that there is for 'platform devices' should be duplicated to support that sequence for testability purposes.
Any device can access devm_ code, there's no need for it to be a platform device at all.
The biggest thing for me is that I don't like the idea of creating own 'test device' in <add subsystem here> while we already have some in DRM (or others). Thus, I do see value in adding generic helpers for supporting running KUnit tests on devm_* APIs. Hence it'd be good to have _some_ support for it.
I agree, let's use a virtual device and a virtual bus (you can use the auxbus code for this as that's all there for this type of thing)
Hm. The auxiliary_devices require parent. What would be the best way to deal with that in KUnit tests?
If you use NULL as the parent, it goes into the root.
then you can test the devm_* code, _AND_ you can test the driver core at the same time.
And having them in drivers/base/test seemed like a correct place to me. What I really don't know is if there are legitimate use-cases for using platform_devices in DRM tests. Perhaps Maxime can shed light on that.
I agree that this could be in drivers/base/test/ but then let's test the driver core, not just provide a dummy platform device.
If you want to test the platform driver/device api, that would be great too, that can be plaform device/driver specific, but don't use one for some other random driver core functionality please.
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? If you only test things that don't change then no tests fail, and so, why have the test at all?
On the contrary, tests should be used to verify things that are changing all the time, to ensure that we don't break things. 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.
- or to functions which I think are error-prone. So, I am probably
one of the last persons adding UTs to code I don't know :)
That's fine, you don't have to add test code for stuff you don't know.
But again, do NOT abuse a platform device for this, that's not ok, and the in-kernel code that does do this should be fixed up.
thanks,
greg k-h