From: Hirofumi Ichihara <ichihara.hirofumi@lab.ntt.co.jp>
Date: Mon, 5 Oct 2015 22:50:18 +0000 (+0900)
Subject: Use assertTrue(observed) instead of assertEqual(True, observed)
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2c62cd6ec0751a355c4db3f430e1c5d9e7a45bf7;p=openstack-build%2Fneutron-build.git

Use assertTrue(observed) instead of assertEqual(True, observed)

We should use assertTrue not assertEqual.

Closes-Bug: #1503071

Change-Id: Ib75dd9f8965fd04fe581f09a5e5df3df43542d89
---

diff --git a/HACKING.rst b/HACKING.rst
index 8db85525d..6b9df2590 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -17,6 +17,7 @@ Neutron Specific Commandments
 - [N325] Python 3: Do not use xrange.
 - [N326] Python 3: do not use basestring.
 - [N327] Python 3: do not use dict.iteritems.
+- [N328] Detect wrong usage with assertEqual
 
 Creating Unit Tests
 -------------------
diff --git a/doc/source/devref/effective_neutron.rst b/doc/source/devref/effective_neutron.rst
index 8b7981b11..0d99e0035 100644
--- a/doc/source/devref/effective_neutron.rst
+++ b/doc/source/devref/effective_neutron.rst
@@ -138,6 +138,7 @@ For anything more elaborate, please visit the testing section.
   reviewers to understand which one is the expected/observed value in non-trivial
   assertions. The expected and observed values are also labeled in the output when
   the assertion fails.
+* Prefer specific assertions (assertTrue, assertFalse) over assertEqual(True/False, observed).
 * Don't write tests that don't test the intended code. This might seem silly but
   it's easy to do with a lot of mocks in place. Ensure that your tests break as
   expected before your code change.
diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py
index cc6d6419b..383c64e9f 100644
--- a/neutron/hacking/checks.py
+++ b/neutron/hacking/checks.py
@@ -174,6 +174,18 @@ def check_python3_no_iteritems(logical_line):
         yield(0, msg)
 
 
+def check_asserttrue(logical_line, filename):
+    if 'neutron/tests/' in filename:
+        if re.search(r"assertEqual\(True,.*\)", logical_line):
+            msg = ("N328: Use assertTrue(observed) instead of"
+                   "assertEqual(True, observed)")
+            yield (0, msg)
+        if re.search(r"assertEqual\(.*, True\)", logical_line):
+            msg = ("N328: Use assertTrue(observed) instead of"
+                   "assertEqual(True, observed)")
+            yield (0, msg)
+
+
 def factory(register):
     register(validate_log_translations)
     register(use_jsonutils)
@@ -184,3 +196,4 @@ def factory(register):
     register(check_python3_xrange)
     register(check_no_basestring)
     register(check_python3_no_iteritems)
+    register(check_asserttrue)
diff --git a/neutron/tests/api/test_flavors_extensions.py b/neutron/tests/api/test_flavors_extensions.py
index 8575c6f31..31e7898ef 100644
--- a/neutron/tests/api/test_flavors_extensions.py
+++ b/neutron/tests/api/test_flavors_extensions.py
@@ -113,7 +113,7 @@ class TestFlavorsJson(base.BaseAdminNetworkTest):
                          service_profile['description'])
         self.assertEqual(self.service_profile['metainfo'],
                          service_profile['metainfo'])
-        self.assertEqual(True, service_profile['enabled'])
+        self.assertTrue(service_profile['enabled'])
 
     @test.attr(type='smoke')
     @test.idempotent_id('30abb445-0eea-472e-bd02-8649f54a5968')
@@ -124,7 +124,7 @@ class TestFlavorsJson(base.BaseAdminNetworkTest):
         self.assertEqual(self.flavor['id'], flavor['id'])
         self.assertEqual(self.flavor['description'], flavor['description'])
         self.assertEqual(self.flavor['name'], flavor['name'])
-        self.assertEqual(True, flavor['enabled'])
+        self.assertTrue(flavor['enabled'])
 
     @test.attr(type='smoke')
     @test.idempotent_id('e2fb2f8c-45bf-429a-9f17-171c70444612')
