[libvirt] [PATCH python] Improve quality of sanitytest check

[Daniel P. Berrange]

On Tue, Nov 26, 2013 at 01:16:24PM -0600, Doug Goldstein wrote:
> On Tue, Nov 26, 2013 at 12:32 PM, Daniel P. Berrange
> <berrange at redhat.com> wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> >
> > Validate that every public API method is mapped into the python
> > and that every python method has a sane C API.
> 
> Looks like we had the same idea and even a similar approach as well.
> 
> >
> > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > ---
> >  sanitytest.py | 309 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >  setup.py      |  35 +++----
> >  2 files changed, 294 insertions(+), 50 deletions(-)
> >  mode change 100755 => 100644 sanitytest.py
> >
> > diff --git a/sanitytest.py b/sanitytest.py
> > old mode 100755
> > new mode 100644
> > index 517054b..9e4c261
> > --- a/sanitytest.py
> > +++ b/sanitytest.py
> > @@ -1,40 +1,283 @@
> >  #!/usr/bin/python
> >
> >  import sys
> > +import lxml
> > +import lxml.etree
> > +import string
> >
> > +# Munge import path to insert build location for libvirt mod
> >  sys.path.insert(0, sys.argv[1])
> > -
> >  import libvirt
> > +import libvirtmod
> 
> I wouldn't directly import libvirtmod due to Cygwin. I'd just use
> libvirt.libvirtmod which is what its available as.

Ah, good point.

> > +# Phase 1: Identify all functions and enums in public API
> > +set = tree.xpath('/api/files/file/exports[@type="function"]/@symbol')
> > +for n in set:
> > +    wantfunctions.append(n)
> > +
> > +set = tree.xpath('/api/files/file/exports[@type="enum"]/@symbol')
> > +for n in set:
> > +    wantenums.append(n)
> > +
> 
> Maybe its a bit ugly but I actually grabbed the typedef's as well to
> check the various namespaces (e.g. virConnect, virDomain) but not sure
> if we want that.

I used the method names themselves to detect this. Could perhaps do
both to have double the sanity test, but this can wait for now.

> 
> > +
> > +# Phase 2: Identify all classes and methods in the 'libvirt' python module
> > +gotenums = []
> > +gottypes = []
> > +gotfunctions = { "libvirt": [] }
> > +
> > +for name in dir(libvirt):
> > +    if name[0] == '_':
> > +        continue
> > +    thing = getattr(libvirt, name)
> > +    if type(thing) == int:
> > +        gotenums.append(name)
> > +    elif type(thing) == type:
> > +        gottypes.append(name)
> > +        gotfunctions[name] = []
> > +    elif callable(thing):
> > +        gotfunctions["libvirt"].append(name)
> > +    else:
> > +       pass
> 
> Could the body of this be made into a function reused below?

Well the two loops are not really the same, so don't think
it would save much code.

> 
> > +
> > +for klassname in gottypes:
> > +    klassobj = getattr(libvirt, klassname)
> > +    for name in dir(klassobj):
> > +        if name[0] == '_':
> > +            continue
> > +        thing = getattr(klassobj, name)
> > +        if callable(thing):
> > +            gotfunctions[klassname].append(name)
> > +        else:
> > +            pass

> Just some visual comments until I get a chance to really play with
> this. I stopped at the fixup area, which in my code is equally painful
> as well. You're obviously a bit more knowledgable about Python than I
> am because your fixups are a bit cleaner.

With all the fixes I've sent so far, I'm finally able to run this sanity
test against builds of the python done against historical versions, which
means we're getting alot better at compat.


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 :|