Re: [Libvirt-cim] [PATCH] [TEST][Resubmitting: Addition] : Adding 01_forward.py tc to verify ReferencedProfile
Hi Dan,
Please see my comments inline.
Thanks and Regards,
Deepti.
Dan Smith wrote:
DK> # HG changeset patch
DK> # User Deepti B. Kalakeri <deeptik(a)linux.vnet.ibm.com>
DK> # Date 1207117030 -19800
DK> # Node ID 57b84aa20438fe7f8b7f2fa5e0098e30e2198745
DK> # Parent e2cd9e869996152255adcf54e4c4e38331fcf760
DK> [TEST][Resubmitting: Addition] : Adding 01_forward.py tc to verify
ReferencedProfile.
This one ran fine when I tried it, but I have some comments.
DK> + try:
DK> + key_list = ["InstanceID"]
DK> + proflist = enumclass.enumerate(server, eval('enumclass.' +
reg_classname), key_list, virt)
Doesn't enumclass.enumerate take a classname? We should avoid
eval()'ing all over the place for no reason.
yes it does, I have changed this.
DK> + if len(proflist) < 5 :
This will break when we add one more profile. Please make sure this
is >0 and then check that the proper profiles are returned somewhere
else.
Well, if we add a new profile later then the len(proflist) will be 6 and
the condition (6<5) will still hold good.
Also, by explicitly verifying the len to be 5 makes sure that we dont
get anything less than the previously implemented profiles.
DK> + profiles_instid_list = []
DK> + for profile in proflist:
DK> + if not ("DSP1042" in profile.InstanceID):
DK> + profiles_instid_list.append(profile.InstanceID)
This is randomly filtering out DSP1042? Why?
Changed this.
DK> +def verify_ref_assoc_info(assoc_info, sys_prof_info):
DK> + if assoc_info['InstanceID'] != sys_prof_info['InstanceID']:
DK> + print_field_error('InstanceID', assoc_info['InstanceID'],
sys_prof_info['InstanceID'])
DK> + return FAIL
DK> + if assoc_info['RegisteredOrganization'] !=
sys_prof_info['RegisteredOrganization']:
DK> + print_field_error('RegisteredOrganization',
assoc_info['RegisteredOrganization'],
DK> +
sys_prof_info['RegisteredOrganization'])
DK> + return FAIL
DK> + if assoc_info['RegisteredName'] !=
sys_prof_info['RegisteredName']:
DK> + print_field_error('RegisteredName',
assoc_info['RegisteredName'],
DK> +
sys_prof_info['RegisteredName'])
DK> + return FAIL
DK> + if assoc_info['RegisteredVersion'] !=
sys_prof_info['RegisteredVersion']:
DK> + print_field_error('RegisteredVersion',
assoc_info['RegisteredVersion'],
DK> +
sys_prof_info['RegisteredVersion'])
DK> + return FAIL
DK> + return PASS
The entire blob above can be replaced with this:
for f in ["RegisteredOrganization", "RegisteredName",
"RegisteredVersion"]:
if assoc_info[f] != sys_prof_info[f]:
print_field_error(f, assoc_info[f], sys_prof_info[f])
return FAIL
return PASS
This is better way, thanks I used this :).
Thanks!
------------------------------------------------------------------------
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim