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