From c58320a4eaa6743e59b31ec90f9f4f38cb9e3b3c Mon Sep 17 00:00:00 2001 From: Kui Shi Date: Tue, 15 Oct 2013 00:35:58 +0800 Subject: [PATCH] align the order of parameters for urlencode() In Python 3.3, hash randomization is enabled by default. It causes the iteration order of dicts and sets to be unpredictable and differ across Python runs. In the test case, the fixed expecting string will not match the test result, it is relying on the dict order. This change transforms the input dict to a sequence of two-element list, with fixed order, and update the related expecitng string / fixture in test cases. Partial Implement: blueprint py33-support Change-Id: I6dccde9e584be8335a6375f5fbad5c5cbd7b9b6d --- ceilometerclient/common/utils.py | 2 +- ceilometerclient/tests/v2/test_alarms.py | 14 +++++++------- ceilometerclient/tests/v2/test_options.py | 6 +++--- ceilometerclient/tests/v2/test_resources.py | 4 ++-- ceilometerclient/tests/v2/test_samples.py | 2 +- ceilometerclient/tests/v2/test_statistics.py | 2 +- ceilometerclient/v2/options.py | 5 ++++- 7 files changed, 19 insertions(+), 16 deletions(-) diff --git a/ceilometerclient/common/utils.py b/ceilometerclient/common/utils.py index 3f2fa91..fe21378 100644 --- a/ceilometerclient/common/utils.py +++ b/ceilometerclient/common/utils.py @@ -68,7 +68,7 @@ def print_dict(d, dict_property="Property", wrap=0): pt = prettytable.PrettyTable([dict_property, 'Value'], caching=False, print_empty=False) pt.align = 'l' - for k, v in six.iteritems(d): + for k, v in sorted(six.iteritems(d)): # convert dict to str to check length if isinstance(v, dict): v = str(v) diff --git a/ceilometerclient/tests/v2/test_alarms.py b/ceilometerclient/tests/v2/test_alarms.py index eae8521..bc04714 100644 --- a/ceilometerclient/tests/v2/test_alarms.py +++ b/ceilometerclient/tests/v2/test_alarms.py @@ -187,8 +187,8 @@ fixtures = { ), }, - '/v2/alarms?q.op=&q.op=&q.value=project-id&q.value=SwiftObjectAlarm' - '&q.field=project_id&q.field=name': + '/v2/alarms?q.field=project_id&q.field=name&q.op=&q.op=' + '&q.value=project-id&q.value=SwiftObjectAlarm': { 'GET': ( {}, @@ -209,7 +209,7 @@ fixtures = { ALARM_HISTORY, ), }, - '/v2/alarms/alarm-id/history?q.op=&q.value=NOW&q.field=timestamp': + '/v2/alarms/alarm-id/history?q.field=timestamp&q.op=&q.value=NOW': { 'GET': ( {}, @@ -244,8 +244,8 @@ class AlarmManagerTest(testtools.TestCase): ])) expect = [ ('GET', - '/v2/alarms?q.op=&q.op=&q.value=project-id&q.value=' - 'SwiftObjectAlarm&q.field=project_id&q.field=name', + '/v2/alarms?q.field=project_id&q.field=name&q.op=&q.op=' + '&q.value=project-id&q.value=SwiftObjectAlarm', {}, None), ] self.assertEqual(self.api.calls, expect) @@ -334,8 +334,8 @@ class AlarmManagerTest(testtools.TestCase): def test_get_constrained_history(self): q = [dict(field='timestamp', value='NOW')] - url = ('/v2/alarms/alarm-id/history' - '?q.op=&q.value=NOW&q.field=timestamp') + url = ('/v2/alarms/alarm-id/history?q.field=timestamp' + '&q.op=&q.value=NOW') self._do_test_get_history(q, url) diff --git a/ceilometerclient/tests/v2/test_options.py b/ceilometerclient/tests/v2/test_options.py index 27f9020..b771ec6 100644 --- a/ceilometerclient/tests/v2/test_options.py +++ b/ceilometerclient/tests/v2/test_options.py @@ -21,7 +21,7 @@ class BuildUrlTest(utils.BaseTestCase): url = options.build_url('/', [{'field': 'this', 'op': 'gt', 'value': 43}]) - self.assertEqual(url, '/?q.op=gt&q.value=43&q.field=this') + self.assertEqual(url, '/?q.field=this&q.op=gt&q.value=43') def test_two(self): url = options.build_url('/', [{'field': 'this', @@ -33,12 +33,12 @@ class BuildUrlTest(utils.BaseTestCase): ops = 'q.op=gt&q.op=lt' vals = 'q.value=43&q.value=88' fields = 'q.field=this&q.field=that' - self.assertEqual(url, '/?%s&%s&%s' % (ops, vals, fields)) + self.assertEqual(url, '/?%s&%s&%s' % (fields, ops, vals)) def test_default_op(self): url = options.build_url('/', [{'field': 'this', 'value': 43}]) - self.assertEqual(url, '/?q.op=&q.value=43&q.field=this') + self.assertEqual(url, '/?q.field=this&q.op=&q.value=43') def test_one_param(self): url = options.build_url('/', None, ['period=60']) diff --git a/ceilometerclient/tests/v2/test_resources.py b/ceilometerclient/tests/v2/test_resources.py index 8c0632e..0ddccb0 100644 --- a/ceilometerclient/tests/v2/test_resources.py +++ b/ceilometerclient/tests/v2/test_resources.py @@ -37,7 +37,7 @@ fixtures = { ] ), }, - '/v2/resources?q.op=&q.value=a&q.field=resource_id': + '/v2/resources?q.field=resource_id&q.op=&q.value=a': { 'GET': ( {}, @@ -97,7 +97,7 @@ class ResourceManagerTest(utils.BaseTestCase): "value": "a"}, ])) expect = [ - ('GET', '/v2/resources?q.op=&q.value=a&q.field=resource_id', + ('GET', '/v2/resources?q.field=resource_id&q.op=&q.value=a', {}, None), ] self.assertEqual(self.api.calls, expect) diff --git a/ceilometerclient/tests/v2/test_samples.py b/ceilometerclient/tests/v2/test_samples.py index 32084c9..a6eff4f 100644 --- a/ceilometerclient/tests/v2/test_samples.py +++ b/ceilometerclient/tests/v2/test_samples.py @@ -35,7 +35,7 @@ del CREATE_SAMPLE['message_id'] del CREATE_SAMPLE['source'] base_url = '/v2/meters/instance' -args = 'q.op=&q.op=&q.value=foo&q.value=bar&q.field=resource_id&q.field=source' +args = 'q.field=resource_id&q.field=source&q.op=&q.op=&q.value=foo&q.value=bar' fixtures = { base_url: { diff --git a/ceilometerclient/tests/v2/test_statistics.py b/ceilometerclient/tests/v2/test_statistics.py index a233b23..ba3fe7a 100644 --- a/ceilometerclient/tests/v2/test_statistics.py +++ b/ceilometerclient/tests/v2/test_statistics.py @@ -17,7 +17,7 @@ from ceilometerclient.tests import utils import ceilometerclient.v2.statistics base_url = '/v2/meters/instance/statistics' -qry = 'q.op=&q.op=&q.value=foo&q.value=bar&q.field=resource_id&q.field=source' +qry = 'q.field=resource_id&q.field=source&q.op=&q.op=&q.value=foo&q.value=bar' period = '&period=60' samples = [{ u'count': 135, diff --git a/ceilometerclient/v2/options.py b/ceilometerclient/v2/options.py index b3a6f3e..349e04c 100644 --- a/ceilometerclient/v2/options.py +++ b/ceilometerclient/v2/options.py @@ -34,7 +34,10 @@ def build_url(path, q, params=None): for name in ['field', 'op', 'value']: query_params['q.%s' % name].append(query.get(name, '')) - path += "?" + urlutils.urlencode(query_params, doseq=True) + # Transform the dict to a sequence of two-element tuples in fixed + # order, then the encoded string will be consistent in Python 2&3. + new_qparams = sorted(query_params.items(), key=lambda x: x[0]) + path += "?" + urlutils.urlencode(new_qparams, doseq=True) if params: for p in params: