]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Predictable field and filter ordering
authorSam Betts <sam@code-smash.net>
Tue, 5 Aug 2014 23:05:05 +0000 (00:05 +0100)
committerSam Betts <sam@code-smash.net>
Mon, 11 Aug 2014 16:33:24 +0000 (17:33 +0100)
This fixes the fields and filters units tests that break with a
randomized PYTHONHASHSEED (see the bug report).

The RESOURCE_ATTRIBUTE_MAP is stored as a dict leading to an
unpredictable output order. Values in kvp strings are being stored as
sets underpinned by dicts when converted, leading to unpredictable
ordering of values when read.

Discovered with PYTHONHASHSEED = 2455351445 on these tests:
test_api_v2.APIv2TestCase.test_fields
test_api_v2.APIv2TestCase.test_fields_multiple
test_api_v2.FiltersTestCase.test_attr_info_with_convert_list_to
test_api_v2.APIv2TestCase.test_filters_with_fields
test_api_v2.APIv2TestCase.test_fields_multiple_with_empty

There are 3 parts to this fix:
1. Update the APIv2TestCase _do_field_list function to construct
field list in the same order as the controller constructs its list.
2. Ensure the APIv2TestCase _get_collection_kwargs maintains order
throughout.
3. Use new assertOrderedEqual function to sort values before assertion
in test_attr_info_with_convert_list_to

Change-Id: I547cfa80cf83b0340b459279df9283443562326b
Partial-bug: #1348818

neutron/tests/base.py
neutron/tests/unit/test_api_v2.py

index 99a91affaf0a179e839342f91bfcff18512a8ba0..7cce573fdd8de06c23b8ddc4b53c3e39e866c5c7 100644 (file)
@@ -217,3 +217,16 @@ class BaseTestCase(testtools.TestCase):
             yield
             return
         self.fail('Execution of this test timed out')
+
+    def assertOrderedEqual(self, expected, actual):
+        expect_val = self.sort_dict_lists(expected)
+        actual_val = self.sort_dict_lists(actual)
+        self.assertEqual(expect_val, actual_val)
+
+    def sort_dict_lists(self, dic):
+        for key, value in dic.iteritems():
+            if isinstance(value, list):
+                dic[key] = sorted(value)
+            elif isinstance(value, dict):
+                dic[key] = self.sort_dict_lists(value)
+        return dic
index 8275149384b13906d8ec47150d6a592ce32fc1a6..1cc50d38cdb2ae0552dc2a2311d6f935133d5fa1 100644 (file)
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import collections
 import os
 
 import mock
@@ -132,8 +133,10 @@ class APIv2TestCase(APIv2TestBase):
     def _do_field_list(self, resource, base_fields):
         attr_info = attributes.RESOURCE_ATTRIBUTE_MAP[resource]
         policy_attrs = [name for (name, info) in attr_info.items()
-                        if info.get('required_by_policy') or
-                        info.get('primary_key')]
+                        if info.get('required_by_policy')]
+        for name, info in attr_info.items():
+            if info.get('primary_key'):
+                policy_attrs.append(name)
         fields = base_fields
         fields.extend(policy_attrs)
         return fields
@@ -141,8 +144,8 @@ class APIv2TestCase(APIv2TestBase):
     def _get_collection_kwargs(self, skipargs=[], **kwargs):
         args_list = ['filters', 'fields', 'sorts', 'limit', 'marker',
                      'page_reverse']
-        args_dict = dict((arg, mock.ANY)
-                         for arg in set(args_list) - set(skipargs))
+        args_dict = collections.OrderedDict(
+            (arg, mock.ANY) for arg in set(args_list) - set(skipargs))
         args_dict.update(kwargs)
         return args_dict
 
@@ -1522,7 +1525,7 @@ class FiltersTestCase(base.BaseTestCase):
         }
         expect_val = {'foo': {'key': ['2', '4']}, 'bar': ['3'], 'qux': ['1']}
         actual_val = api_common.get_filters(request, attr_info)
-        self.assertEqual(actual_val, expect_val)
+        self.assertOrderedEqual(expect_val, actual_val)
 
     def test_attr_info_with_convert_to(self):
         path = '/?foo=4&bar=3&baz=2&qux=1'