Merge lp:~jtv/maas/for-underscore into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Standardise on “_” for unused loop variables; it's what we use in most places so we might as well have a bit more consistency.

This is another Doris branch. Python doesn't let you write “for” loops without a variable, so we have a lot of loops with single-letter variable names where the variable isn't used, and about twice as many with just an underscore as the unused variable name.

I'm not looking to enforce a rule or anything, but I think we'll get a more harmonious style without the existing mix of ‘i’ and ‘x’ variables.

Most of this is simple mechanical replacement, but there was one case where I used ‘zip’ instead of array indexing; and one case in a test where I replaced a loop to assert equality on list elements with a single equality assertion on the lists. It's a bit fragile for cases where the return value might become a tuple or some other listlike-but-not-actually-list container. But, the name of the test asserts that it's a list so the test might as well fail if it's not. The ordering of the test assertion may seem odd, but that's because the variable called “expected” actually contains the return value that's being checked. I have no idea whatsoever why this was done; the surrounding tests do the same thing.

Jeroen

[Julian Edwards (julian-edwards)]

 review: approve

review: Approve

=== modified file 'src/maasserver/api/tests/test_nodes.py'
--- src/maasserver/api/tests/test_nodes.py	2014-09-09 07:21:43 +0000
+++ src/maasserver/api/tests/test_nodes.py	2014-09-13 12:40:38 +0000
@@ -450,7 +450,7 @@
         # filter returned results.
         current_token = get_auth_tokens(self.logged_in_user)[0]
         nodes = []
-        for i in range(3):
+        for _ in range(3):
             nodes.append(factory.make_Node(
                 status=NODE_STATUS.ALLOCATED,
                 owner=self.logged_in_user, token=current_token))

=== modified file 'src/maasserver/models/tests/test_components.py'
--- src/maasserver/models/tests/test_components.py	2014-08-13 21:49:35 +0000
+++ src/maasserver/models/tests/test_components.py	2014-09-13 12:40:38 +0000
@@ -70,7 +70,7 @@
 
     def get_persistent_errors_returns_text_for_error_codes(self):
         errors, components = [], []
-        for i in range(3):
+        for _ in range(3):
             error_message = factory.make_string()
             component = get_random_component()
             register_persistent_error(component, error_message)

=== modified file 'src/maasserver/models/tests/test_dhcplease.py'
--- src/maasserver/models/tests/test_dhcplease.py	2014-09-05 16:38:32 +0000
+++ src/maasserver/models/tests/test_dhcplease.py	2014-09-13 12:40:38 +0000
@@ -174,7 +174,7 @@
     def test_get_hostname_ip_mapping_returns_mapping(self):
         nodegroup = factory.make_NodeGroup()
         expected_mapping = {}
-        for i in range(3):
+        for _ in range(3):
             status = random.choice(
                 [NODE_STATUS.DEPLOYED, NODE_STATUS.DEPLOYING])
             node = factory.make_Node(
@@ -190,7 +190,7 @@
     def test_get_hostname_ip_mapping_ignores_non_deployed_nodes(self):
         nodegroup = factory.make_NodeGroup()
         # Create non-allocated nodes with leases.
-        for i in range(10):
+        for _ in range(10):
             status = factory.pick_choice(
                 NODE_STATUS_CHOICES,
                 but_not=[NODE_STATUS.DEPLOYED, NODE_STATUS.DEPLOYING])

=== modified file 'src/maasserver/models/tests/test_managers.py'
--- src/maasserver/models/tests/test_managers.py	2013-10-18 16:57:37 +0000
+++ src/maasserver/models/tests/test_managers.py	2014-09-13 12:40:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test maasserver model managers."""
@@ -28,9 +28,9 @@
 
     def test_manager_iterator_uses_cache(self):
         parents = set()
-        for i in range(3):
+        for _ in range(3):
             parents.add(BulkManagerParentTestModel.objects.create())
-        for i in range(10):
+        for _ in range(10):
             for parent in parents:
                 BulkManagerTestModel.objects.create(parent=parent)
         parents = BulkManagerParentTestModel.objects.all().prefetch_related(

=== modified file 'src/maasserver/models/tests/test_staticipaddress.py'
--- src/maasserver/models/tests/test_staticipaddress.py	2014-09-05 16:38:32 +0000
+++ src/maasserver/models/tests/test_staticipaddress.py	2014-09-13 12:40:38 +0000
@@ -235,7 +235,7 @@
     def test_get_hostname_ip_mapping_returns_mapping(self):
         nodegroup = factory.make_NodeGroup()
         expected_mapping = {}
-        for i in range(3):
+        for _ in range(3):
             node = factory.make_node_with_mac_attached_to_nodegroupinterface(
                 nodegroup=nodegroup)
             staticip = factory.make_StaticIPAddress(mac=node.get_primary_mac())

=== modified file 'src/maasserver/models/tests/test_userprofile.py'
--- src/maasserver/models/tests/test_userprofile.py	2014-09-05 16:38:32 +0000
+++ src/maasserver/models/tests/test_userprofile.py	2014-09-13 12:40:38 +0000
@@ -82,7 +82,7 @@
         profile = factory.make_User().get_profile()
         token_ids = []
         consumer_ids = []
-        for i in range(3):
+        for _ in range(3):
             token, consumer = profile.create_authorisation_token()
             token_ids.append(token.id)
             consumer_ids.append(consumer.id)
@@ -102,7 +102,7 @@
         self.assertEqual(users, all_users)
 
     def test_manager_all_users_no_system_user(self):
-        for i in range(3):
+        for _ in range(3):
             factory.make_User()
         usernames = set(
             user.username for user in UserProfile.objects.all_users())

=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py	2014-09-11 16:17:15 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py	2014-09-13 12:40:38 +0000
@@ -1601,7 +1601,7 @@
     @transactional
     def create_cluster_and_interfaces(self):
         cluster = factory.make_NodeGroup()
-        for i in range(3):
+        for _ in range(3):
             factory.make_NodeGroupInterface(cluster)
         interfaces = [
             {

=== modified file 'src/maasserver/tests/test_dhcp.py'
--- src/maasserver/tests/test_dhcp.py	2014-09-05 23:01:31 +0000
+++ src/maasserver/tests/test_dhcp.py	2014-09-13 12:40:38 +0000
@@ -569,11 +569,11 @@
         # nodegroups get their DHCP config re-written.
         num_active_nodegroups = random.randint(1, 10)
         num_inactive_nodegroups = random.randint(1, 10)
-        for x in range(num_active_nodegroups):
+        for _ in range(num_active_nodegroups):
             factory.make_NodeGroup(
                 status=NODEGROUP_STATUS.ACCEPTED,
                 management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
-        for x in range(num_inactive_nodegroups):
+        for _ in range(num_inactive_nodegroups):
             factory.make_NodeGroup(
                 status=NODEGROUP_STATUS.PENDING,
                 management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)

=== modified file 'src/maasserver/views/tests/test_nodes.py'
--- src/maasserver/views/tests/test_nodes.py	2014-09-11 18:30:39 +0000
+++ src/maasserver/views/tests/test_nodes.py	2014-09-13 12:40:38 +0000
@@ -463,9 +463,8 @@
         macs = [
             factory.make_MACAddress(node=node).mac_address for i in range(2)]
         ips = [factory.getRandomIPAddress() for i in range(2)]
-        for i in range(2):
-            factory.make_DHCPLease(
-                nodegroup=nodegroup, mac=macs[i], ip=ips[i])
+        for mac, ip in zip(macs, ips):
+            factory.make_DHCPLease(nodegroup=nodegroup, mac=mac, ip=ip)
         node_link = reverse('node-view', args=[node.system_id])
         response = self.client.get(node_link)
         self.assertThat(response.content, ContainsAll(ips))

=== modified file 'src/maasserver/views/tests/test_prefs.py'
--- src/maasserver/views/tests/test_prefs.py	2014-09-05 16:38:32 +0000
+++ src/maasserver/views/tests/test_prefs.py	2014-09-13 12:40:38 +0000
@@ -54,7 +54,7 @@
         self.client_log_in()
         user = self.logged_in_user
         # Create a few tokens.
-        for i in range(3):
+        for _ in range(3):
             user.get_profile().create_authorisation_token()
         response = self.client.get('/account/prefs/')
         doc = fromstring(response.content)

=== modified file 'src/provisioningserver/drivers/hardware/tests/test_virsh.py'
--- src/provisioningserver/drivers/hardware/tests/test_virsh.py	2014-09-11 21:08:12 +0000
+++ src/provisioningserver/drivers/hardware/tests/test_virsh.py	2014-09-13 12:40:38 +0000
@@ -173,8 +173,7 @@
         output = SAMPLE_IFLIST % (macs[0], macs[1])
         conn = self.configure_virshssh(output)
         expected = conn.get_mac_addresses('')
-        for i in range(2):
-            self.assertEqual(macs[i], expected[i])
+        self.assertEqual(macs, expected)
 
     def test_get_arch_returns_valid(self):
         arch = factory.make_name('arch')

=== modified file 'src/provisioningserver/testing/tests/test_bindfixture.py'
--- src/provisioningserver/testing/tests/test_bindfixture.py	2014-01-05 21:27:58 +0000
+++ src/provisioningserver/testing/tests/test_bindfixture.py	2014-09-13 12:40:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2012, 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the BIND fixture."""
@@ -133,7 +133,7 @@
     def test_defaults_reallocated_after_teardown(self):
         seen_homedirs = set()
         resources = BINDServerResources()
-        for i in range(2):
+        for _ in range(2):
             with resources:
                 self.assertTrue(os.path.exists(resources.homedir))
                 self.assertNotIn(resources.homedir, seen_homedirs)