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

We should use assertTrue not assertEqual.

Closes-Bug: #1503071

Change-Id: Ib75dd9f8965fd04fe581f09a5e5df3df43542d89
This commit is contained in:
Hirofumi Ichihara 2015-10-06 07:50:18 +09:00
parent 7aed40f891
commit 2c62cd6ec0
17 changed files with 70 additions and 36 deletions

View File

@ -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
-------------------

View File

@ -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.

View File

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

View File

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

View File

@ -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()

View File

@ -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,

View File

@ -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):

View File

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

View File

@ -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:

View File

@ -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,

View File

@ -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()

View File

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

View File

@ -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"}

View File

@ -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"))))

View File

@ -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):

View File

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

View File

@ -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