[PATCH] Adding a testcase to test the invalid inputs to the KVMRedirectionSAP class

# HG changeset patch # User Veerendra C <vechandr@in.ibm.com> # Date 1228732224 28800 # Node ID 3f098b127064b5cc08b6a5ae1c197b0457a69048 # Parent 701f3228bdfe740f4a504dce1dfab844c812b9d5 Adding a testcase to test the invalid inputs to the KVMRedirectionSAP class diff -r 701f3228bdfe -r 3f098b127064 suites/libvirt-cim/cimtest/RedirectionService/03_RedirectionSAP_errs.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/suites/libvirt-cim/cimtest/RedirectionService/03_RedirectionSAP_errs.py Mon Dec 08 02:30:24 2008 -0800 @@ -0,0 +1,124 @@ +#!/usr/bin/python +################################################################################ +# Copyright 2008 IBM Corp. +# +# Authors: +# Veerendra C <vechandr@in.ibm.com> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +################################################################################ +# Example : +# wbemcli gi 'http://root:passwd@localhost:5988/root/virt:KVM_KVMRedirectionSAP. +# CreationClassName="KVM_KVMRedirectionSAP",Name="1:1",SystemCreationClassName= +# "KVM_ComputerSystem",SystemName="demo"' -nl +# +# Test Case Info: +# -------------- +# This testcase is used to verify if appropriate exceptions are +# returned by KVMRedirectionSAP on giving invalid inputs for keyvalue's. +# +################################################################################ + +import sys +import pywbem +from XenKvmLib import assoc +from XenKvmLib.common_util import try_getinstance +from XenKvmLib.classes import get_typed_class +from XenKvmLib import vxml +from XenKvmLib.test_doms import destroy_and_undefine_all +from CimTest.ReturnCodes import PASS, FAIL, SKIP +from CimTest.Globals import logger, CIM_USER, CIM_PASS, CIM_NS +from XenKvmLib.const import do_main, get_provider_version + +test_dom = "demo" +test_vcpus = 1 +sup_types = ['Xen', 'XenFV', 'KVM', 'LXC'] + +@do_main(sup_types) +def main(): + options = main.options + server = main.options.ip + libvirtcim_hr_crs_changes = 688 + status = FAIL + + # This check is required for libivirt-cim providers which do not have + # CRS changes in it and the CRS provider is available with revision >= 688. + curr_cim_rev, changeset = get_provider_version(options.virt, options.ip) + if (curr_cim_rev < libvirtcim_hr_crs_changes): + logger.info("ConsoleRedirectionService provider not supported, " + "hence skipping the test ....") + return SKIP + + name = "%s:%s" % ("1", "1") + status = PASS + + # Getting the VS list and deleting the test_dom if it already exists. + cxml = vxml.get_class(options.virt)(test_dom, vcpus=test_vcpus) + ret = cxml.cim_define(options.ip) + + if not ret : + logger.error("ERROR: VS '%s' is not defined", test_dom) + return status + + conn = assoc.myWBEMConnection( 'http://%s' % options.ip, + (CIM_USER, CIM_PASS), CIM_NS) + + classname = get_typed_class(options.virt, 'KVMRedirectionSAP') + + key_vals = { + 'SystemName': test_dom, + 'CreationClassName': classname, + 'SystemCreationClassName': get_typed_class(options.virt, + 'ComputerSystem'), + 'Name': name + } + + 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_nameport" : {'rc' : pywbem.CIM_ERR_FAILED, + 'desc' : " Unable to determine console port for guest" }, + "invalid_sysval" : {'rc' : pywbem.CIM_ERR_NOT_FOUND, + 'desc' : "No such instance" } + } + + tc_scen = { + 'invalid_ccname' : 'CreationClassName', + 'invalid_sccname' : 'SystemCreationClassName', + 'invalid_nameport' : 'Name', + 'invalid_sysval' : 'SystemName', + } + + # Looping by passing invalid key values + for field, test_val in tc_scen.items(): + newkey_vals = key_vals.copy() + newkey_vals[test_val] = field + ret_value = try_getinstance(conn, classname, newkey_vals, + field_name=test_val, + expr_values = expr_values[field], + bug_no = "") + if ret_value != PASS: + logger.error(" -------------- FAILED %s ----------- : " % field) + break + + cxml.destroy(options.ip) + cxml.undefine(options.ip) + return status + +if __name__ == "__main__": + sys.exit(main()) +