diff --git a/neutron/tests/functional/api/test_policies.py b/neutron/tests/functional/api/test_policies.py
index c73400a26..c5b705833 100644
--- a/neutron/tests/functional/api/test_policies.py
+++ b/neutron/tests/functional/api/test_policies.py
@@ -71,8 +71,7 @@ class APIPolicyTestCase(base.BaseTestCase):
         tenant_context = context.Context('test_user', 'test_tenant_id', False)
         extension_manager.extend_resources(self.api_version,
                                            attributes.RESOURCE_ATTRIBUTE_MAP)
-        self.assertEqual(self._check_external_router_policy(admin_context),
-                         True)
+        self.assertTrue(self._check_external_router_policy(admin_context))
         self.assertEqual(self._check_external_router_policy(tenant_context),
                          False)
 
@@ -87,10 +86,8 @@ class APIPolicyTestCase(base.BaseTestCase):
                                            attributes.RESOURCE_ATTRIBUTE_MAP)
         admin_context = context.get_admin_context()
         tenant_context = context.Context('test_user', 'test_tenant_id', False)
-        self.assertEqual(self._check_external_router_policy(admin_context),
-                         True)
-        self.assertEqual(self._check_external_router_policy(tenant_context),
-                         True)
+        self.assertTrue(self._check_external_router_policy(admin_context))
+        self.assertTrue(self._check_external_router_policy(tenant_context))
 
     def tearDown(self):
         policy.reset()
diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index a40a29dcc..60812cf20 100644
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -204,7 +204,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             agent = l3_agent.L3NATAgentWithStateReport(host=HOSTNAME,
                                                        conf=self.conf)
 
-            self.assertEqual(agent.agent_state['start_flag'], True)
+            self.assertTrue(agent.agent_state['start_flag'])
             use_call_arg = agent.use_call
             agent.after_start()
             report_state.assert_called_once_with(agent.context,
diff --git a/neutron/tests/unit/api/v2/test_base.py b/neutron/tests/unit/api/v2/test_base.py
index 1491dd7ed..8507df9f9 100644
--- a/neutron/tests/unit/api/v2/test_base.py
+++ b/neutron/tests/unit/api/v2/test_base.py
@@ -790,7 +790,7 @@ class JSONV2TestCase(APIv2TestBase, testlib_api.WebTestCase):
         self.assertIn('network', res)
         net = res['network']
         self.assertEqual(net['id'], net_id)
-        self.assertEqual(net['admin_state_up'], True)
+        self.assertTrue(net['admin_state_up'])
         self.assertEqual(net['status'], "ACTIVE")
 
     def test_create_no_keystone_env(self):
diff --git a/neutron/tests/unit/db/test_allowedaddresspairs_db.py b/neutron/tests/unit/db/test_allowedaddresspairs_db.py
index af0ec336d..ecf2670f6 100644
--- a/neutron/tests/unit/db/test_allowedaddresspairs_db.py
+++ b/neutron/tests/unit/db/test_allowedaddresspairs_db.py
@@ -133,7 +133,7 @@ class TestAllowedAddressPairs(AllowedAddressPairDBTestCase):
                                     port_security_enabled=True,
                                     allowed_address_pairs=address_pairs)
             port = self.deserialize(self.fmt, res)
-            self.assertEqual(port['port'][psec.PORTSECURITY], True)
+            self.assertTrue(port['port'][psec.PORTSECURITY])
             self.assertEqual(port['port'][addr_pair.ADDRESS_PAIRS],
                              address_pairs)
             self._delete('ports', port['port']['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 319229733..759eccffb 100644
--- a/neutron/tests/unit/db/test_db_base_plugin_v2.py
+++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py
@@ -1303,7 +1303,7 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s
                              'device_id': port['port']['device_id']}}
             req = self.new_update_request('ports', data, port['port']['id'])
             res = self.deserialize(self.fmt, req.get_response(self.api))
-            self.assertEqual(res['port']['admin_state_up'], True)
+            self.assertTrue(res['port']['admin_state_up'])
 
     def test_update_device_id_null(self):
         with self.port() as port:
@@ -2306,7 +2306,7 @@ class TestNetworksV2(NeutronDbPluginV2TestCase):
                 ctx = context.Context('', '', is_admin=True)
                 subnet_db = manager.NeutronManager.get_plugin().get_subnet(
                     ctx, subnet['subnet']['id'])
