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

[Deepti B Kalakeri]

Hi Dan,

Please see my replies inline.

Thanks and Regards,
Deepti.

Dan Smith wrote:
> DK> +def verify_fields(assoc_info, prof_info):
> DK> +    if assoc_info['InstanceID'] != prof_info['InstanceID']:
> DK> +        print_field_error('InstanceID', assoc_info['InstanceID'], prof_info['InstanceID']) 
> DK> +        return FAIL
> DK> +    if assoc_info['RegisteredOrganization'] != prof_info['RegisteredOrganization']:
> DK> +        print_field_error('RegisteredOrganization', assoc_info['RegisteredOrganization'], \
> DK> +                                                    prof_info['RegisteredOrganization']) 
> DK> +        return FAIL
> DK> +    if assoc_info['RegisteredName'] != prof_info['RegisteredName']:
> DK> +        print_field_error('RegisteredName', assoc_info['RegisteredName'], \
> DK> +                                           prof_info['RegisteredName']) 
> DK> +        return FAIL
> DK> +    if assoc_info['RegisteredVersion'] != prof_info['RegisteredVersion']:
> DK> +        print_field_error('RegisteredVersion', assoc_info['RegisteredVersion'], \
> DK> +                                               prof_info['RegisteredVersion']) 
> DK> +        return FAIL
> DK> +    return PASS
>
> Same comment as before for this as well.
>   
Changed this.
> DK> +def verify_ref_assoc_info(assoc_info):
> DK> +    status = PASS
> DK> +    vs_prof, gen_dev_prof, mem_res_prof, vs_mig_prof = init_list() 
> DK> +    for i in range(len(assoc_info)):
>
> Please use the following syntax for for loops (unless you really need
> the index):
>
>   for i in assoc_info:
>   
Done.
> DK> +        instid = assoc_info[i]['InstanceID']
> DK> +        if instid.find("DSP1045") >=0 :
> DK> +            status = verify_fields(assoc_info[i], mem_res_prof) 
> DK> +        elif instid.find("DSP1057") >=0 :
> DK> +            status = verify_fields(assoc_info[i], vs_prof)
> DK> +        elif instid.find("DSP1059") >=0 :
> DK> +            status = verify_fields(assoc_info[i], gen_dev_prof)
> DK> +        elif instid.find("DSP1081") >=0 :
> DK> +            status = verify_fields(assoc_info[i], vs_mig_prof)
> DK> +        else:
> DK> +            status = FAIL
> DK> +        if status != PASS:
> DK> +            break
> DK> +    return status
>
> In the interest of making this more extensible in the future, why not
> embed the information you need in the profile information dictionary
> in some way to avoid this sort of static structure?  Assuming the dict
> has a "profnum" field:
>
>   
Done.
>   profiles = init_list()
>   for i in assoc_info:
>       status = None
>       for profile in profiles:
>           if profile["profnum"] in i['InstanceID']:
>               status = verify_fields(i, profile)
>               break
>       if status == FAIL:
>           break
>
> You may need to rearrange your profile dict a little to make the key
> properties distinct from other bits, but the result will be that when
> we add a new supported profile, you can just add an entry to the
> top-level list of profiles and not have to modify lots of other bits
> in the code.
>
> Most of your other tests can probably be modified in this way as well,
> which leads me to ask:  Why not define a single set of profiles in a
> common file and import them?  That way, all of the profile tests will
> work against a single set of profiles, which I think makes a lot more
> sense.
>   
Done.
>       
> DK> +def get_refprof_verify_info():
> DK> +    assoc_info = []
> DK> +    status = PASS
> DK> +    assoc_name =  get_typed_class(virt, 'ReferencedProfile')
> DK> +    instid = "CIM:DSP1042-SystemVirtualization-1.0.0"
> DK> +    try:
> DK> +        assoc_info = Associators(server, assoc_name, reg_classname, virt, InstanceID = instid, 
> DK> +                                                            CreationClassName = reg_classname)
> DK> +        if len(assoc_info) < 4:
> DK> +            logger.error("%s returned %i %s objects, expected atleast 4",
> DK> +                                     assoc_name, len(assoc_info), 'Profile')
> DK> +            status = FAIL
> DK> +
> DK> +        if status != PASS:
> DK> +            return status 
> DK> +        status = verify_ref_assoc_info(assoc_info)
> DK> +    except Exception, detail:
> DK> +        logger.error(CIM_ERROR_ASSOCIATORS, assoc_name)
> DK> +        logger.error("Exception: %s", detail)
> DK> +        status = FAIL
> DK> +    return status
>
> Perhaps you can explain the need for the repeated special case
> distinction for this profile?
>
>   
Merged in this with the other tc.
> ------------------------------------------------------------------------
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim