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

[Ilias Stamatis]

On Fri, Jun 7, 2019 at 12:25 PM Erik Skultety <eskultet at redhat.com> wrote:
>
> 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

I'm also fine with dropping the sleep, and I was a bit skeptical about
adding it in the first place.

Ilias