[Libvirt-cim] [PATCH] [TEST] Add revision branch for VSMS.02_destroysystem

[Zhengang Li]

Jay Gagnon wrote:

> zli at linux.vnet.ibm.com wrote:
>> # HG changeset patch
>> # User Zhengang Li <lizg at cn.ibm.com>
>> # Date 1209391870 -28800
>> # Node ID 17873ab6228cc92c6cd5e92a41b5a19ed312792c
>> # Parent  5ed6cd841f7b65eeea4af7af845c4ebfc2da4cf1
>> [TEST] Add revision branch for VSMS.02_destroysystem
>>
>> Signed-off-by: Zhengang Li <lizg at cn.ibm.com>
>>
>> diff -r 5ed6cd841f7b -r 17873ab6228c 
>> suites/libvirt-cim/cimtest/VirtualSystemManagementService/02_destroysystem.py 
>>
>> --- 
>> a/suites/libvirt-cim/cimtest/VirtualSystemManagementService/02_destroysystem.py    
>> Mon Apr 28 17:00:36 2008 +0800
>> +++ 
>> b/suites/libvirt-cim/cimtest/VirtualSystemManagementService/02_destroysystem.py    
>> Mon Apr 28 22:11:10 2008 +0800
>> @@ -25,15 +25,17 @@ import pywbem
>>  import pywbem
>>  from pywbem.cim_obj import CIMInstanceName
>>  from VirtLib import utils
>> -from VirtLib.live import domain_list
>> +from VirtLib.live import domain_list, active_domain_list
>>  from XenKvmLib import vsms, vxml
>>  from XenKvmLib.classes import get_typed_class
>> +from XenKvmLib.const import CIM_REV
>>  from CimTest.Globals import do_main
>>  from CimTest.Globals import logger
>>  from CimTest.ReturnCodes import PASS, FAIL
>>
>>  sup_types = ['Xen', 'KVM', 'XenFV']
>>  default_dom = 'test_domain'
>> +rev = 528
>>
>>  @do_main(sup_types)
>>  def main():
>> @@ -48,8 +50,8 @@ def main():
>>      cs_ref = CIMInstanceName(classname, keybindings = {
>>                                          'Name':default_dom,
>>                                          'CreationClassName':classname})
>> -    -    list_before = domain_list(options.ip, options.virt)
>> +    dl_func = CIM_REV < rev and active_domain_list or domain_list
>> +    list_before = dl_func(options.ip, options.virt)
>>   
> The idea here is that if CIM_REV is less than rev we use 
> active_domain_list, otherwise we use domain_list, right?  If I 
> understand that correctly, while I appreciate the cleverness, I think that
> 
> if CIM_REV < rev:
>    dl_func = active_domain_list
> else:
>    dl_func = domain_list
Let's use this. New patches for VSMS and RASD are on the way.

> 
> is probably preferable.  It achieves the same result of not typing the 
> arg list more than once but is significantly more obvious with regards 
> to what's going on.  Reducing line count is great, but I think here lost 
> clarity is too costly.  If you really want to keep it to the one line 
> like that, I would say at least throw in some parentheses.
> 
> 


-- 
- Zhengang