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

[Kaitlin Rupert]

> +    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 at linux.vnet.ibm.com