openstack/python-novaclient
Code Issues Proposed changes

Eliminate test comprehensions

Browse Source 
The novaclient tests are rife with examples of using a list comprehension
to perform a series of asserts, when a simple for loop would be more
appropriate and more idiomatic; part of the problem is the propensity
for people to cut and paste.  Eliminating the existing examples should
remove the temptation for developers to use this non-idiom to perform
tests.

Change-Id: I2cc8979e720740eae81692a60e30a3d95dce2a2c
This commit is contained in:
Kevin L. Mitchell 2015-05-21 19:12:35 -05:00
parent c5b850fb1e
commit 0a327ce375
10 changed files with 40 additions and 23 deletions

3
novaclient/tests/unit/v2/test_cloudpipe.py

@ -29,7 +29,8 @@ class CloudpipeTest(utils.FixturedTestCase):
def test_list_cloudpipes(self):
cp = self.cs.cloudpipe.list()
self.assert_called('GET', '/os-cloudpipe')
[self.assertIsInstance(c, cloudpipe.Cloudpipe) for c in cp]
for c in cp:
self.assertIsInstance(c, cloudpipe.Cloudpipe)
def test_create(self):
project = "test"

9
novaclient/tests/unit/v2/test_flavor_access.py

@ -27,7 +27,8 @@ class FlavorAccessTest(utils.TestCase):
kwargs = {'flavor': cs.flavors.get(2)}
r = cs.flavor_access.list(**kwargs)
cs.assert_called('GET', '/flavors/2/os-flavor-access')
[self.assertIsInstance(a, flavor_access.FlavorAccess) for a in r]
for a in r:
self.assertIsInstance(a, flavor_access.FlavorAccess)
def test_add_tenant_access(self):
flavor = cs.flavors.get(2)
@ -41,7 +42,8 @@ class FlavorAccessTest(utils.TestCase):
}
cs.assert_called('POST', '/flavors/2/action', body)
[self.assertIsInstance(a, flavor_access.FlavorAccess) for a in r]
for a in r:
self.assertIsInstance(a, flavor_access.FlavorAccess)
def test_remove_tenant_access(self):
flavor = cs.flavors.get(2)
@ -55,7 +57,8 @@ class FlavorAccessTest(utils.TestCase):
}
cs.assert_called('POST', '/flavors/2/action', body)
[self.assertIsInstance(a, flavor_access.FlavorAccess) for a in r]
for a in r:
self.assertIsInstance(a, flavor_access.FlavorAccess)
def test_repr_flavor_access(self):
flavor = cs.flavors.get(2)

8
novaclient/tests/unit/v2/test_floating_ips_bulk.py

@ -27,14 +27,14 @@ class FloatingIPsBulkTest(utils.FixturedTestCase):
def test_list_floating_ips_bulk(self):
fl = self.cs.floating_ips_bulk.list()
self.assert_called('GET', '/os-floating-ips-bulk')
[self.assertIsInstance(f, floating_ips.FloatingIP)
for f in fl]
for f in fl:
self.assertIsInstance(f, floating_ips.FloatingIP)
def test_list_floating_ips_bulk_host_filter(self):
fl = self.cs.floating_ips_bulk.list('testHost')
self.assert_called('GET', '/os-floating-ips-bulk/testHost')
[self.assertIsInstance(f, floating_ips.FloatingIP)
for f in fl]
for f in fl:
self.assertIsInstance(f, floating_ips.FloatingIP)
def test_create_floating_ips_bulk(self):
fl = self.cs.floating_ips_bulk.create('192.168.1.0/30')

13
novaclient/tests/unit/v2/test_hosts.py

@ -25,19 +25,22 @@ class HostsTest(utils.FixturedTestCase):
def test_describe_resource(self):
hs = self.cs.hosts.get('host')
self.assert_called('GET', '/os-hosts/host')
[self.assertIsInstance(h, hosts.Host) for h in hs]
for h in hs:
self.assertIsInstance(h, hosts.Host)
def test_list_host(self):
hs = self.cs.hosts.list()
self.assert_called('GET', '/os-hosts')
[self.assertIsInstance(h, hosts.Host) for h in hs]
[self.assertEqual(h.zone, 'nova1') for h in hs]
for h in hs:
self.assertIsInstance(h, hosts.Host)
self.assertEqual(h.zone, 'nova1')
def test_list_host_with_zone(self):
hs = self.cs.hosts.list('nova')
self.assert_called('GET', '/os-hosts?zone=nova')
[self.assertIsInstance(h, hosts.Host) for h in hs]
[self.assertEqual(h.zone, 'nova') for h in hs]
for h in hs:
self.assertIsInstance(h, hosts.Host)
self.assertEqual(h.zone, 'nova')
def test_update_enable(self):
host = self.cs.hosts.get('sample_host')[0]

