[Libvirt-cim] [PATCH] [TEST][Resubmitting: Addition] : Adding 01_forward.py tc to verify ReferencedProfile

[Deepti B Kalakeri]

Hi Dan,

Please see my comments inline.

Thanks and Regards,
Deepti.

Dan Smith wrote:
> DK> # HG changeset patch
> DK> # User Deepti B. Kalakeri <deeptik at 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim