From f276a96d84ee5250c134599c129ea6a9e8a1d3a6 Mon Sep 17 00:00:00 2001 From: gengchc2 Date: Thu, 22 Sep 2016 16:09:15 +0800 Subject: [PATCH] Replace assertEqual(None, *) with assertIsNone in tests Replace assertEqual(None, *) with assertIsNone in tests to have more clear messages in case of failure. Change-Id: Ice018dfd3e48326ce9cac58342d03979559b9e82 --- HACKING.rst | 1 + neutron/hacking/checks.py | 14 ++++++++++++++ neutron/tests/tempest/api/test_floating_ips.py | 2 +- neutron/tests/unit/db/test_l3_hamode_db.py | 3 +-- neutron/tests/unit/hacking/test_checks.py | 10 ++++++++++ 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 80d58701c64..058dc84162d 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -32,6 +32,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 a253a15c1a6..820cac7312d 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -414,6 +414,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) @@ -435,3 +448,4 @@ def factory(register): register(check_delayed_string_interpolation) 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 02f983d5274..9f1832f14ed 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -756,8 +756,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 536f58716c8..7a101295b4e 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -360,6 +360,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