6
novaclient/tests/unit/v2/test_images.py

@ -25,13 +25,15 @@ class ImagesTest(utils.FixturedTestCase):
def test_list_images(self):
il = self.cs.images.list()
self.assert_called('GET', '/images/detail')
[self.assertIsInstance(i, images.Image) for i in il]
for i in il:
self.assertIsInstance(i, images.Image)
self.assertEqual(2, len(il))
def test_list_images_undetailed(self):
il = self.cs.images.list(detailed=False)
self.assert_called('GET', '/images')
[self.assertIsInstance(i, images.Image) for i in il]
for i in il:
self.assertIsInstance(i, images.Image)
def test_list_images_with_limit(self):
self.cs.images.list(limit=4)

3
novaclient/tests/unit/v2/test_keypairs.py

@ -42,7 +42,8 @@ class KeypairsTest(utils.FixturedTestCase):
def test_list_keypairs(self):
kps = self.cs.keypairs.list()
self.assert_called('GET', '/%s' % self.keypair_prefix)
[self.assertIsInstance(kp, keypairs.Keypair) for kp in kps]
for kp in kps:
self.assertIsInstance(kp, keypairs.Keypair)
def test_delete_keypair(self):
kp = self.cs.keypairs.list()[0]

3
novaclient/tests/unit/v2/test_networks.py

@ -25,7 +25,8 @@ class NetworksTest(utils.FixturedTestCase):
def test_list_networks(self):
fl = self.cs.networks.list()
self.assert_called('GET', '/os-networks')
[self.assertIsInstance(f, networks.Network) for f in fl]
for f in fl:
self.assertIsInstance(f, networks.Network)
def test_get_network(self):
f = self.cs.networks.get(1)

6
novaclient/tests/unit/v2/test_servers.py

@ -40,12 +40,14 @@ class ServersTest(utils.FixturedTestCase):
def test_list_servers(self):
sl = self.cs.servers.list()
self.assert_called('GET', '/servers/detail')
[self.assertIsInstance(s, servers.Server) for s in sl]
for s in sl:
self.assertIsInstance(s, servers.Server)
def test_list_servers_undetailed(self):
sl = self.cs.servers.list(detailed=False)
self.assert_called('GET', '/servers')
[self.assertIsInstance(s, servers.Server) for s in sl]
for s in sl:
self.assertIsInstance(s, servers.Server)
def test_list_servers_with_marker_limit(self):
sl = self.cs.servers.list(marker=1234, limit=2)

3
novaclient/tests/unit/v2/test_usage.py

@ -40,7 +40,8 @@ class UsageTest(utils.TestCase):
("start=%s&" % now.isoformat()) +
("end=%s&" % now.isoformat()) +
("detailed=%s" % int(bool(detailed))))
[self.assertIsInstance(u, usage.Usage) for u in usages]
for u in usages:
self.assertIsInstance(u, usage.Usage)
def test_usage_list_detailed(self):
self.test_usage_list(True)

9
novaclient/tests/unit/v2/test_volumes.py

@ -26,12 +26,14 @@ class VolumesTest(utils.TestCase):
def test_list_servers(self):
vl = cs.volumes.list()
cs.assert_called('GET', '/volumes/detail')
[self.assertIsInstance(v, volumes.Volume) for v in vl]
for v in vl:
self.assertIsInstance(v, volumes.Volume)
def test_list_volumes_undetailed(self):
vl = cs.volumes.list(detailed=False)
cs.assert_called('GET', '/volumes')
[self.assertIsInstance(v, volumes.Volume) for v in vl]
for v in vl:
self.assertIsInstance(v, volumes.Volume)
def test_get_volume_details(self):
vol_id = '15e59938-07d5-11e1-90e3-e3dffe0c5983'
@ -86,7 +88,8 @@ class VolumesTest(utils.TestCase):
def test_list_server_volumes(self):
vl = cs.volumes.get_server_volumes(1234)
cs.assert_called('GET', '/servers/1234/os-volume_attachments')
[self.assertIsInstance(v, volumes.Volume) for v in vl]
for v in vl:
self.assertIsInstance(v, volumes.Volume)
def test_delete_server_volume(self):
cs.volumes.delete_server_volume(1234, 'Work')