[Libvirt-cim] [PATCH] [TEST] Fix VirtualSystemManagementService/05_destroysystem_neg.py with provider's updates of error message

[Deepti B Kalakeri]

yunguol at cn.ibm.com wrote:
> # HG changeset patch
> # User Guolian Yun <yunguol at cn.ibm.com>
> # Date 1236912499 25200
> # Node ID 6e1a1d69588b34117d087a3f8c9e8dbc50618f9b
> # Parent  676a8b05baa09b69052d519c7b438b301bea849c
> [TEST] Fix VirtualSystemManagementService/05_destroysystem_neg.py with provider's updates of error message
>
>
>
> Tested for KVM with current sources and rpm
>
> Signed-off-by: Guolian Yun<yunguol at cn.ibm.com>
>
> diff -r 676a8b05baa0 -r 6e1a1d69588b suites/libvirt-cim/cimtest/VirtualSystemManagementService/05_destroysystem_neg.py
> --- a/suites/libvirt-cim/cimtest/VirtualSystemManagementService/05_destroysystem_neg.py	Tue Mar 10 22:27:59 2009 -0700
> +++ b/suites/libvirt-cim/cimtest/VirtualSystemManagementService/05_destroysystem_neg.py	Thu Mar 12 19:48:19 2009 -0700
> @@ -29,16 +29,18 @@
>   
Please put a small description at the beginning of the tc.
>  from XenKvmLib.classes import get_typed_class
>  from XenKvmLib.test_doms import undefine_test_domain
>   
Remove the above ununsed undefine_test_domain statement.
>  from CimTest.Globals import logger
> -from XenKvmLib.const import do_main
> +from XenKvmLib.const import do_main, get_provider_version
>  from CimTest.ReturnCodes import FAIL, PASS, SKIP
>   
remove the unused utils, import statement.
>  sup_types = ['Xen', 'KVM', 'XenFV', 'LXC']
>  vsms_status_version = 534
>   
Remove vsms_status_version variable as it not used
> +vsms_err_message = 814
>
>  def destroysystem_fail(tc, options):
>      service = vsms.get_vsms_class(options.virt)(options.ip)
>      
>      classname = get_typed_class(options.virt, 'ComputerSystem')
> +    curr_cim_rev, changeset = get_provider_version(options.virt, options.ip)
>
>      if tc == 'noname':
>          cs_ref = CIMInstanceName(classname, 
> @@ -47,6 +49,10 @@
>          exp_value = { 'rc'    : pywbem.CIM_ERR_FAILED,
>                        'desc'  : 'Unable to retrieve domain name.'
>                      }
> +        if curr_cim_rev >= vsms_err_message:
> +            exp_value = { 'rc'    : pywbem.CIM_ERR_NOT_FOUND,
> +                          'desc'  : 'Unable to retrieve domain name: Error 0'
> +                        }
>
>   
You are assigning the values to expr_value twice in case curr_cim_rev >= 
vsms_err_message.
You can instead use the following or something better:
if curr_cim_rev >= vsms_err_message:
exp_value = { 'rc' : pywbem.CIM_ERR_NOT_FOUND,
'desc' : 'Unable to retrieve domain name: Error 0'
}
else:
exp_value = { 'rc' : pywbem.CIM_ERR_FAILED,
'desc' : 'Unable to retrieve domain name.'
}


>      elif tc == 'nonexistent':
>          cs_ref = CIMInstanceName(classname,keybindings = {
> @@ -56,6 +62,10 @@
>          exp_value = { 'rc'   : pywbem.CIM_ERR_FAILED,
>                        'desc' : 'Failed to find domain' 
>                      }
> +        if curr_cim_rev >= vsms_err_message:
> +            exp_value = { 'rc'   : pywbem.CIM_ERR_NOT_FOUND,
> +                          'desc' : "Referenced domain `##@@!!cimtest_domain'" \
> +                                   " does not exist: Domain not found"}
>
>   
Same here.
>      else:
>          return SKIP
>   
There is no need for this SKIP here.
Do you have it for some specific reason ?

Also, at line 73 status = FAIL is not needed as it is not used.
> @@ -68,7 +78,7 @@
>          err_no   = details[0]
>          err_desc = details[1]
>          if err_no == exp_value['rc'] and err_desc.find(exp_value['desc']) >= 0:
> -            logger.error("For Invalid Scenario '%s'", tc)
> +            logger.info("For Invalid Scenario '%s'", tc)
>              logger.info('Got expected error no: %s', err_no)
>              logger.info('Got expected error desc: %s',err_desc)
>              return PASS
>   
Can you include something like this outside the if condition to report 
the mismatching exception which does not match the exception we intend 
to verify.
logger.error("Unknown exceptions %s", details)

Line no 86 - 88
logger.error('destroy_fail>> %s: Error executing DestroySystem', tc)
return FAIL
These lines can be outside the expect block. We need this outside in 
case if we dont get an exception.

> @@ -82,7 +92,7 @@
>      options = main.options
>      rc1 = destroysystem_fail('noname', options)
>      rc2 = destroysystem_fail('nonexistent', options)
> -
> +    
>      status = FAIL
>      if rc1 == PASS and rc2 == PASS:
>          status = PASS
>   
Instead use return PASS
Line no: 99 - 103
99 else:
100 rclist = [rc1, rc2]
101 rclist.sort()
102 if rclist[0] == PASS and rclist[1] == SKIP:
103 status = PASS

Why do we need to set status = PASS when rclist[1] == SKIP.
I think this was related to vsms_status_version branching and I believe 
we can remove this check now.

PS: All the line nos are the ones after your patches are applied.

> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
>   

-- 
Thanks and Regards,
Deepti B. Kalakeri
IBM Linux Technology Center
deeptik at linux.vnet.ibm.com