> 
> On Nov 27, 2013, at 6:19 AM, "Daniel P. Berrange" <berra...@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
>> <berra...@redhat.com> wrote:
>>> From: "Daniel P. Berrange" <berra...@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 <berra...@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to