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')