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

diff --git a/doc/source/devref/effective_neutron.rst b/doc/source/devref/effective_neutron.rst
index 89500f546ecdb50b07a539fadfc55c9989a67999..98bf8e9fb68138d33016623cf294f793c54d2a61 100644 (file)
--- 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 6db8d21907c9968cb7b3f6e37c76059df65cac24..5978856d87134ce8a7c685308f2409538f6bb61a 100644 (file)
--- 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 c1142bbc449ccac3f395da104f380e2b16afd354..7f4c365df492e43ea6608566e389b9aad556d09a 100644 (file)
--- 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 ef5d614602fd24882006deb7a9ba592077f47173..0e6287e27fea91f21535fea8d7793309f7564790 100644 (file)
--- 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 4e197940d8e8d2bfae1f082983b1068bcb3cfd08..3192297333b26060c5d7adf8c93386e6bc3c8e6d 100644 (file)
--- 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 c9f170f058e3828f2fb38b47813cbc317baeb964..ef03866572756ab66468bd96a8d1071396f1e25a 100644 (file)
--- 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 4d36c886ff9737502a59374d2fc513a7acbafde0..7a54b39af9c3b7bf33f03aeddf90ace77a4dd19f 100644 (file)
--- 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')