diff --git a/HACKING.rst b/HACKING.rst index 48e4ed6b8f0..0d710b51b69 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -31,6 +31,7 @@ Neutron Specific Commandments - [N343] Production code must not import from neutron.tests.* - [N344] Python 3: Do not use filter(lambda obj: test(obj), data). Replace it with [obj for obj in data if test(obj)]. +- [N345] Detect wrong usage with assertEqual(None, A) and assertEqual(A, None). Creating Unit Tests ------------------- diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index b83767af785..b01ef849521 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -389,6 +389,19 @@ def check_python3_no_filter(logical_line): yield(0, msg) +@flake8ext +def check_assertIsNone(logical_line, filename): + """N345 - Enforce using assertIsNone.""" + if 'neutron/tests/' in filename: + asse_eq_end_with_none_re = re.compile(r"assertEqual\(.*?,\s+None\)$") + asse_eq_start_with_none_re = re.compile(r"assertEqual\(None,") + res = (asse_eq_start_with_none_re.search(logical_line) or + asse_eq_end_with_none_re.search(logical_line)) + if res: + yield (0, "N345: assertEqual(A, None) or assertEqual(None, A) " + "sentences not allowed") + + def factory(register): register(validate_log_translations) register(use_jsonutils) @@ -409,3 +422,4 @@ def factory(register): register(check_unittest_imports) register(check_no_imports_from_tests) register(check_python3_no_filter) + register(check_assertIsNone) diff --git a/neutron/tests/tempest/api/test_floating_ips.py b/neutron/tests/tempest/api/test_floating_ips.py index bafa54c4b29..6e722dbade2 100644 --- a/neutron/tests/tempest/api/test_floating_ips.py +++ b/neutron/tests/tempest/api/test_floating_ips.py @@ -100,4 +100,4 @@ class FloatingIPTestJSON(base.BaseNetworkTest): # disassociate body = self.client.update_floatingip(body['floatingip']['id'], port_id=None) - self.assertEqual(None, body['floatingip']['port_id']) + self.assertIsNone(body['floatingip']['port_id']) diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index 7694d3623dd..834f38b086f 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -766,8 +766,7 @@ class L3HATestCase(L3HATestFramework): def test_get_active_host_for_ha_router(self): router = self._create_router() - self.assertEqual( - None, + self.assertIsNone( self.plugin.get_active_host_for_ha_router( self.admin_ctx, router['id'])) self.plugin.update_routers_states( diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index c891633b124..649c8cb7d48 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -327,6 +327,16 @@ class HackingTestCase(base.BaseTestCase): self.assertLinePasses(f, "filter(function, range(0,10))") self.assertLinePasses(f, "lambda x, y: x+y") + def test_check_assertIsNone(self): + self.assertEqual(1, len(list(checks.check_assertIsNone( + "self.assertEqual(A, None)", "neutron/tests/test_assert.py")))) + + self.assertEqual(1, len(list(checks.check_assertIsNone( + "self.assertEqual(None, A)", "neutron/tests/test_assert.py")))) + + self.assertEqual(0, len(list(checks.check_assertIsNone( + "self.assertIsNone()", "neutron/tests/test_assert.py")))) + # The following is borrowed from hacking/tests/test_doctest.py. # Tests defined in docstring is easier to understand