From: Sam Betts <sam@code-smash.net>
Date: Tue, 5 Aug 2014 23:05:05 +0000 (+0100)
Subject: Predictable field and filter ordering
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f103ce48ca60e12952be7aa6d5183f64ef826370;p=openstack-build%2Fneutron-build.git

Predictable field and filter ordering

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
---

diff --git a/neutron/tests/base.py b/neutron/tests/base.py
index 99a91affa..7cce573fd 100644
--- a/neutron/tests/base.py
+++ b/neutron/tests/base.py
@@ -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
diff --git a/neutron/tests/unit/test_api_v2.py b/neutron/tests/unit/test_api_v2.py
index 827514938..1cc50d38c 100644
--- a/neutron/tests/unit/test_api_v2.py
+++ b/neutron/tests/unit/test_api_v2.py
@@ -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'