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

On Tue, Nov 26, 2013 at 12:32 PM, Daniel P. Berrange <berrange(a)redhat.com&gt; wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > 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(a)redhat.com&gt; > --- > 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.

> +# 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.

> + > +# 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?

> + > +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.