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