From 49746ca4749c45d2772289061f161145eb5ad2b8 Mon Sep 17 00:00:00 2001 From: Cedric Brandily Date: Sun, 4 Oct 2015 20:48:44 +0200 Subject: [PATCH] Use assertIn and assertNotIn Neutron tests should use: self.assertIn(value, list) self.assertNotIn(value, list) instead of: self.assertTrue(value in list) self.assertFalse(value in list) because assertIn and assertNotIn raise more meaningful errors: self.assertIn(3, [1, 2] >>> MismatchError: 3 not in [1, 2] self.assertTrue(3 in [1, 2]) >>> AssertionError: False is not true Closes-Bug: #1218713 Change-Id: Ic8492a88935bf005feb9dae726a4bee604a8bd09 --- doc/source/devref/effective_neutron.rst | 13 +++++++++ neutron/tests/api/test_ports.py | 2 +- .../unit/agent/l3/test_item_allocator.py | 28 +++++++++---------- .../agent/l3/test_router_processing_queue.py | 10 +++---- .../tests/unit/db/test_db_base_plugin_v2.py | 8 +++--- .../ml2/drivers/l2pop/test_mech_driver.py | 2 +- neutron/tests/unit/plugins/ml2/test_rpc.py | 2 +- 7 files changed, 39 insertions(+), 26 deletions(-) diff --git a/doc/source/devref/effective_neutron.rst b/doc/source/devref/effective_neutron.rst index 89500f546ec..98bf8e9fb68 100644 --- a/doc/source/devref/effective_neutron.rst +++ b/doc/source/devref/effective_neutron.rst @@ -86,6 +86,19 @@ For anything more elaborate, please visit the testing section. * Preferring low level testing versus full path testing (e.g. not testing database via client calls). The former is to be favored in unit testing, whereas the latter is to be favored in functional testing. +* Prefer specific assertions (assert(Not)In, assert(Not)IsInstance, assert(Not)IsNone, + etc) over generic ones (assertTrue/False, assertEqual) because they raise more + meaningful errors: + + .. code:: python + + def test_specific(self): + self.assertIn(3, [1, 2]) + # raise meaningful error: "MismatchError: 3 not in [1, 2]" + + def test_generic(self): + self.assertTrue(3 in [1, 2]) + # raise meaningless error: "AssertionError: False is not true" Backward compatibility ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/neutron/tests/api/test_ports.py b/neutron/tests/api/test_ports.py index 6db8d21907c..5978856d871 100644 --- a/neutron/tests/api/test_ports.py +++ b/neutron/tests/api/test_ports.py @@ -49,7 +49,7 @@ class PortsTestJSON(sec_base.BaseSecGroupTest): self.client.delete_port(port_id) body = self.client.list_ports() ports_list = body['ports'] - self.assertFalse(port_id in [n['id'] for n in ports_list]) + self.assertNotIn(port_id, [n['id'] for n in ports_list]) @test.attr(type='smoke') @test.idempotent_id('c72c1c0c-2193-4aca-aaa4-b1442640f51c') diff --git a/neutron/tests/unit/agent/l3/test_item_allocator.py b/neutron/tests/unit/agent/l3/test_item_allocator.py index c1142bbc449..7f4c365df49 100644 --- a/neutron/tests/unit/agent/l3/test_item_allocator.py +++ b/neutron/tests/unit/agent/l3/test_item_allocator.py @@ -37,9 +37,9 @@ class TestItemAllocator(base.BaseTestCase): a = ia.ItemAllocator('/file', TestObject, test_pool) test_object = a.allocate('test') - self.assertTrue('test' in a.allocations) - self.assertTrue(test_object in a.allocations.values()) - self.assertTrue(test_object not in a.pool) + self.assertIn('test', a.allocations) + self.assertIn(test_object, a.allocations.values()) + self.assertNotIn(test_object, a.pool) self.assertTrue(write.called) def test__init__readfile(self): @@ -48,7 +48,7 @@ class TestItemAllocator(base.BaseTestCase): read.return_value = ["da873ca2,10\n"] a = ia.ItemAllocator('/file', TestObject, test_pool) - self.assertTrue('da873ca2' in a.remembered) + self.assertIn('da873ca2', a.remembered) self.assertEqual({}, a.allocations) def test_allocate(self): @@ -57,9 +57,9 @@ class TestItemAllocator(base.BaseTestCase): with mock.patch.object(ia.ItemAllocator, '_write') as write: test_object = a.allocate('test') - self.assertTrue('test' in a.allocations) - self.assertTrue(test_object in a.allocations.values()) - self.assertTrue(test_object not in a.pool) + self.assertIn('test', a.allocations) + self.assertIn(test_object, a.allocations.values()) + self.assertNotIn(test_object, a.pool) self.assertTrue(write.called) def test_allocate_from_file(self): @@ -72,9 +72,9 @@ class TestItemAllocator(base.BaseTestCase): t_obj = a.allocate('deadbeef') self.assertEqual('33000', t_obj._value) - self.assertTrue('deadbeef' in a.allocations) - self.assertTrue(t_obj in a.allocations.values()) - self.assertTrue(33000 not in a.pool) + self.assertIn('deadbeef', a.allocations) + self.assertIn(t_obj, a.allocations.values()) + self.assertNotIn(33000, a.pool) self.assertFalse(write.called) def test_allocate_exhausted_pool(self): @@ -86,8 +86,8 @@ class TestItemAllocator(base.BaseTestCase): with mock.patch.object(ia.ItemAllocator, '_write') as write: allocation = a.allocate('abcdef12') - self.assertFalse('deadbeef' in a.allocations) - self.assertTrue(allocation not in a.pool) + self.assertNotIn('deadbeef', a.allocations) + self.assertNotIn(allocation, a.pool) self.assertTrue(write.called) def test_release(self): @@ -98,7 +98,7 @@ class TestItemAllocator(base.BaseTestCase): write.reset_mock() a.release('deadbeef') - self.assertTrue('deadbeef' not in a.allocations) - self.assertTrue(allocation in a.pool) + self.assertNotIn('deadbeef', a.allocations) + self.assertIn(allocation, a.pool) self.assertEqual({}, a.allocations) write.assert_called_once_with([]) diff --git a/neutron/tests/unit/agent/l3/test_router_processing_queue.py b/neutron/tests/unit/agent/l3/test_router_processing_queue.py index ef5d614602f..0e6287e27fe 100644 --- a/neutron/tests/unit/agent/l3/test_router_processing_queue.py +++ b/neutron/tests/unit/agent/l3/test_router_processing_queue.py @@ -58,22 +58,22 @@ class TestExclusiveRouterProcessor(base.BaseTestCase): master_2.__exit__(None, None, None) def test__enter__(self): - self.assertFalse(FAKE_ID in l3_queue.ExclusiveRouterProcessor._masters) + self.assertNotIn(FAKE_ID, l3_queue.ExclusiveRouterProcessor._masters) master = l3_queue.ExclusiveRouterProcessor(FAKE_ID) master.__enter__() - self.assertTrue(FAKE_ID in l3_queue.ExclusiveRouterProcessor._masters) + self.assertIn(FAKE_ID, l3_queue.ExclusiveRouterProcessor._masters) master.__exit__(None, None, None) def test__exit__(self): master = l3_queue.ExclusiveRouterProcessor(FAKE_ID) not_master = l3_queue.ExclusiveRouterProcessor(FAKE_ID) master.__enter__() - self.assertTrue(FAKE_ID in l3_queue.ExclusiveRouterProcessor._masters) + self.assertIn(FAKE_ID, l3_queue.ExclusiveRouterProcessor._masters) not_master.__enter__() not_master.__exit__(None, None, None) - self.assertTrue(FAKE_ID in l3_queue.ExclusiveRouterProcessor._masters) + self.assertIn(FAKE_ID, l3_queue.ExclusiveRouterProcessor._masters) master.__exit__(None, None, None) - self.assertFalse(FAKE_ID in l3_queue.ExclusiveRouterProcessor._masters) + self.assertNotIn(FAKE_ID, l3_queue.ExclusiveRouterProcessor._masters) def test_data_fetched_since(self): master = l3_queue.ExclusiveRouterProcessor(FAKE_ID) diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 4e197940d8e..3192297333b 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -2839,7 +2839,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): res = subnet_req.get_response(self.api) subnet = self.deserialize(self.fmt, res)['subnet'] ip_net = netaddr.IPNetwork(subnet['cidr']) - self.assertTrue(ip_net in netaddr.IPNetwork(subnetpool_prefix)) + self.assertIn(ip_net, netaddr.IPNetwork(subnetpool_prefix)) self.assertEqual(27, ip_net.prefixlen) self.assertEqual(subnetpool_id, subnet['subnetpool_id']) @@ -2863,7 +2863,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): subnet = self.deserialize(self.fmt, res)['subnet'] self.assertEqual(subnetpool_id, subnet['subnetpool_id']) ip_net = netaddr.IPNetwork(subnet['cidr']) - self.assertTrue(ip_net in netaddr.IPNetwork(subnetpool_prefix)) + self.assertIn(ip_net, netaddr.IPNetwork(subnetpool_prefix)) self.assertEqual(64, ip_net.prefixlen) def test_create_subnet_bad_V4_cidr_prefix_len(self): @@ -4188,7 +4188,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): list(res['subnet']['allocation_pools'][1].values()) ) for pool_val in ['10', '20', '30', '40']: - self.assertTrue('192.168.0.%s' % (pool_val) in res_vals) + self.assertIn('192.168.0.%s' % (pool_val), res_vals) if with_gateway_ip: self.assertEqual((res['subnet']['gateway_ip']), '192.168.0.9') @@ -4777,7 +4777,7 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): subnet['subnet']['id']) delete_response = delete_request.get_response(self.api) - self.assertTrue('NeutronError' in delete_response.json) + self.assertIn('NeutronError', delete_response.json) self.assertEqual('SubnetInUse', delete_response.json['NeutronError']['type']) diff --git a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py index c9f170f058e..ef038665727 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/l2pop/test_mech_driver.py @@ -808,7 +808,7 @@ class TestL2PopulationMechDriver(base.BaseTestCase): def test_get_tunnels(self): tunnels = self._test_get_tunnels('20.0.0.1') - self.assertTrue('20.0.0.1' in tunnels) + self.assertIn('20.0.0.1', tunnels) def test_get_tunnels_no_ip(self): tunnels = self._test_get_tunnels(None) diff --git a/neutron/tests/unit/plugins/ml2/test_rpc.py b/neutron/tests/unit/plugins/ml2/test_rpc.py index 4d36c886ff9..7a54b39af9c 100644 --- a/neutron/tests/unit/plugins/ml2/test_rpc.py +++ b/neutron/tests/unit/plugins/ml2/test_rpc.py @@ -116,7 +116,7 @@ class RpcCallbacksTestCase(base.BaseTestCase): {"id": "fake_network"}) self.callbacks.get_device_details(mock.Mock(), host='fake_host', cached_networks=cached_networks) - self.assertTrue('fake_port' in cached_networks) + self.assertIn('fake_port', cached_networks) def test_get_device_details_wrong_host(self): port = collections.defaultdict(lambda: 'fake')