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

[Kaitlin Rupert]

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.

> +        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()

-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com