Open Stack

On Fri, Aug 22, 2014 at 11:32:31AM -0400, Solly Ross wrote:
> (response inline)
> 
> ----- Original Message -----
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > To: "Solly Ross" <sross at redhat.com>
> > Cc: "OpenStack Development Mailing List (not for usage questions)" <openstack-dev at lists.openstack.org>
> > Sent: Thursday, August 21, 2014 11:23:17 AM
> > Subject: Re: [openstack-dev] [nova][libvirt] Non-readonly connection to libvirt in unit tests
> > 
> > On Thu, Aug 21, 2014 at 11:14:33AM -0400, Solly Ross wrote:
> > > (reply inline)
> > > 
> > > ----- Original Message -----
> > > > From: "Daniel P. Berrange" <berrange at redhat.com>
> > > > To: "OpenStack Development Mailing List (not for usage questions)"
> > > > <openstack-dev at lists.openstack.org>
> > > > Sent: Thursday, August 21, 2014 11:05:18 AM
> > > > Subject: Re: [openstack-dev] [nova][libvirt] Non-readonly connection to
> > > > libvirt in unit tests
> > > > 
> > > > On Thu, Aug 21, 2014 at 10:52:42AM -0400, Solly Ross wrote:
> > > > > FYI, the context of this is that I would like to be able to test some
> > > > > of the libvirt storage pool code against a live file system, as we
> > > > > currently test the storage pool code.  To do this, we need at least to
> > > > > be able to get a proper connection to a session daemon.  IMHO, since
> > > > > these calls aren't "expensive", so to speak, it should be fine to have
> > > > > them run against a real libvirt.
> > > > 
> > > > No it really isn't OK to run against the real libvirt host system when
> > > > in the unit tests. Unit tests must *not* rely on external system state
> > > > in this way because it will lead to greater instability and unreliability
> > > > of our unit tests. If you want to test stuff against the real libvirt
> > > > storage pools then that becomes a functional / integration test suite
> > > > which is pretty much what tempest is targetting.
> > > 
> > > That's all well and good, but we *currently* manipulates the actual file
> > > system manually in tests.  Should we then say that we should never
> > > manipulate
> > > the actual file system either?  In that case, there are some tests which
> > > need to be refactored.
> > 
> > Places where the tests manipulate the filesystem though should be doing
> > so in an isolated playpen directory, not in the "live" location where
> > a deployed nova runs, so that's not the same thing.
> 
> Ah, but in the case I mentioned before, we're dealing with storage pools,
> which can just be created in the "playpen directory".  In that case, libvirt
> is simply acting as a library for filesystem access.  To further ensure isolation,
> you could also connect to a session daemon instead of a system daemon.
> 
> I'm of the opinion that requiring some form of libvirt to be installed to run the *libvirt*
> unit tests isn't actually that big of a deal, since you can build libvirt without "extra stuff"
> and get a libvirt that has just enough for you to test against.  Generally it's the developers
> that will be running the unit tests (and the CI), and if a developer is running the libvirt
> unit tests, he or she is probably developing for the libvirt driver, and thus should probably
> have libvirt installed in some form. 

The unit tests are run regardless of whether the developer is working on
libvirt or not. The more libvirt setup we require for the tests the more
pain we're inflicting on non-libvirt developers. That is a big deal for
them.

> > > > I'm actually increasingly of the opinion that we should not in fact
> > > > be trying to use the real libvirt library in the unit tests at all
> > > > as it is not really adding any value. We typically nmock out all the
> > > > actual API calls we exercise so despite "using" libvirt-python we
> > > > are not in fact exercising its code or even validating that we're
> > > > passing the correct numbers of parameters to API calls. Pretty much
> > > > all we really relying on is the existance of the various global
> > > > constants that are defined, and that has been nothing but trouble
> > > > because the constants may or may not be defined depending on the
> > > > version.
> > > 
> > > Isn't that what 'test:///default' is supposed to be?  A version of libvirt
> > > with libvirt not actually touching the rest of the system?
> > 
> > Yes, that is what it allows for, however, even if we used that URI we
> > still wouldn't be actually exercising any of the libvirt code in any
> > meaningful way because our unit tests mock out all the API calls that
> > get touched. So using libvirt-python + test:///default URI doesn't
> > really seem to buy us anything, but it does still mean that developers
> > need to have libvirt installed in order to run  the unit tests. I'm
> > not convinced that is a beneficial tradeoff.
> 
> I think it would make writing unit tests easier, because you don't have
> to worry about making sure that the fakelibvirt implementation matches
> the real libvirt implementation, and you don't have to go adding extra
> methods to fakelibvirt to get things to work.

Those problems are all artifacts of the way fakelibvirt is /currently/
written. You can easily solve them by auto-generating the stub classes
and methods with a script, so there is no longer an issue keeping them
in sync or manual work to add more methods, as described below:

> > > > The downside of fakelibvirt is that it is a half-assed implementation
> > > > of libvirt that we evolve in an adhoc fashion. I'm exploring the idea
> > > > of using pythons introspection abilities to query the libvirt-python
> > > > API and automatically generate a better 'fakelibvirt' that we can
> > > > guarantee to match the signatures of the real libvirt library. If we
> > > > had something like that which we had more confidence in, then we could
> > > > make the unit tests use that unconditionally. This would make our unit
> > > > tests more reliable since we would not be suspectible to different API
> > > > coverage in different libvirt module versions which have tripped us up
> > > > so many times

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|