diff --git a/HACKING.rst b/HACKING.rst index 03e2d7ade5..3ec9a23712 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -54,6 +54,9 @@ Nova Specific Commandments - [N341] contextlib.nested is deprecated - [N342] Config options should be in the central location ``nova/conf/`` - [N343] Check for common double word typos +- [N344] Python 3: do not use dict.iteritems. +- [N345] Python 3: do not use dict.iterkeys. +- [N346] Python 3: do not use dict.itervalues. Creating Unit Tests ------------------- diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index 856eac91da..79ed1b676a 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -591,6 +591,27 @@ def check_doubled_words(physical_line, filename): return (0, msg % {'word': match.group(1)}) +def check_python3_no_iteritems(logical_line): + msg = ("N344: Use six.iteritems() instead of dict.iteritems().") + + if re.search(r".*\.iteritems\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_iterkeys(logical_line): + msg = ("N345: Use six.iterkeys() instead of dict.iterkeys().") + + if re.search(r".*\.iterkeys\(\)", logical_line): + yield(0, msg) + + +def check_python3_no_itervalues(logical_line): + msg = ("N346: Use six.itervalues() instead of dict.itervalues().") + + if re.search(r".*\.itervalues\(\)", logical_line): + yield(0, msg) + + def factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) @@ -621,3 +642,6 @@ def factory(register): register(check_greenthread_spawns) register(check_config_option_in_central_place) register(check_doubled_words) + register(check_python3_no_iteritems) + register(check_python3_no_iterkeys) + register(check_python3_no_itervalues) diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 78b6818c63..06e0dc530d 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -1520,11 +1520,8 @@ class ModelsObjectComparatorMixin(object): def _dict_from_object(self, obj, ignored_keys): if ignored_keys is None: ignored_keys = [] - if isinstance(obj, dict): - obj_items = obj.items() - else: - obj_items = obj.iteritems() - return {k: v for k, v in obj_items + + return {k: v for k, v in obj.items() if k not in ignored_keys} def _assertEqualObjects(self, obj1, obj2, ignored_keys=None): @@ -7385,7 +7382,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): service_data['host'] = 'host2' service = db.service_create(self.ctxt, service_data) - existing_node = dict(self.item.iteritems()) + existing_node = dict(self.item.items()) expected = [existing_node] for name in ['bm_node1', 'bm_node2']: diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index a3040e55af..8093e677a5 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -639,3 +639,24 @@ class HackingTestCase(test.NoDBTestCase): code = "This is the then best comment" self._assert_has_no_errors(code, checks.check_doubled_words) + + def test_dict_iteritems(self): + self.assertEqual(1, len(list(checks.check_python3_no_iteritems( + "obj.iteritems()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iteritems( + "six.iteritems(ob))")))) + + def test_dict_iterkeys(self): + self.assertEqual(1, len(list(checks.check_python3_no_iterkeys( + "obj.iterkeys()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_iterkeys( + "six.iterkeys(ob))")))) + + def test_dict_itervalues(self): + self.assertEqual(1, len(list(checks.check_python3_no_itervalues( + "obj.itervalues()")))) + + self.assertEqual(0, len(list(checks.check_python3_no_itervalues( + "six.itervalues(ob))"))))