On Tue, Mar 15, 2022 at 12:11:50AM +0100, Brendan Higgins wrote:
On Fri, Mar 11, 2022 at 11:24 AM Vincent Whitchurch vincent.whitchurch@axis.com wrote:
+class TestOPT3001(UMLTestCase):
I am partial to starting with UML since there are a lot of nice easy things about starting there; however, I imagine people will eventually want to use this on other architectures (speaking from experience). How difficult do you think it would be to extend this to support manipulating fake devices in say QEMU?
It should be possible, but upstream QEMU doesn't have everything that we need so some work is needed there. Also, of course work is need to provide user space for running the tests and communicating between the virtual machine and the backend:
- We need user space, so build scripts would need to be provided to cross-compile busybox and Python (and whatever libraries it needs) for the target architecture.
- We also use UML's hostfs feature to make things transparent to the user and to avoid having to set up things like networking for communication between the host and the backend. I think QEMU's 9pfs support can be used as a rootfs too but it's not something I've personally tested.
- We use virtio-i2c and virtio-gpio and use virtio-uml which uses the vhost-user API to communicate from UML to the backend. The latest version of QEMU has support for vhost-user-i2c, but vhost-user-gpio doesn't seem to have been merged yet, so work is needed on the QEMU side. This will also be true for other buses in the future, if they are implemented with new virtio devices.
- For MMIO, UML has virtio-mmio which allows implementing any PCIe device (and by extension any platform device) outside of UML, but last I checked, upstream QEMU did not have something similar.
I also have some colleagues inside of Google that worked on some projects to simulate simple devices on an FPGA to test software and adjacent devices in a conceptually similar way; one of these teams built a Domain Specific Language kind of like roadtest to implement the tests and the environment for the tests. The main reason I mention this here is I am thinking about maybe one day having an API you can implement so you can run your roadtests on UML, QEMU, or on any emulator or hardware testbed that implements the appropriate API.
I'll try to dig up some people who might be interested and add them here.
- dts = DtFragment(
src="""+&i2c {
- light-sensor@$addr$ {
compatible = "ti,opt3001";reg = <0x$addr$>;- };
+};
""",variables={"addr": DtVar.I2C_ADDR,},- )
- @classmethod
- def setUpClass(cls) -> None:
insmod("opt3001")- @classmethod
- def tearDownClass(cls) -> None:
rmmod("opt3001")- def setUp(self) -> None:
self.driver = I2CDriver("opt3001")self.hw = Hardware("i2c")self.hw.load_model(OPT3001)- def tearDown(self) -> None:
self.hw.close()- def test_illuminance(self) -> None:
data = [# Some values from datasheet, and 0(0b_0000_0000_0000_0000, 0),(0b_0000_0000_0000_0001, 0.01),(0b_0011_0100_0101_0110, 88.80),(0b_0111_1000_1001_1010, 2818.56),]with self.driver.bind(self.dts["addr"]) as dev:luxfile = dev.path / "iio:device0/in_illuminance_input"for regval, lux in data:self.hw.reg_write(REG_RESULT, regval)self.assertEqual(read_float(luxfile), lux)I love the framework; this looks very easy to use.
One nit about this test; it seems like you cover just one test case here - the happy path. Can you cover some other one? Particularly some error paths?
Sorry, I am not trying to be cheeky here; it looks like this driver actually should probably be fully (or very close to fully) testable via roadtest as I understand it. It only looks like there are a handful of cases to cover for the driver: the device is busy, the device returned something invalid, the user requested something invalid, and several SMBus read/write failures - it really only looks like there are a handful of paths and I think they are all accessible via the I2C interface (except for maybe the user requesting something invalid).
Yes, there are more things that could be tested in this driver. However, as the commit message says, I only indented this particular test to serve as a regression test for the specific bug fix, which would need an environment where the chip detects 0 lux to be able to test on real hardware. There are a few reasons for this:
- Unlike the other drivers being tested in this series, I don't have access to boards with this chip so my interest in this particular piece of hardware is limited.
- I actually started writing more tests for this driver earlier on (specifically, testing the configuration which uses interrupts), but I quickly discovered that this driver has race conditions which result in unbalanced mutex locking (in brief: the ok_to_ignore_lock stuff is broken). This shows the value of the test framework, but I also didn't want to write non-trivial fixes for drivers where I didn't have real hardware to test.
- Also, some paths in this driver needs a modification to be tested under roadtest. It uses wait_event_timeout() with a fixed value, but we cannot guarantee that this constraint is met in the test environment since it depends on things like CPU load on the host.
(Also, we use UML's "time travel" feature which essentially fast-forwards through idle time, so the constraint can never be met in practice.)
So the timeout parameter would have to be made adjustable via say a module parameter, to be able to make it infinite (to test the normal case) and not (to be able to test timeout handling). I think this could be done fairly cleanly with a one- or two-liner patch to the driver and by hiding the details in a header file behind a roadtest-specific config option, but I wanted to avoid having to patch the kernel proper for the initial version of the framework.
For vcnl4000, I have actually inherited some out-of-tree patches which are in need of mainlining so the tests are a bit more complete since I'm hoping to send some patches to that driver soon. The period mode busy handling is not tested there either though, I can try to add that.
As for I2C API failures, I have not added tests for them in any of the drivers. There's not much the test cases could assert, other than perhaps error propagation, so it's unclear if there is enough value compared to the effort required to implement test cases to make sure that every I2C transaction's failure path is tested.
But I think that we do want to at least make sure the error paths are executed, to check that drivers don't crash or deadlock due to faulty cleanups and the like. A way to solve this could be to implement fault injection support in the I2C framework. The fail-nth feature could be used to systemically trigger each and every I2C transaction failure and check that the driver doesn't blow up, while using the roadtest as a means to hit the various code paths in the driver during each of the iterations of fail-nth. Fault injection support would also be helpful when testing on real hardware.