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

[Jay Gagnon]

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

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.


-- 

-Jay