-                self.assertEqual(subnet_db['shared'], True)
+                self.assertTrue(subnet_db['shared'])
 
     def test_update_network_set_not_shared_single_tenant(self):
         with self.network(shared=True) as network:
diff --git a/neutron/tests/unit/extensions/test_external_net.py b/neutron/tests/unit/extensions/test_external_net.py
index 8e887973b..cf833b9d7 100644
--- a/neutron/tests/unit/extensions/test_external_net.py
+++ b/neutron/tests/unit/extensions/test_external_net.py
@@ -161,8 +161,7 @@ class ExtNetDBTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
 
     def test_create_external_network_admin_succeeds(self):
         with self.network(router__external=True) as ext_net:
-            self.assertEqual(ext_net['network'][external_net.EXTERNAL],
-                             True)
+            self.assertTrue(ext_net['network'][external_net.EXTERNAL])
 
     def test_delete_network_check_disassociated_floatingips(self):
         with mock.patch.object(manager.NeutronManager,
diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py
index 19b7abac0..4be077e1e 100644
--- a/neutron/tests/unit/extensions/test_l3.py
+++ b/neutron/tests/unit/extensions/test_l3.py
@@ -114,7 +114,7 @@ class L3NatExtensionTestCase(test_extensions_base.ExtensionTestCase):
         router = res['router']
         self.assertEqual(router['id'], router_id)
         self.assertEqual(router['status'], "ACTIVE")
-        self.assertEqual(router['admin_state_up'], True)
+        self.assertTrue(router['admin_state_up'])
 
     def test_router_list(self):
         router_id = _uuid()
diff --git a/neutron/tests/unit/extensions/test_portsecurity.py b/neutron/tests/unit/extensions/test_portsecurity.py
index 76a269839..353477b6c 100644
--- a/neutron/tests/unit/extensions/test_portsecurity.py
+++ b/neutron/tests/unit/extensions/test_portsecurity.py
@@ -176,7 +176,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
     def test_create_network_with_portsecurity_mac(self):
         res = self._create_network('json', 'net1', True)
         net = self.deserialize('json', res)
-        self.assertEqual(net['network'][psec.PORTSECURITY], True)
+        self.assertTrue(net['network'][psec.PORTSECURITY])
 
     def test_create_network_with_portsecurity_false(self):
         res = self._create_network('json', 'net1', True,
@@ -189,7 +189,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
         res = self._create_network('json', 'net1', True,
                                    port_security_enabled='True')
         net = self.deserialize('json', res)
-        self.assertEqual(net['network'][psec.PORTSECURITY], True)
+        self.assertTrue(net['network'][psec.PORTSECURITY])
         update_net = {'network': {psec.PORTSECURITY: False}}
         req = self.new_update_request('networks', update_net,
                                       net['network']['id'])
@@ -203,7 +203,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
         with self.network() as net:
             res = self._create_port('json', net['network']['id'])
             port = self.deserialize('json', res)
-            self.assertEqual(port['port'][psec.PORTSECURITY], True)
+            self.assertTrue(port['port'][psec.PORTSECURITY])
             self._delete('ports', port['port']['id'])
 
     def test_create_port_passing_true(self):
@@ -213,7 +213,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
         net = self.deserialize('json', res)
         res = self._create_port('json', net['network']['id'])
         port = self.deserialize('json', res)
-        self.assertEqual(port['port'][psec.PORTSECURITY], True)
+        self.assertTrue(port['port'][psec.PORTSECURITY])
         self._delete('ports', port['port']['id'])
 
     def test_create_port_on_port_security_false_network(self):
@@ -235,7 +235,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
                                 arg_list=('port_security_enabled',),
                                 port_security_enabled=True)
         port = self.deserialize('json', res)
-        self.assertEqual(port['port'][psec.PORTSECURITY], True)
+        self.assertTrue(port['port'][psec.PORTSECURITY])
         self._delete('ports', port['port']['id'])
 
     def test_create_port_fails_with_secgroup_and_port_security_false(self):
@@ -261,7 +261,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
             with self.subnet(network=net):
                 res = self._create_port('json', net['network']['id'])
                 port = self.deserialize('json', res)
-                self.assertEqual(port['port'][psec.PORTSECURITY], True)
+                self.assertTrue(port['port'][psec.PORTSECURITY])
                 self.assertEqual(len(port['port'][ext_sg.SECURITYGROUPS]), 1)
                 self._delete('ports', port['port']['id'])
 
@@ -285,7 +285,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
                                 port_security_enabled=True,
                                 security_groups=[security_group_id])
         port = self.deserialize('json', res)
