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

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.

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:

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:

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.

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?

------------------------------------------------------------------------ _______________________________________________ Libvirt-cim mailing list Libvirt-cim(a)redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim