attrs_exist() misleadingly checks expected_attrs

Bug #1285964 reported by Michael Yu
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack DBaaS (Trove)
Fix Released
Low
mariam john

Bug Description

https://github.com/openstack/trove/blob/7fc263fc3fd4d2f11a1456ba162c854aad14d00e/trove/tests/util/check.py#L114-L118

The name "expected_attrs" is misleading. If you look at the code, it's more like "allowed_attrs".

For example:
list = [type]
expected_attrs = [type, version]

There are no failures. But based on the name "expected_attrs"... I am expecting that "list" has all of the attributes in "expected_attrs".

The proposal for this bug is:

1) Add a more strict function that actually does check for all of the expected attributes in list. Something like this:

def contains_expected_attrs(self, list, expected_attrs, msg=None):
        for attr in expected_attrs:
            if attr not in list:
                self.fail("%s should contain '%s'" % (msg, attr))

2) Refactor attrs_exist() to contains_allowed_attrs(self, list, allowed_attrs, msg=None) + all areas that used it

Changed in trove:
importance: Undecided → Low
status: New → Triaged
mariam john (mariamj)
Changed in trove:
assignee: nobody → mariam john (mariamj)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to trove (master)

Fix proposed to branch: master
Review: https://review.openstack.org/121018

Changed in trove:
status: Triaged → In Progress
Changed in trove:
milestone: none → juno-rc1
Revision history for this message
Nikhil Manchanda (slicknik) wrote :

This issue is not release critical to hold juno-rc1 for. Moving to kilo-1.

Changed in trove:
milestone: juno-rc1 → kilo-1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to trove (master)

Reviewed: https://review.openstack.org/121018
Committed: https://git.openstack.org/cgit/openstack/trove/commit/?id=3864816fd6071f54ccbabd3e3837ee147b996161
Submitter: Jenkins
Branch: master

commit 3864816fd6071f54ccbabd3e3837ee147b996161
Author: mariam john <email address hidden>
Date: Sat Sep 6 23:54:01 2014 -0500

    Rename attrs_exist() to contains_allowed_attrs()

    attrs_exist() checks the attributes in the given list against the
    attributes in the expected_attrs list. The name expected_attrs is
    misleading since based on the name one would expect the given list
    to contain all the attributes in the expected list but thats not
    what this function does. Based on this, the following changes were
    made:

    - renamed attr_exist() to contains_allowed_attrs() which checks
      whether all the attributes in the given list is present in the
      allowed_attrs list.

    Change-Id: Ie169271e211aa93c1bd3f64a2723aa1cd03a56e9
    Closes-Bug: #1285964

Changed in trove:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in trove:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in trove:
milestone: kilo-1 → 2015.1.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.