From ae1fb6e87225900bedbc68d014369a1adaa4fcab Mon Sep 17 00:00:00 2001 From: Hirofumi Ichihara Date: Fri, 9 Oct 2015 23:38:27 +0900 Subject: [PATCH] Use assertFalse(observed) instead of assertEqual(False, observed) The patch also improves the way in which the assertTrue (and similarly assertFalse) are done, We should use assertFalse not assertEqual. Co-Authored-By: Gary Kotton Closes-Bug: #1503074 Change-Id: I5f527ddf2ca522cdf101de2482d59f059eca010f --- neutron/hacking/checks.py | 21 +++++++++++++---- neutron/tests/api/test_routers.py | 4 ++-- neutron/tests/functional/api/test_policies.py | 3 +-- .../unit/agent/test_securitygroups_rpc.py | 4 ++-- .../tests/unit/db/test_db_base_plugin_v2.py | 2 +- neutron/tests/unit/extensions/test_l3.py | 4 ++-- .../unit/extensions/test_portsecurity.py | 10 ++++---- neutron/tests/unit/hacking/test_checks.py | 23 +++++++++++++++++++ neutron/tests/unit/objects/qos/test_policy.py | 2 +- neutron/tests/unit/test_auth.py | 2 +- neutron/tests/unit/test_policy.py | 2 +- 11 files changed, 56 insertions(+), 21 deletions(-) diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index a1d2ae4f394..a8eee49d0b4 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -177,12 +177,12 @@ def check_python3_no_iteritems(logical_line): 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" + if re.search(r"assertEqual\(\s*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" + if re.search(r"assertEqual\([^,]*,\s*True(,[^,]*)?\)", logical_line): + msg = ("N328: Use assertTrue(observed) instead of " "assertEqual(True, observed)") yield (0, msg) @@ -193,6 +193,18 @@ def no_mutable_default_args(logical_line): yield (0, msg) +def check_assertfalse(logical_line, filename): + if 'neutron/tests/' in filename: + if re.search(r"assertEqual\(\s*False,[^,]*(,[^,]*)?\)", logical_line): + msg = ("N328: Use assertFalse(observed) instead of " + "assertEqual(False, observed)") + yield (0, msg) + if re.search(r"assertEqual\([^,]*,\s*False(,[^,]*)?\)", logical_line): + msg = ("N328: Use assertFalse(observed) instead of " + "assertEqual(False, observed)") + yield (0, msg) + + def factory(register): register(validate_log_translations) register(use_jsonutils) @@ -205,3 +217,4 @@ def factory(register): register(check_python3_no_iteritems) register(check_asserttrue) register(no_mutable_default_args) + register(check_assertfalse) diff --git a/neutron/tests/api/test_routers.py b/neutron/tests/api/test_routers.py index 4cde8fb82c1..064157e47cd 100644 --- a/neutron/tests/api/test_routers.py +++ b/neutron/tests/api/test_routers.py @@ -55,14 +55,14 @@ class RoutersTest(base.BaseRouterTest): self.assertEqual( create_body['router']['external_gateway_info']['network_id'], CONF.network.public_network_id) - self.assertEqual(create_body['router']['admin_state_up'], False) + self.assertFalse(create_body['router']['admin_state_up']) # Show details of the created router show_body = self.client.show_router(create_body['router']['id']) self.assertEqual(show_body['router']['name'], name) self.assertEqual( show_body['router']['external_gateway_info']['network_id'], CONF.network.public_network_id) - self.assertEqual(show_body['router']['admin_state_up'], False) + self.assertFalse(show_body['router']['admin_state_up']) # List routers and verify if created router is there in response list_body = self.client.list_routers() routers_list = list() diff --git a/neutron/tests/functional/api/test_policies.py b/neutron/tests/functional/api/test_policies.py index c5b70583355..1912f1d01e8 100644 --- a/neutron/tests/functional/api/test_policies.py +++ b/neutron/tests/functional/api/test_policies.py @@ -72,8 +72,7 @@ class APIPolicyTestCase(base.BaseTestCase): extension_manager.extend_resources(self.api_version, attributes.RESOURCE_ATTRIBUTE_MAP) self.assertTrue(self._check_external_router_policy(admin_context)) - self.assertEqual(self._check_external_router_policy(tenant_context), - False) + self.assertFalse(self._check_external_router_policy(tenant_context)) def test_proper_load_order(self): """ diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py index 800f08fa03e..beebcad3c61 100644 --- a/neutron/tests/unit/agent/test_securitygroups_rpc.py +++ b/neutron/tests/unit/agent/test_securitygroups_rpc.py @@ -1635,12 +1635,12 @@ class SecurityGroupAgentRpcApiTestCase(base.BaseTestCase): def test_security_groups_rule_not_updated(self): self.notifier.security_groups_rule_updated( None, security_groups=[]) - self.assertEqual(False, self.mock_cast.called) + self.assertFalse(self.mock_cast.called) def test_security_groups_member_not_updated(self): self.notifier.security_groups_member_updated( None, security_groups=[]) - self.assertEqual(False, self.mock_cast.called) + self.assertFalse(self.mock_cast.called) #Note(nati) bn -> binary_name # id -> device_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 cc87fc1bf0f..a2300ba1726 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -1298,7 +1298,7 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s neutron_context = context.Context('', 'not_admin') port = self._update('ports', port['port']['id'], data, neutron_context=neutron_context) - self.assertEqual(port['port']['admin_state_up'], False) + self.assertFalse(port['port']['admin_state_up']) def test_update_device_id_unchanged(self): with self.port() as port: diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index 77183081787..f9dc6a8c8ca 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -160,7 +160,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'], False) + self.assertFalse(router['admin_state_up']) def test_router_get(self): router_id = _uuid() @@ -182,7 +182,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'], False) + self.assertFalse(router['admin_state_up']) def test_router_delete(self): router_id = _uuid() diff --git a/neutron/tests/unit/extensions/test_portsecurity.py b/neutron/tests/unit/extensions/test_portsecurity.py index ed3deb8d39c..734ad8ca1b2 100644 --- a/neutron/tests/unit/extensions/test_portsecurity.py +++ b/neutron/tests/unit/extensions/test_portsecurity.py @@ -182,7 +182,7 @@ class TestPortSecurity(PortSecurityDBTestCase): arg_list=('port_security_enabled',), port_security_enabled=False) net = self.deserialize('json', res) - self.assertEqual(net['network'][psec.PORTSECURITY], False) + self.assertFalse(net['network'][psec.PORTSECURITY]) def test_updating_network_port_security(self): res = self._create_network('json', 'net1', True, @@ -193,10 +193,10 @@ class TestPortSecurity(PortSecurityDBTestCase): req = self.new_update_request('networks', update_net, net['network']['id']) net = self.deserialize('json', req.get_response(self.api)) - self.assertEqual(net['network'][psec.PORTSECURITY], False) + self.assertFalse(net['network'][psec.PORTSECURITY]) req = self.new_show_request('networks', net['network']['id']) net = self.deserialize('json', req.get_response(self.api)) - self.assertEqual(net['network'][psec.PORTSECURITY], False) + self.assertFalse(net['network'][psec.PORTSECURITY]) def test_create_port_default_true(self): with self.network() as net: @@ -222,7 +222,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], False) + self.assertFalse(port['port'][psec.PORTSECURITY]) self._delete('ports', port['port']['id']) def test_create_port_security_overrides_network_value(self): @@ -338,7 +338,7 @@ class TestPortSecurity(PortSecurityDBTestCase): req = self.new_update_request('ports', update_port, port['port']['id']) port = self.deserialize('json', req.get_response(self.api)) - self.assertEqual(port['port'][psec.PORTSECURITY], False) + self.assertFalse(port['port'][psec.PORTSECURITY]) self.assertEqual(len(port['port'][ext_sg.SECURITYGROUPS]), 0) self._delete('ports', port['port']['id']) diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index 97193bf28a9..2853fc08267 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -190,3 +190,26 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual(0, len(list(checks.no_mutable_default_args( "defined, undefined = [], {}")))) + + def test_assertfalse(self): + fail_code1 = """ + test_bool = False + self.assertEqual(False, test_bool) + """ + fail_code2 = """ + test_bool = False + self.assertEqual(test_bool, False) + """ + pass_code = """ + test_bool = False + self.assertFalse(test_bool) + """ + self.assertEqual( + 1, len(list(checks.check_assertfalse(fail_code1, + "neutron/tests/test_assert.py")))) + self.assertEqual( + 1, len(list(checks.check_assertfalse(fail_code2, + "neutron/tests/test_assert.py")))) + self.assertEqual( + 0, len(list(checks.check_assertfalse(pass_code, + "neutron/tests/test_assert.py")))) diff --git a/neutron/tests/unit/objects/qos/test_policy.py b/neutron/tests/unit/objects/qos/test_policy.py index 6b29b06bb59..05baad58aea 100644 --- a/neutron/tests/unit/objects/qos/test_policy.py +++ b/neutron/tests/unit/objects/qos/test_policy.py @@ -267,7 +267,7 @@ class QosPolicyDbObjectTestCase(test_base.BaseDbObjectTestCase, def test_shared_default(self): self.db_obj.pop('shared') obj = self._test_class(self.context, **self.db_obj) - self.assertEqual(False, obj.shared) + self.assertFalse(obj.shared) def test_delete_not_allowed_if_policy_in_use_by_port(self): obj = self._create_test_policy() diff --git a/neutron/tests/unit/test_auth.py b/neutron/tests/unit/test_auth.py index ad20a2321aa..d64daf6c3f6 100644 --- a/neutron/tests/unit/test_auth.py +++ b/neutron/tests/unit/test_auth.py @@ -63,7 +63,7 @@ class NeutronKeystoneContextTestCase(base.BaseTestCase): self.assertEqual(response.status, '200 OK') self.assertEqual(self.context.roles, ['role1', 'role2', 'role3', 'role4', 'role5']) - self.assertEqual(self.context.is_admin, False) + self.assertFalse(self.context.is_admin) def test_roles_with_admin(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 b2002422dfe..c6618db121c 100644 --- a/neutron/tests/unit/test_policy.py +++ b/neutron/tests/unit/test_policy.py @@ -89,7 +89,7 @@ class PolicyTestCase(base.BaseTestCase): def test_check_bad_action_noraise(self): action = "example:denied" result = policy.check(self.context, action, self.target) - self.assertEqual(result, False) + self.assertFalse(result) def test_check_non_existent_action(self): action = "example:idonotexist"