[libvirt] [PATCH python] Improve quality of sanitytest check
From: "Daniel P. Berrange" <berrange@redhat.com> Validate that every public API method is mapped into the python and that every python method has a sane C API. Signed-off-by: Daniel P. Berrange <berrange@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 + +# Path to the libvirt API XML file +xml = sys.argv[2] + +f = open(xml, "r") +tree = lxml.etree.parse(f) + +verbose = False + +wantenums = [] +wantfunctions = [] + +# 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) + + +# 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 + +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 + + +# Phase 3: First cut at mapping of C APIs to python classes + methods +basicklassmap = {} + +for cname in wantfunctions: + name = cname + # Some virConnect APIs have stupid names + if name[0:7] == "virNode" and name[0:13] != "virNodeDevice": + name = "virConnect" + name[7:] + if name[0:7] == "virConn" and name[0:10] != "virConnect": + name = "virConnect" + name[7:] + + # The typed param APIs are only for internal use + if name[0:14] == "virTypedParams": + continue + + # These aren't functions, they're callback signatures + if name in ["virConnectAuthCallbackPtr", "virConnectCloseFunc", + "virStreamSinkFunc", "virStreamSourceFunc", "virStreamEventCallback", + "virEventHandleCallback", "virEventTimeoutCallback", "virFreeCallback"]: + continue + if name[0:21] == "virConnectDomainEvent" and name[-8:] == "Callback": + continue + + + # virEvent APIs go into main 'libvirt' namespace not any class + if name[0:8] == "virEvent": + if name[-4:] == "Func": + continue + basicklassmap[name] = ["libvirt", name, cname] + else: + found = False + # To start with map APIs to classes based on the + # naming prefix. Mistakes will be fixed in next + # loop + for klassname in gottypes: + klen = len(klassname) + if name[0:klen] == klassname: + found = True + if name not in basicklassmap: + basicklassmap[name] = [klassname, name[klen:], cname] + elif len(basicklassmap[name]) < klassname: + basicklassmap[name] = [klassname, name[klen:], cname] + + # Anything which can't map to a class goes into the + # global namespaces + if not found: + basicklassmap[name] = ["libvirt", name[3:], cname] + + +# Phase 4: Deal with oh so many special cases in C -> python mapping +finalklassmap = {} + +for name in sorted(basicklassmap): + klass = basicklassmap[name][0] + func = basicklassmap[name][1] + cname = basicklassmap[name][2] + + # The object lifecycle APIs are irrelevant since they're + # used inside the object constructors/destructors. + if func in ["Ref", "Free", "New", "GetConnect", "GetDomain"]: + if klass == "virStream" and func == "New": + klass = "virConnect" + func = "NewStream" + else: + continue + + + # All the error handling methods need special handling + if klass == "libvirt": + if func in ["CopyLastError", "DefaultErrorFunc", + "ErrorFunc", "FreeError", + "SaveLastError", "ResetError"]: + continue + elif func in ["GetLastError", "GetLastErrorMessage", "ResetLastError", "Initialize"]: + func = "vir" + func + elif func == "SetErrorFunc": + func = "RegisterErrorHandler" + elif klass == "virConnect": + if func in ["CopyLastError", "SetErrorFunc"]: + continue + elif func in ["GetLastError", "ResetLastError"]: + func = "virConn" + func + + # Remove 'Get' prefix from most APIs, except those in virConnect + # and virDomainSnapshot namespaces which stupidly used a different + # convention which we now can't fix without breaking API + if func[0:3] == "Get" and klass not in ["virConnect", "virDomainSnapshot", "libvirt"]: + if func not in ["GetCPUStats"]: + func = func[3:] + + # The object creation and lookup APIs all have to get re-mapped + # into the parent class + if func in ["CreateXML", "CreateLinux", "CreateXMLWithFiles", + "DefineXML", "CreateXMLFrom", "LookupByUUID", + "LookupByUUIDString", "LookupByVolume" "LookupByName", + "LookupByID", "LookupByName", "LookupByKey", "LookupByPath", + "LookupByMACString", "LookupByUsage", "LookupByVolume", + "LookupSCSIHostByWWN", "Restore", "RestoreFlags", + "SaveImageDefineXML", "SaveImageGetXMLDesc"]: + if klass != "virDomain": + func = klass[3:] + func + + if klass == "virDomainSnapshot": + klass = "virDomain" + func = func[6:] + elif klass == "virStorageVol" and func in ["StorageVolCreateXMLFrom", "StorageVolCreateXML"]: + klass = "virStoragePool" + func = func[10:] + elif func == "StoragePoolLookupByVolume": + klass = "virStorageVol" + elif func == "StorageVolLookupByName": + klass = "virStoragePool" + else: + klass = "virConnect" + + # The open methods get remapped to primary namespace + if klass == "virConnect" and func in ["Open", "OpenAuth", "OpenReadOnly"]: + klass = "libvirt" + + # These are inexplicably renamed in the python API + if func == "ListDomains": + func = "ListDomainsID" + elif func == "ListAllNodeDevices": + func = "ListAllDevices" + elif func == "ListNodeDevices": + func = "ListDevices" + + # The virInterfaceChangeXXXX APIs go into virConnect. Stupidly + # they have lost their 'interface' prefix in names, but we can't + # fix this name + if func[0:6] == "Change": + klass = "virConnect" + + # Need to special case the snapshot APIs + if klass == "virDomainSnapshot" and func in ["Current", "ListNames", "Num"]: + klass = "virDomain" + func = "snapshot" + func + + # Names should stsart with lowercase letter... + func = string.lower(func[0:1]) + func[1:] + if func[0:8] == "nWFilter": + func = "nwfilter" + func[8:] + + # ...except when they don't. More stupid naming + # decisions we can't fix + if func == "iD": + func = "ID" + if func == "uUID": + func = "UUID" + if func == "uUIDString": + func = "UUIDString" + if func == "oSType": + func = "OSType" + if func == "xMLDesc": + func = "XMLDesc" + if func == "mACString": + func = "MACString" + + finalklassmap[name] = [klass, func, cname] + + +# Phase 5: Validate sure that every C API is mapped to a python API +fail = False +usedfunctions = {} +for name in sorted(finalklassmap): + klass = finalklassmap[name][0] + func = finalklassmap[name][1] + + if func in gotfunctions[klass]: + usedfunctions["%s.%s" % (klass, func)] = 1 + if verbose: + print "PASS %s -> %s.%s" % (name, klass, func) + else: + print "FAIL %s -> %s.%s (C API not mapped to python)" % (name, klass, func) + fail = True + + +# Phase 6: Validate that every python API has a corresponding C API +for klass in gotfunctions: + if klass == "libvirtError": + continue + for func in sorted(gotfunctions[klass]): + # These are pure python methods with no C APi + if func in ["connect", "getConnect", "domain", "getDomain"]: + continue + + key = "%s.%s" % (klass, func) + if not key in usedfunctions: + print "FAIL %s.%s (Python API not mapped to C)" % (klass, func) + fail = True + else: + if verbose: + print "PASS %s.%s" % (klass, func) + +# Phase 7: Validate that all the low level C APIs have binding +for name in sorted(finalklassmap): + cname = finalklassmap[name][2] + + pyname = cname + if pyname == "virSetErrorFunc": + pyname = "virRegisterErrorHandler" + elif pyname == "virConnectListDomains": + pyname = "virConnectListDomainsID" + + # These exist in C and exist in python, but we've got + # a pure-python impl so don't check them + if name in ["virStreamRecvAll", "virStreamSendAll"]: + continue + + try: + thing = getattr(libvirtmod, pyname) + except AttributeError: + print "FAIL libvirtmod.%s (C binding does not exist)" % pyname + fail = True -globals = dir(libvirt) - -# Sanity test that the generator hasn't gone wrong - -# Look for core classes -for clsname in ["virConnect", - "virDomain", - "virDomainSnapshot", - "virInterface", - "virNWFilter", - "virNodeDevice", - "virNetwork", - "virSecret", - "virStoragePool", - "virStorageVol", - "virStream", - ]: - assert(clsname in globals) - assert(object in getattr(libvirt, clsname).__bases__) - -# Constants -assert("VIR_CONNECT_RO" in globals) - -# Error related bits -assert("libvirtError" in globals) -assert("VIR_ERR_AUTH_FAILED" in globals) -assert("virGetLastError" in globals) - -# Some misc methods -assert("virInitialize" in globals) -assert("virEventAddHandle" in globals) -assert("virEventRegisterDefaultImpl" in globals) +if fail: + sys.exit(1) +else: + sys.exit(0) diff --git a/setup.py b/setup.py index 17b4722..bf222f8 100755 --- a/setup.py +++ b/setup.py @@ -59,6 +59,20 @@ def get_pkgconfig_data(args, mod, required=True): return line +def get_api_xml_files(): + """Check with pkg-config that libvirt is present and extract + the API XML file paths we need from it""" + + libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt") + + offset = libvirt_api.index("-api.xml") + libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml" + + offset = libvirt_api.index("-api.xml") + libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml" + + return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api) + ldflags = get_pkgconfig_data(["--libs-only-L"], "libvirt", False) cflags = get_pkgconfig_data(["--cflags"], "libvirt", False) @@ -105,23 +119,8 @@ if have_libvirt_lxc: class my_build(build): - def get_api_xml_files(self): - """Check with pkg-config that libvirt is present and extract - the API XML file paths we need from it""" - - libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt") - - offset = libvirt_api.index("-api.xml") - libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml" - - offset = libvirt_api.index("-api.xml") - libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml" - - return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api) - - def run(self): - apis = self.get_api_xml_files() + apis = get_api_xml_files() self.spawn(["python", "generator.py", "libvirt", apis[0]]) self.spawn(["python", "generator.py", "libvirt-qemu", apis[1]]) @@ -266,7 +265,9 @@ class my_test(Command): Run test suite """ - self.spawn(["python", "sanitytest.py", self.build_platlib]) + apis = get_api_xml_files() + + self.spawn(["python", "sanitytest.py", self.build_platlib, apis[0]]) class my_clean(clean): -- 1.8.3.1
On Tue, Nov 26, 2013 at 12:32 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
From: "Daniel P. Berrange" <berrange@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@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.
+ +# Path to the libvirt API XML file +xml = sys.argv[2] + +f = open(xml, "r") +tree = lxml.etree.parse(f) + +verbose = False + +wantenums = [] +wantfunctions = [] + +# 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 + + +# Phase 3: First cut at mapping of C APIs to python classes + methods +basicklassmap = {} + +for cname in wantfunctions: + name = cname + # Some virConnect APIs have stupid names + if name[0:7] == "virNode" and name[0:13] != "virNodeDevice": + name = "virConnect" + name[7:] + if name[0:7] == "virConn" and name[0:10] != "virConnect": + name = "virConnect" + name[7:] + + # The typed param APIs are only for internal use + if name[0:14] == "virTypedParams": + continue + + # These aren't functions, they're callback signatures + if name in ["virConnectAuthCallbackPtr", "virConnectCloseFunc", + "virStreamSinkFunc", "virStreamSourceFunc", "virStreamEventCallback", + "virEventHandleCallback", "virEventTimeoutCallback", "virFreeCallback"]: + continue + if name[0:21] == "virConnectDomainEvent" and name[-8:] == "Callback": + continue + + + # virEvent APIs go into main 'libvirt' namespace not any class + if name[0:8] == "virEvent": + if name[-4:] == "Func": + continue + basicklassmap[name] = ["libvirt", name, cname] + else: + found = False + # To start with map APIs to classes based on the + # naming prefix. Mistakes will be fixed in next + # loop + for klassname in gottypes: + klen = len(klassname) + if name[0:klen] == klassname: + found = True + if name not in basicklassmap: + basicklassmap[name] = [klassname, name[klen:], cname] + elif len(basicklassmap[name]) < klassname: + basicklassmap[name] = [klassname, name[klen:], cname] + + # Anything which can't map to a class goes into the + # global namespaces + if not found: + basicklassmap[name] = ["libvirt", name[3:], cname] + + +# Phase 4: Deal with oh so many special cases in C -> python mapping +finalklassmap = {} + +for name in sorted(basicklassmap): + klass = basicklassmap[name][0] + func = basicklassmap[name][1] + cname = basicklassmap[name][2] + + # The object lifecycle APIs are irrelevant since they're + # used inside the object constructors/destructors. + if func in ["Ref", "Free", "New", "GetConnect", "GetDomain"]: + if klass == "virStream" and func == "New": + klass = "virConnect" + func = "NewStream" + else: + continue + + + # All the error handling methods need special handling + if klass == "libvirt": + if func in ["CopyLastError", "DefaultErrorFunc", + "ErrorFunc", "FreeError", + "SaveLastError", "ResetError"]: + continue + elif func in ["GetLastError", "GetLastErrorMessage", "ResetLastError", "Initialize"]: + func = "vir" + func + elif func == "SetErrorFunc": + func = "RegisterErrorHandler" + elif klass == "virConnect": + if func in ["CopyLastError", "SetErrorFunc"]: + continue + elif func in ["GetLastError", "ResetLastError"]: + func = "virConn" + func + + # Remove 'Get' prefix from most APIs, except those in virConnect + # and virDomainSnapshot namespaces which stupidly used a different + # convention which we now can't fix without breaking API + if func[0:3] == "Get" and klass not in ["virConnect", "virDomainSnapshot", "libvirt"]: + if func not in ["GetCPUStats"]: + func = func[3:] + + # The object creation and lookup APIs all have to get re-mapped + # into the parent class + if func in ["CreateXML", "CreateLinux", "CreateXMLWithFiles", + "DefineXML", "CreateXMLFrom", "LookupByUUID", + "LookupByUUIDString", "LookupByVolume" "LookupByName", + "LookupByID", "LookupByName", "LookupByKey", "LookupByPath", + "LookupByMACString", "LookupByUsage", "LookupByVolume", + "LookupSCSIHostByWWN", "Restore", "RestoreFlags", + "SaveImageDefineXML", "SaveImageGetXMLDesc"]: + if klass != "virDomain": + func = klass[3:] + func + + if klass == "virDomainSnapshot": + klass = "virDomain" + func = func[6:] + elif klass == "virStorageVol" and func in ["StorageVolCreateXMLFrom", "StorageVolCreateXML"]: + klass = "virStoragePool" + func = func[10:] + elif func == "StoragePoolLookupByVolume": + klass = "virStorageVol" + elif func == "StorageVolLookupByName": + klass = "virStoragePool" + else: + klass = "virConnect" + + # The open methods get remapped to primary namespace + if klass == "virConnect" and func in ["Open", "OpenAuth", "OpenReadOnly"]: + klass = "libvirt" + + # These are inexplicably renamed in the python API + if func == "ListDomains": + func = "ListDomainsID" + elif func == "ListAllNodeDevices": + func = "ListAllDevices" + elif func == "ListNodeDevices": + func = "ListDevices" + + # The virInterfaceChangeXXXX APIs go into virConnect. Stupidly + # they have lost their 'interface' prefix in names, but we can't + # fix this name + if func[0:6] == "Change": + klass = "virConnect" + + # Need to special case the snapshot APIs + if klass == "virDomainSnapshot" and func in ["Current", "ListNames", "Num"]: + klass = "virDomain" + func = "snapshot" + func + + # Names should stsart with lowercase letter... + func = string.lower(func[0:1]) + func[1:] + if func[0:8] == "nWFilter": + func = "nwfilter" + func[8:] + + # ...except when they don't. More stupid naming + # decisions we can't fix + if func == "iD": + func = "ID" + if func == "uUID": + func = "UUID" + if func == "uUIDString": + func = "UUIDString" + if func == "oSType": + func = "OSType" + if func == "xMLDesc": + func = "XMLDesc" + if func == "mACString": + func = "MACString" + + finalklassmap[name] = [klass, func, cname] + + +# Phase 5: Validate sure that every C API is mapped to a python API +fail = False +usedfunctions = {} +for name in sorted(finalklassmap): + klass = finalklassmap[name][0] + func = finalklassmap[name][1] + + if func in gotfunctions[klass]: + usedfunctions["%s.%s" % (klass, func)] = 1 + if verbose: + print "PASS %s -> %s.%s" % (name, klass, func) + else: + print "FAIL %s -> %s.%s (C API not mapped to python)" % (name, klass, func) + fail = True + + +# Phase 6: Validate that every python API has a corresponding C API +for klass in gotfunctions: + if klass == "libvirtError": + continue + for func in sorted(gotfunctions[klass]): + # These are pure python methods with no C APi + if func in ["connect", "getConnect", "domain", "getDomain"]: + continue + + key = "%s.%s" % (klass, func) + if not key in usedfunctions: + print "FAIL %s.%s (Python API not mapped to C)" % (klass, func) + fail = True + else: + if verbose: + print "PASS %s.%s" % (klass, func) + +# Phase 7: Validate that all the low level C APIs have binding +for name in sorted(finalklassmap): + cname = finalklassmap[name][2] + + pyname = cname + if pyname == "virSetErrorFunc": + pyname = "virRegisterErrorHandler" + elif pyname == "virConnectListDomains": + pyname = "virConnectListDomainsID" + + # These exist in C and exist in python, but we've got + # a pure-python impl so don't check them + if name in ["virStreamRecvAll", "virStreamSendAll"]: + continue + + try: + thing = getattr(libvirtmod, pyname) + except AttributeError: + print "FAIL libvirtmod.%s (C binding does not exist)" % pyname + fail = True
-globals = dir(libvirt) - -# Sanity test that the generator hasn't gone wrong - -# Look for core classes -for clsname in ["virConnect", - "virDomain", - "virDomainSnapshot", - "virInterface", - "virNWFilter", - "virNodeDevice", - "virNetwork", - "virSecret", - "virStoragePool", - "virStorageVol", - "virStream", - ]: - assert(clsname in globals) - assert(object in getattr(libvirt, clsname).__bases__) - -# Constants -assert("VIR_CONNECT_RO" in globals) - -# Error related bits -assert("libvirtError" in globals) -assert("VIR_ERR_AUTH_FAILED" in globals) -assert("virGetLastError" in globals) - -# Some misc methods -assert("virInitialize" in globals) -assert("virEventAddHandle" in globals) -assert("virEventRegisterDefaultImpl" in globals) +if fail: + sys.exit(1) +else: + sys.exit(0) diff --git a/setup.py b/setup.py index 17b4722..bf222f8 100755 --- a/setup.py +++ b/setup.py @@ -59,6 +59,20 @@ def get_pkgconfig_data(args, mod, required=True):
return line
+def get_api_xml_files(): + """Check with pkg-config that libvirt is present and extract + the API XML file paths we need from it""" + + libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt") + + offset = libvirt_api.index("-api.xml") + libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml" + + offset = libvirt_api.index("-api.xml") + libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml" + + return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api) + ldflags = get_pkgconfig_data(["--libs-only-L"], "libvirt", False) cflags = get_pkgconfig_data(["--cflags"], "libvirt", False)
@@ -105,23 +119,8 @@ if have_libvirt_lxc:
class my_build(build):
- def get_api_xml_files(self): - """Check with pkg-config that libvirt is present and extract - the API XML file paths we need from it""" - - libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt") - - offset = libvirt_api.index("-api.xml") - libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml" - - offset = libvirt_api.index("-api.xml") - libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml" - - return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api) - - def run(self): - apis = self.get_api_xml_files() + apis = get_api_xml_files()
self.spawn(["python", "generator.py", "libvirt", apis[0]]) self.spawn(["python", "generator.py", "libvirt-qemu", apis[1]]) @@ -266,7 +265,9 @@ class my_test(Command): Run test suite """
- self.spawn(["python", "sanitytest.py", self.build_platlib]) + apis = get_api_xml_files() + + self.spawn(["python", "sanitytest.py", self.build_platlib, apis[0]])
class my_clean(clean): -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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. -- Doug Goldstein
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@redhat.com> wrote:
From: "Daniel P. Berrange" <berrange@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@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 :|
On Nov 27, 2013, at 6:19 AM, "Daniel P. Berrange" <berrange@redhat.com> wrote:
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@redhat.com> wrote:
From: "Daniel P. Berrange" <berrange@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@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
Can we drop the explicit lxml import since we are only using etree? Then we can try lxml.etree and xml.etree as fallbacks. Do we need lxml added to the spec file for building as well?
+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.
Works for me. This is a giant leap forward for the tests.
+ +# 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.
Overall ACK with the fix for Cygwin. It's a great improvement. The lxml import might be nice too. Sorry I don't have a specific example as I'm tapping this out on a phone in the car. -- Doug
On Wed, Nov 27, 2013 at 10:28:08AM -0500, Doug Goldstein wrote:
On Nov 27, 2013, at 6:19 AM, "Daniel P. Berrange" <berrange@redhat.com> wrote:
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@redhat.com> wrote:
From: "Daniel P. Berrange" <berrange@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@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
Can we drop the explicit lxml import since we are only using etree? Then we can try lxml.etree and xml.etree as fallbacks. Do we need lxml added to the spec file for building as well?
The 'xpath' function is lxml only - the regular etree module only has 'find' which doesn't do proper xpath queries.
+# 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) +
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 :|
participants (3)
-
Daniel P. Berrange -
Doug Goldstein -
Doug Goldstein