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

[Dan Smith]

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?

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms at us.ibm.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20080402/7f169846/attachment.sig>