[Libvirt-cim] [PATCH 1 of 2] [TEST] Created testcases for the forward HostedAccessPoint association (HostSystem -> KVMRedirectionSAP

[Richard Maciel]

Kaitlin Rupert wrote:
> Accidentally hit send before finishing my review..
> 
>> +def get_kvmrsap_inst(virt, ip, guest_name):
>> +    kvmrsap_inst = None +
>> +    try:
>> +        kvmrsap_cn = get_typed_class(virt, 'KVMRedirectionSAP')
>> +        enum_list = EnumInstances(ip, kvmrsap_cn)
>> +
>> +        for kvmrsap in enum_list:
>> +            if kvmrsap.SystemName == guest_name:
>> +                if kvmrsap_inst is not None:
>> +                    print "More than one KVMRedirectionSAP found \
>> +                           the same guest"
> 
> Instead of using print, elogger.error() should be used.
> 
>> +                    return kvmrsap_inst, FAIL
> 
> Since you also return the inst and FAIL in the except block, you could 
> remove the line above and change the print to a "raise Exception()" call.
> 
> Either one is valid, though I think raising an exception is cleaner.
> 
>> +                kvmrsap_inst = kvmrsap
>> +
>> +    except Exception, details:
>> +        logger.error(details)
>> +        return kvmrsap_inst, FAIL
>> +
>> +    return kvmrsap_inst, PASS
>> +
>> +def verify_kvmrsap(enum_list, kvmrsap_inst): +    status = FAIL
>> +
>> +    for item in enum_list:
> 
> You should get get only one KVMRedirectionSAP.  Verify enum_list only 
> has one element.  If it has more than one, return an error.
> 
> Remove the for loop - since there's only one element in the list, no 
> need to use a loop here.

I don't get it. When I execute the ServiceAccessBySAP query 
(ConsoleRedirectionService -> KVMRedirectionSAP) I receive one item per 
virtual machine configured in the host. The same happens when I do the 
query using pywbem.

> 
>> +        if item.classname != kvmrsap_inst.Classname:
>> +            print "Wrong returned class name (%s)", item.classname
> 
> This needs to be a logger.error() call instead of a print.
> 
>> +            return status
>> +
>> +        if compare_all_prop(item, kvmrsap_inst) == PASS:
>> +            if status == PASS:
>> +                print "More than one instance found"
> 
> Replace the print with logger.error()
> 


-- 
Richard Maciel, MSc
IBM Linux Technology Center
rmaciel at linux.vnet.ibm.com