On 8/21/2014 11:37 AM, Clark Boylan wrote:


On Thu, Aug 21, 2014, at 09:25 AM, Matt Riedemann wrote:


On 8/21/2014 10:23 AM, Daniel P. Berrange wrote:
On Thu, Aug 21, 2014 at 11:14:33AM -0400, Solly Ross wrote:
(reply inline)

----- Original Message -----
From: "Daniel P. Berrange" <berra...@redhat.com>
To: "OpenStack Development Mailing List (not for usage questions)" 
<openstack-dev@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.

So If we require libvirt-python for tests and that requires
libvirt-bin, what's stopping us from just removing fakelibvirt since
it's kind of useless now anyway, right?

The thing about fakelibvirt is that it allows us to operate against
against a libvirt API without actually doing libvirt-y things like
launching VMs.  Now, libvirt does have a "test:///default" URI that
IIRC has similar functionality, so we could start to phase out fake
libvirt in favor of that.  However, there are probably still some
spots where we'll want to use fakelibvirt.

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.

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


+1000 to removing the need to have libvirt installed to run unit tests,
but that's what I'm seeing today unless I'm mistaken since we require
libvirt-python which requires libvirt as already pointed out.

If you revert the change to require libvirt-python and try to run the
unit tests, it fails, see bug 1357437 [1].

[1] https://bugs.launchpad.net/nova/+bug/1357437

Reverting the change to require libvirt-python is insufficient. That
revert will flip back to using system packages and include libvirt
python lib from your operating system. Libvirt will still be required
just via a different avenue (nova does try to fall back on its fake
libvirt but iirc that doesn't always work so well).

If you want to stop depending on libvirt in unittests you should remove
the libvirt-python requirement and not try to use the system package
installed version of libvirt. Then the fake libvirt will have to keep
working and will be tested in the gate.

Clark

_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Yeah I'm not suggesting we revert that change to test-requirements, I'm more agreeing with the sentiment to use fakelibvirt only and make that thing work (and keep it current).

--

Thanks,

Matt Riedemann


_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to