Re: [Libvirt-cim] [PATCH] [TEST] #2 Fix potiential false positive in AC 01

>> +def enum_pools_and_ac(ip, virt, cn): >> + pools = {} >> + ac = [] >> + >> + pt = ['MemoryPool', 'ProcessorPool', 'DiskPool', 'NetworkPool'] >> + >> + try: >> + key = ["InstanceID"] >> + ac = enumclass.enumerate(ip, cn, key, virt) >> + >> + for p in pt: >> + enum_list = enumclass.enumerate(ip, p, key, virt) >> + >> + if len(enum_list) < 1: >> + logger.error("%s did not return any instances" % p) >> + return pools, ac + >> + for pool in enum_list: >> + pools[pool.InstanceID] = pool + >> + except Exception, details: >> + logger.error(CIM_ERROR_ENUMERATE, cn) >> + logger.error(details) >> + return pools, ac + >> + if len(ac) != len(pools): >> + logger.error("%s returned %s instances, expected %s" % (cn, >> len(ac), + len(pools))) >> + return pools, ac + >> enum_pools_and_ac(options.ip, options.virt, cn) >> + if len(pools) < 1: >> + cleanup_restore(options.ip, options.virt) >> + destroy_netpool(options.ip, options.virt, test_network) >> return FAIL > I think the check used in the prev patch, len(ac) != len(pools) is > better This check is still here - it's included in the enum_pools_and_ac() function. See the piece quoted above. It can be moved outside of this function to help with clarity. and also we should be checking len(ac) != 4 to make sure that AC > values == enum of MemoryPool + ProcessorPool + DiskPool + > NetworkPool. Any specific reason for not doing this ? > otherwise +1 for me. You can't guarantee 4 pools. The test creates a disk pool and a network pool, but it's possible for additional net and disk pools to exist on the system. That's why you want to check len(ac) != len(pools).