From 0f50d8c06f4bead8ee5fc4a90524b338f86a8ed6 Mon Sep 17 00:00:00 2001 From: lzklibj Date: Thu, 28 Jan 2016 17:42:53 +0800 Subject: [PATCH] Add hacking check for assertEqual HTTP code Add a hacking check for wrong usage, like: assertEqual(observed.code, webob.exc.HTTP**.code) the correct usage should be: assertEqual(webob.exc.HTTP**.code, observed.code) Change-Id: I8c038bae61c528d30af43e904a9bb98ac4cd4d3a --- HACKING.rst | 2 ++ neutron/hacking/checks.py | 10 ++++++ .../unit/agent/test_securitygroups_rpc.py | 8 ++--- neutron/tests/unit/api/v2/test_resource.py | 2 +- .../tests/unit/db/test_db_base_plugin_v2.py | 6 ++-- neutron/tests/unit/extensions/test_l3.py | 2 +- .../unit/extensions/test_securitygroup.py | 34 +++++++++---------- neutron/tests/unit/hacking/test_checks.py | 14 ++++++++ 8 files changed, 52 insertions(+), 26 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index b1ecb523b35..89c1bbe1701 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -22,6 +22,8 @@ Neutron Specific Commandments - [N330] Use assertEqual(*empty*, observed) instead of assertEqual(observed, *empty*) - [N331] Detect wrong usage with assertTrue(isinstance()). +- [N332] Use assertEqual(expected_http_code, observed_http_code) instead of + assertEqual(observed_http_code, expected_http_code). Creating Unit Tests ------------------- diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index 3441b68a0d0..a458fab8951 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -228,6 +228,15 @@ def check_assertisinstance(logical_line, filename): yield (0, msg) +def check_assertequal_for_httpcode(logical_line, filename): + msg = ("N332: Use assertEqual(expected_http_code, observed_http_code) " + "instead of assertEqual(observed_http_code, expected_http_code)") + if 'neutron/tests/' in filename: + if re.search(r"assertEqual\(\s*[^,]*,[^,]*HTTP[^\.]*\.code\s*\)", + logical_line): + yield (0, msg) + + def factory(register): register(validate_log_translations) register(use_jsonutils) @@ -243,3 +252,4 @@ def factory(register): register(check_assertfalse) register(check_assertempty) register(check_assertisinstance) + register(check_assertequal_for_httpcode) diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py index b7f1e4b4471..183f7318580 100644 --- a/neutron/tests/unit/agent/test_securitygroups_rpc.py +++ b/neutron/tests/unit/agent/test_securitygroups_rpc.py @@ -224,7 +224,7 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase): rule2['security_group_rule']]} res = self._create_security_group_rule(self.fmt, rules) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPCreated.code) + self.assertEqual(webob.exc.HTTPCreated.code, res.status_int) res1 = self._create_port( self.fmt, n['network']['id'], @@ -396,7 +396,7 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase): rule2['security_group_rule']]} res = self._create_security_group_rule(self.fmt, rules) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPCreated.code) + self.assertEqual(webob.exc.HTTPCreated.code, res.status_int) res1 = self._create_port( self.fmt, n['network']['id'], @@ -446,7 +446,7 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase): 'security_group_rules': [rule1['security_group_rule']]} res = self._create_security_group_rule(self.fmt, rules) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPCreated.code) + self.assertEqual(webob.exc.HTTPCreated.code, res.status_int) res1 = self._create_port( self.fmt, n['network']['id'], @@ -567,7 +567,7 @@ class SGServerRpcCallBackTestCase(test_sg.SecurityGroupDBTestCase): rule2['security_group_rule']]} res = self._create_security_group_rule(self.fmt, rules) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPCreated.code) + self.assertEqual(webob.exc.HTTPCreated.code, res.status_int) dhcp_port = self._create_port( self.fmt, n['network']['id'], diff --git a/neutron/tests/unit/api/v2/test_resource.py b/neutron/tests/unit/api/v2/test_resource.py index 0b0764a621c..41faf77588e 100644 --- a/neutron/tests/unit/api/v2/test_resource.py +++ b/neutron/tests/unit/api/v2/test_resource.py @@ -215,7 +215,7 @@ class ResourceTestCase(base.BaseTestCase): 'format': 'json'})} res = resource.get('', extra_environ=environ, expect_errors=True) - self.assertEqual(res.status_int, exc.HTTPGatewayTimeout.code) + self.assertEqual(exc.HTTPGatewayTimeout.code, res.status_int) self.assertIn(msg_translation, str(wsgi.JSONDeserializer().deserialize(res.body))) 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 065ea48b7bf..cb4aa71d089 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -556,7 +556,7 @@ class NeutronDbPluginV2TestCase(testlib_api.WebTestCase): def _validate_behavior_on_bulk_success(self, res, collection, names=['test_0', 'test_1']): - self.assertEqual(res.status_int, webob.exc.HTTPCreated.code) + self.assertEqual(webob.exc.HTTPCreated.code, res.status_int) items = self.deserialize(self.fmt, res)[collection] self.assertEqual(len(items), 2) self.assertEqual(items[0]['name'], 'test_0') @@ -1674,8 +1674,8 @@ fixed_ips=ip_address%%3D%s&fixed_ips=ip_address%%3D%s&fixed_ips=subnet_id%%3D%s 'ip_address': '2607:f0d0:1002:51::5'}]} net_id = subnet['subnet']['network_id'] res = self._create_port(self.fmt, net_id=net_id, **kwargs) - self.assertEqual(res.status_int, - webob.exc.HTTPClientError.code) + self.assertEqual(webob.exc.HTTPClientError.code, + res.status_int) @mock.patch.object(non_ipam.IpamNonPluggableBackend, '_allocate_specific_ip') diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index d4e4e6d15f2..f83ea385701 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -667,7 +667,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): self.fmt, _uuid(), arg_list=('external_gateway_info',), set_context=True, external_gateway_info=ext_info ) - self.assertEqual(res.status_int, exc.HTTPForbidden.code) + self.assertEqual(exc.HTTPForbidden.code, res.status_int) def test_router_list(self): with self.router() as v1, self.router() as v2, self.router() as v3: diff --git a/neutron/tests/unit/extensions/test_securitygroup.py b/neutron/tests/unit/extensions/test_securitygroup.py index 25d90748491..56c5d21b5ea 100644 --- a/neutron/tests/unit/extensions/test_securitygroup.py +++ b/neutron/tests/unit/extensions/test_securitygroup.py @@ -849,7 +849,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): remote_group_id) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) def test_create_security_group_rule_bad_security_group_id(self): security_group_id = "4cd70774-cc67-4a87-9b39-7d1db38eb087" @@ -864,7 +864,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): remote_ip_prefix) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPNotFound.code) + self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int) def test_create_security_group_rule_bad_tenant(self): with self.security_group() as sg: @@ -880,7 +880,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): tenant_id='bad_tenant', set_context=True) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPNotFound.code) + self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int) def test_create_security_group_rule_bad_tenant_remote_group_id(self): with self.security_group() as sg: @@ -901,7 +901,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): tenant_id='bad_tenant', set_context=True) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPNotFound.code) + self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int) def test_create_security_group_rule_bad_tenant_security_group_rule(self): with self.security_group() as sg: @@ -921,7 +921,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): tenant_id='bad_tenant', set_context=True) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPNotFound.code) + self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int) def test_create_security_group_rule_bad_remote_group_id(self): name = 'webservers' @@ -939,7 +939,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): remote_group_id=remote_group_id) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPNotFound.code) + self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int) def test_create_security_group_rule_duplicate_rules(self): name = 'webservers' @@ -953,7 +953,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self._create_security_group_rule(self.fmt, rule) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPConflict.code) + self.assertEqual(webob.exc.HTTPConflict.code, res.status_int) def test_create_security_group_rule_min_port_greater_max(self): name = 'webservers' @@ -968,8 +968,8 @@ class TestSecurityGroups(SecurityGroupDBTestCase): 'ingress', protocol, '50', '22') res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, - webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, + res.status_int) def test_create_security_group_rule_ports_but_no_protocol(self): name = 'webservers' @@ -981,7 +981,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): sg['security_group']['id'], 'ingress', None, '22', '22') res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) def test_create_security_group_rule_port_range_min_only(self): name = 'webservers' @@ -994,7 +994,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): const.PROTO_NAME_TCP, '22', None) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) def test_create_security_group_rule_port_range_max_only(self): name = 'webservers' @@ -1007,7 +1007,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): const.PROTO_NAME_TCP, None, '22') res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) def test_create_security_group_rule_icmp_type_too_big(self): name = 'webservers' @@ -1020,7 +1020,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): const.PROTO_NAME_ICMP, '256', None) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) def test_create_security_group_rule_icmp_code_too_big(self): name = 'webservers' @@ -1033,7 +1033,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): const.PROTO_NAME_ICMP, '8', '256') res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) def test_create_security_group_rule_icmp_with_code_only(self): name = 'webservers' @@ -1047,8 +1047,8 @@ class TestSecurityGroups(SecurityGroupDBTestCase): const.PROTO_NAME_ICMP, None, code) res = self._create_security_group_rule(self.fmt, rule) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, - webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, + res.status_int) def test_list_ports_security_group(self): with self.network() as n: @@ -1527,7 +1527,7 @@ class TestSecurityGroups(SecurityGroupDBTestCase): security_groups=['not_valid']) self.deserialize(self.fmt, res) - self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) def test_create_security_group_rule_with_specific_id(self): neutron_context = context.Context('', 'test-tenant') diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index 91d9af3ae0f..c77c2d523ea 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -260,3 +260,17 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual( 0, len(list(checks.check_assertisinstance(pass_code2, "neutron/tests/test_assert.py")))) + + def test_assertequal_for_httpcode(self): + fail_code = """ + self.assertEqual(res.status_int, webob.exc.HTTPNoContent.code) + """ + pass_code = """ + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) + """ + self.assertEqual( + 1, len(list(checks.check_assertequal_for_httpcode(fail_code, + "neutron/tests/test_assert.py")))) + self.assertEqual( + 0, len(list(checks.check_assertequal_for_httpcode(pass_code, + "neutron/tests/test_assert.py"))))