-        self.assertEqual(port['port'][psec.PORTSECURITY], True)
+        self.assertTrue(port['port'][psec.PORTSECURITY])
         self.assertEqual(port['port']['security_groups'], [security_group_id])
         self._delete('ports', port['port']['id'])
 
@@ -307,7 +307,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
             with self.subnet(network=net):
                 res = self._create_port('json', net['network']['id'])
                 port = self.deserialize('json', res)
-                self.assertEqual(port['port'][psec.PORTSECURITY], True)
+                self.assertTrue(port['port'][psec.PORTSECURITY])
 
                 update_port = {'port': {psec.PORTSECURITY: False}}
                 req = self.new_update_request('ports', update_port,
@@ -331,7 +331,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
                                         arg_list=('port_security_enabled',),
                                         port_security_enabled=True)
                 port = self.deserialize('json', res)
-                self.assertEqual(port['port'][psec.PORTSECURITY], True)
+                self.assertTrue(port['port'][psec.PORTSECURITY])
 
                 # remove security group on port
                 update_port = {'port': {ext_sg.SECURITYGROUPS: None,
@@ -352,7 +352,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
                                         arg_list=('port_security_enabled',),
                                         port_security_enabled=True)
                 port = self.deserialize('json', res)
-                self.assertEqual(port['port'][psec.PORTSECURITY], True)
+                self.assertTrue(port['port'][psec.PORTSECURITY])
 
                 # remove security group on port
                 update_port = {'port': {ext_sg.SECURITYGROUPS: None,
@@ -369,7 +369,7 @@ class TestPortSecurity(PortSecurityDBTestCase):
                                               port['port']['id'])
 
                 port = self.deserialize('json', req.get_response(self.api))
-                self.assertEqual(port['port'][psec.PORTSECURITY], True)
+                self.assertTrue(port['port'][psec.PORTSECURITY])
                 self.assertEqual(len(port['port'][ext_sg.SECURITYGROUPS]), 1)
                 self._delete('ports', port['port']['id'])
 
diff --git a/neutron/tests/unit/extensions/test_vlantransparent.py b/neutron/tests/unit/extensions/test_vlantransparent.py
index 717d306b7..ee48f0828 100644
--- a/neutron/tests/unit/extensions/test_vlantransparent.py
+++ b/neutron/tests/unit/extensions/test_vlantransparent.py
@@ -82,7 +82,7 @@ class VlanTransparentExtensionTestCase(test_db_base_plugin_v2.TestNetworksV2):
             res = self.deserialize(self.fmt, req.get_response(self.api))
             self.assertEqual(net['network']['name'],
                              res['network']['name'])
-            self.assertEqual(True, res['network'][vlt.VLANTRANSPARENT])
+            self.assertTrue(res['network'][vlt.VLANTRANSPARENT])
 
     def test_network_create_with_bad_vlan_transparent_attr(self):
         vlantrans = {'vlan_transparent': "abc"}
diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py
index f3e98c92d..d6e1d5f8d 100644
--- a/neutron/tests/unit/hacking/test_checks.py
+++ b/neutron/tests/unit/hacking/test_checks.py
@@ -154,3 +154,26 @@ class HackingTestCase(base.BaseTestCase):
         f = checks.check_python3_no_iteritems
         self.assertLineFails(f, "d.iteritems()")
         self.assertLinePasses(f, "six.iteritems(d)")
+
+    def test_asserttrue(self):
+        fail_code1 = """
+               test_bool = True
+               self.assertEqual(True, test_bool)
+               """
+        fail_code2 = """
+               test_bool = True
+               self.assertEqual(test_bool, True)
+               """
+        pass_code = """
+               test_bool = True
+               self.assertTrue(test_bool)
+               """
+        self.assertEqual(
+            1, len(list(checks.check_asserttrue(fail_code1,
+                                            "neutron/tests/test_assert.py"))))
+        self.assertEqual(
+            1, len(list(checks.check_asserttrue(fail_code2,
+                                            "neutron/tests/test_assert.py"))))
+        self.assertEqual(
+            0, len(list(checks.check_asserttrue(pass_code,
+                                            "neutron/tests/test_assert.py"))))
diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
index 9cd455bba..c3539c1c1 100644
--- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py
@@ -108,7 +108,7 @@ class CreateAgentConfigMap(ovs_test_base.OVSAgentConfigTestBase):
         cfg.CONF.set_override('enable_distributed_routing', True,
                               group='AGENT')
         cfgmap = self.mod_agent.create_agent_config_map(cfg.CONF)
-        self.assertEqual(cfgmap['enable_distributed_routing'], True)
+        self.assertTrue(cfgmap['enable_distributed_routing'])
 
 
 class TestOvsNeutronAgent(object):
diff --git a/neutron/tests/unit/test_auth.py b/neutron/tests/unit/test_auth.py
index 74323f071..ad20a2321 100644
--- a/neutron/tests/unit/test_auth.py
+++ b/neutron/tests/unit/test_auth.py
@@ -74,7 +74,7 @@ class NeutronKeystoneContextTestCase(base.BaseTestCase):
         self.assertEqual(response.status, '200 OK')
         self.assertEqual(self.context.roles, ['role1', 'role2', 'role3',
                                               'role4', 'role5', 'AdMiN'])
-        self.assertEqual(self.context.is_admin, True)
+        self.assertTrue(self.context.is_admin)
 
     def test_with_user_tenant_name(self):
         self.request.headers['X_PROJECT_ID'] = 'testtenantid'
diff --git a/neutron/tests/unit/test_policy.py b/neutron/tests/unit/test_policy.py
index ed230179f..019b9f92e 100644
--- a/neutron/tests/unit/test_policy.py
+++ b/neutron/tests/unit/test_policy.py
@@ -103,7 +103,7 @@ class PolicyTestCase(base.BaseTestCase):
     def test_enforce_good_action(self):
         action = "example:allowed"
         result = policy.enforce(self.context, action, self.target)
-        self.assertEqual(result, True)
+        self.assertTrue(result)
 
     @mock.patch.object(urlrequest, 'urlopen',
                        return_value=six.StringIO("True"))
@@ -111,7 +111,7 @@ class PolicyTestCase(base.BaseTestCase):
         action = "example:get_http"
         target = {}
         result = policy.enforce(self.context, action, target)
-        self.assertEqual(result, True)
+        self.assertTrue(result)
 
     def test_enforce_http_false(self):
 
@@ -305,7 +305,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
                               context, action, target)
         else:
             result = policy.enforce(context, action, target)
-            self.assertEqual(result, True)
+            self.assertTrue(result)
 
     def _test_nonadmin_action_on_attr(self, action, attr, value,
                                       exception=None, **kwargs):
@@ -411,7 +411,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
         if kwargs:
             target.update(kwargs)
         result = policy.enforce(admin_context, action, target)
-        self.assertEqual(result, True)
+        self.assertTrue(result)
 
     def test_enforce_adminonly_attribute_create(self):
         self._test_enforce_adminonly_attribute('create_network')
@@ -469,7 +469,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
         action = "create_" + FAKE_RESOURCE_NAME
         target = {'tenant_id': 'fake', 'attr': {'sub_attr_1': 'x'}}
         result = policy.enforce(self.context, action, target, None)
-        self.assertEqual(result, True)
+        self.assertTrue(result)
 
     def test_enforce_admin_only_subattribute(self):
         action = "create_" + FAKE_RESOURCE_NAME
@@ -477,7 +477,7 @@ class NeutronPolicyTestCase(base.BaseTestCase):
                                                 'sub_attr_2': 'y'}}
         result = policy.enforce(context.get_admin_context(),
                                 action, target, None)
-        self.assertEqual(result, True)
+        self.assertTrue(result)
 
     def test_enforce_admin_only_subattribute_nonadminctx_returns_403(self):
         action = "create_" + FAKE_RESOURCE_NAME