[Libvirt-cim] [patch][testcase] Invalid testscenario for KVMRedirectionSAP class.

[Kaitlin Rupert]

Veerendra wrote:
> Hi
> 
> Please find a patch for the invalid scenario on KVMRedirectionSAP class.
> This patch also contains modifications for a library to include the 
> "graphics" definition for the KVM guests.
> 
> Regards
> Veerendra C

Can you send your patch using hg's email feature?  That makes it easier 
to review.

 ># HG changeset patch
 ># User Veerendra C <vechandr at in.ibm.com>
 ># Date 1228287169 28800
 ># Node ID 2b2bb6f1b2a9e560322608d837244677a7a75c45
 ># Parent  70ecd9e8867cc277b31f4703e73b5dba7ad53128
 >Adding a testcase to test the invalid inputs to the KVMRedirectionSAP 
 >class
 >Also included the defintion of graphics for the class KVMXML under 
 >devices.

Need to add DCO to the patch itself.

 >+# wbemcli gi 
'http://root:passwd@localhost:5988/root/virt:KVM_KVMRedirectionSAP. \
 >+# 
CreationClassName="KVM_KVMRedirectionSAP",Name="1:1",SystemCreationClassName= 
  \

These lines are longer than 80 characters.

 >+SHAREMODE = 3
 >+REDIRECTION_SER_TYPE = 3
 >+CRS_MAX_SAP_REV = 724

These aren't used in this test.

 >+sup_types = ['KVM']

This should also run on Xen, XenFV, and LXC.

 >+expr_values = {
 >+    "invalid_ccname"         : {'rc'   : pywbem.CIM_ERR_NOT_FOUND,
 >+                                'desc' : "No such instance 
 >(CreationClassName)" },
 >+    "invalid_sccname"        : {'rc'   : pywbem.CIM_ERR_NOT_FOUND,
 >+                                'desc' : "No such instance 
 >(SystemCreationClassName)" },
 >+    "invalid_name"           : {'rc'   : pywbem.CIM_ERR_NOT_FOUND,
 >+                                'desc' : "No such instance (Name)" },
 >+    "invalid_nameport"       : {'rc'   : pywbem.CIM_ERR_FAILED,
 >+                                'desc' : " Unable to determine 
console >port for guest" },
 >+    "invalid_sysname"        : {'rc'   : pywbem.CIM_ERR_NOT_FOUND,
 >+                                'desc' : "No such instance 
 >(SystemName)" },
 >+    "invalid_sysval"        : {'rc'   : pywbem.CIM_ERR_NOT_FOUND,
 >+                                'desc' : "No such instance" }
 >+              }

Some of these lines are longer than 80 characters.  These return 
messages only work with Pegasus.  This test fails with sfcb.

 >+    vsxml.define(options.ip)

This should be cim_define(), not define().

+    ret = vsxml.start(options.ip)
+    if not ret:
+        logger.error("Failed to Create the dom: %s", test_dom)
+        return FAIL
 >+    global conn

Don't use global variables.

 >+    conn = assoc.myWBEMConnection('http://%s' % options.ip, 
(CIM_USER, CIM_PASS), CIM_NS)

This line is longer than 80 characters.

 >+    global name_val
 >+    global classname

Remove the global here too.

+    classname = get_typed_class(options.virt, 'KVMRedirectionSAP')
+    name_val = [
+               'CreationClassName',       classname,
 >+               'SystemCreationClassName', 
get_typed_class(options.virt, 'ComputerSystem'),

This line is longer than 80 characters.

 >+                'INVALID_cc_Keyname'            : 'invalid_ccname',
 >+                'INVALID_scc_Keyname'           : 'invalid_sccname',
 >+                'INVALID_yname_Keyname'         : 'invalid_name',
 >+                'INVALID_zsysname_Keyname'      : 'invalid_sysname',

You don't need to test the invalid key names - this tests the CIMOM 
itself and not the providers.  The CIMOM's return their own error 
messages and return codes for invalid key names - we have no way of 
controlling these values or this behavior.

 >+
 >+    i = 0
 >+    for field1, field2 in sorted(tc_scen.items()):
 >+        retval = try_invalid_gi(i, field1, field2)
 >+        if retval != PASS:
 >+            status = retval
 >+        i = i + 1

This test is a little difficult to read.  The logic in try_invalid_gi() 
can be simplified if the test is restructured a bit.

I'd reorganize it with the following:

In main:

1) Define guest
2) Use a dictionary for the key values:

     key_vals = { 'SystemName': test_dom,
                  'CreationClassName': classname,
                  'SystemCreationClassName': sscn,
                  'Name': name
                }

3) Create a tc_scen dictionary - have the key of the dictionary be the 
same as the invalid value you'll set for key value (also have it be the 
same as the values used for the exp_values dictionary):

                   test_val         :  field

     tc_scen = {
                 'invalid_ccname'   : 'CreationClassName',
                 'invalid_sccname'  : 'SystemCreationClassName',
                 'invalid_nameport' : 'Name',
                 'invalid_sysval'   : 'SystemName',
               }

4) Loop through the items in tc_scen - call try_invalid_gi(test_val, 
field) each time through.
5) If try_invalid_gi() encounters an error, break from the for loop
6) Undefine the guest and return status

In try_invalid_gi():

1) Have this function take the values from tc_scen.  Also have it take 
key vals.
2) Copy key_vals to keys using .copy()
3) Set the value of keys appropriately:  keys[field] = test_val
4) Call try_getinstance(conn, cn, keys, test_val,
                                 expr_values[test_val], bug_no="")

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