+ status = FAIL + + # This check is required for libivirt-cim providers which do not have + # CRS changes in it and the CRS provider is available with revision >= 688. + curr_cim_rev, changeset = get_provider_version(options.virt, options.ip) + if (curr_cim_rev < libvirtcim_hr_crs_changes): + logger.info("ConsoleRedirectionService provider not supported, " + "hence skipping the test ....") + return SKIP + + name = "%s:%s" % ("1", "1") + status = PASS
Remove this line - status is set to FAIL above. It's better to set status to FAIL because we should only set it to PASS in cases when we're sure a PASS condition has been met.
+ + # Looping by passing invalid key values + for field, test_val in tc_scen.items(): + newkey_vals = key_vals.copy() + newkey_vals[test_val] = field + ret_value = try_getinstance(conn, classname, newkey_vals, + field_name=test_val, + expr_values = expr_values[field], + bug_no = "") + if ret_value != PASS: + logger.error(" -------------- FAILED %s ----------- : " % field) + break
You capture the return of try_getinstance() as ret_value, but at the end of the test, you are returning status. If ret_value is FAIL, but somewhere earlier in the test, status is set to PASS, then you could potientially return a false positive. I would change ret_value to status here.
+ + cxml.destroy(options.ip)
This should be cim_destroy(). Thanks for making all these changes! I think the flow of the test is much easier to read this time around. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com

Kaitlin Rupert wrote: >> + status = FAIL I am changing the status = PASS here >> >> + >> + # This check is required for libivirt-cim providers which do not have >> + # CRS changes in it and the CRS provider is available with revision >> >= 688. >> + curr_cim_rev, changeset = get_provider_version(options.virt, >> options.ip) >> + if (curr_cim_rev < libvirtcim_hr_crs_changes): >> + logger.info("ConsoleRedirectionService provider not supported, " >> + "hence skipping the test ....") >> + return SKIP >> + >> + name = "%s:%s" % ("1", "1") >> + status = PASS > > Remove this line - status is set to FAIL above. It's better to set > status to FAIL because we should only set it to PASS in cases when > we're sure a PASS condition has been met. > Now, I have removed the line status = PASS . >> + >> + # Looping by passing invalid key values + for field, test_val in >> tc_scen.items(): >> + newkey_vals = key_vals.copy() >> + newkey_vals[test_val] = field >> + ret_value = try_getinstance(conn, classname, newkey_vals, >> + field_name=test_val, + expr_values = expr_values[field], + bug_no = >> "") >> + if ret_value != PASS: >> + logger.error(" -------------- FAILED %s ----------- : " % field) >> + break > > You capture the return of try_getinstance() as ret_value, but at the > end of the test, you are returning status. If ret_value is FAIL, but > somewhere earlier in the test, status is set to PASS, then you could > potientially return a false positive. > > I would change ret_value to status here. > Thanks for finding this, now I have changed the status = ret_value . >> + >> + cxml.destroy(options.ip) > > This should be cim_destroy(). > Also changed this to cim_destroy(). > > Thanks for making all these changes! I think the flow of the test is > much easier to read this time around. Earlier I had based my testcase on an existing testcase in the cimtest. Hence quite a few changes required I will submit the testcase freshly. Thanks Kaitlin for the review . Please accept it. Thanks Veerendra C

Thanks for making all these changes! I think the flow of the test is much easier to read this time around.
Earlier I had based my testcase on an existing testcase in the cimtest. Hence quite a few changes required I will submit the testcase freshly. Thanks Kaitlin for the review . Please accept it.
Unfortunately, most of the negative test cases are older and difficult to read. These tests need to be updated, but it's a low priority at the moment. Instead, I'm trying to improve the readability of the newer tests so we won't have to update the new ones as often. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (3)
-
Kaitlin Rupert
-
veeren@linux.vnet.ibm.com
-
Veerendra