openstack
/
neutron
Code Issues Proposed changes

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

This commit is contained in:
Jenkins 2015-10-12 09:13:37 +00:00 committed by Gerrit Code Review
parent f3ccc38c88 2c62cd6ec0
commit abe707eade
17 changed files with 70 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
HACKING.rst
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
-------------------

1
doc/source/devref/effective_neutron.rst
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.

13
neutron/hacking/checks.py
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)

4
neutron/tests/api/test_flavors_extensions.py
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')

9
neutron/tests/functional/api/test_policies.py
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()

2
neutron/tests/unit/agent/l3/test_agent.py
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,

2
neutron/tests/unit/api/v2/test_base.py
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):

2
neutron/tests/unit/db/test_allowedaddresspairs_db.py
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'])

4
neutron/tests/unit/db/test_db_base_plugin_v2.py
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:

3
neutron/tests/unit/extensions/test_external_net.py
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,

2
neutron/tests/unit/extensions/test_l3.py
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()

22
neutron/tests/unit/extensions/test_portsecurity.py
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'])

2
neutron/tests/unit/extensions/test_vlantransparent.py
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"}

23
neutron/tests/unit/hacking/test_checks.py
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"))))

2
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.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):

2
neutron/tests/unit/test_auth.py
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'

12
neutron/tests/unit/test_policy.py
View File

@ -101,7 +101,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)
#TODO(kevinbenton): replace these private method mocks with a fixture
@mock.patch.object(oslo_policy._checks.HttpCheck, '__call__',
@ -110,7 +110,7 @@ class PolicyTestCase(base.BaseTestCase):
action = "example:get_http"
target = {}
result = policy.enforce(self.context, action, target)
self.assertEqual(result, True)
self.assertTrue(result)
#TODO(kevinbenton): replace these private method mocks with a fixture
@mock.patch.object(oslo_policy._checks.HttpCheck, '__call__',
@ -302,7 +302,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):
@ -408,7 +408,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')
@ -466,7 +466,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
@ -474,7 +474,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