Re: [libvirt] Scope of the test driver? (Was: Re: [PATCH] test_driver: implement virDomainSendKey)

On Mon, Jun 03, 2019 at 03:51:29PM +0200, Peter Krempa wrote: > On Mon, Jun 03, 2019 at 14:16:15 +0200, Pavel Hrdina wrote: > > On Mon, Jun 03, 2019 at 01:48:55PM +0200, Peter Krempa wrote: > > [...] > > > > Sure we want to cover it, but to which degree? That's what I want to > > > clarify. If we do that, answering questions how to do things should be > > > easier. > > > > To answer your scope question this is what virt-manager is using test > > driver for: > > > > - It's used by CLI commands to validate our functionality whether we > > can generate correct XMLs and use the specific libvirt features, > > this is probably what you've called "unit" testing. > > This is an interresting point. Havin a way to validate XMLs seems > useful. > > In this case we should e.g. make sure that the APIs taking XMLs do > schema validation maybe even when not asked for via the API flag. > > For normal drivers this could create regressions but this way you force > users to check their XMLs in the test driver, which should be okay in > this case and would actually add some value. > > > - We have some basich UI testing in virt-manager that would ideally > > cover all UI elements and that one may be considered more like CI > > testing even though it uses test-driver as well > > I'd be cautious calling it CI testing. The test driver is equivalent of > the mock data we use in libvirt, which is uncoupled from reality at this > point. It partially approaches CI territory in terms of XML validation > (if used with the schema checker flag), but most other APIs are > copypaste of code from the drivers which is not updated usually when > doing changes in the driver itself. > > > - During development sometimes we use test driver to roughly check > > some feature before it will be tested using real driver with > > proper host and VM setup where that setup might not be easy or > > the specific HW required for some feature is not that easily > > available. > > So this is a one-off effort mostly. And in the end you still need to > check it against a real hypervisor anyways. > > > In libvirt-dbus we use test driver as well to validate that APIs > > provided over DBus actually works but again it's more like the "unit" > > testing where we would not probably care about any specific blocking > > behavior. > > So in this case it's again mostly unit testing so that the libvirt-dbus > project does not need to simulate the backend. The expected returns are > mostly success as it in fact wants to test the transport and not the > APIs themself. > > This scenario would make simulating the delay in the sendkey API mostly > counterproductive as you are not very likely to hit threading problems > with a fixed delay and in the end it will simulate a well known > situation. I guess it's time to make a conclusion. There were arguments supporting either of the approaches, however (at least to me) leaning a bit more towards dropping the sleep as currently the usage is mostly in automated manner. Therefore, I'll proceed with dropping the delay. If you strongly feel like it indeed should be in place, feel free to post a follow-up patch. Erik