Unify using assertIsInstance

Use assertIsInstance(A, B) to replace assertTrue(isinstance(A, B)).

Prefer specific assertions such as assert(Not)IsInstance over generic ones
(assertTrue/False, assertEqual) because they raise more meaningful errors.

Change-Id: I56278b1a74108e2765a8a740658f33954f5404c7
Closes-bug: #1268480

diff --git a/HACKING.rst b/HACKING.rst
index cc185ff4a5518ef0db71a144e548366b5758d994..b1ecb523b35497f2e3baa1dc355aeb0da56cc6a3 100644 (file)
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -21,6 +21,7 @@ Neutron Specific Commandments
 - [N329] Method's default argument shouldn't be mutable
 - [N330] Use assertEqual(*empty*, observed) instead of
          assertEqual(observed, *empty*)
+- [N331] Detect wrong usage with assertTrue(isinstance()).
 
 Creating Unit Tests
 -------------------

diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py
index 4254a648935ba94ef19474f15eacdf49872ab42b..3441b68a0d03ca02bc53f5e3d11b90a111f09bd5 100644 (file)
--- a/neutron/hacking/checks.py
+++ b/neutron/hacking/checks.py
@@ -219,6 +219,15 @@ def check_assertempty(logical_line, filename):
             yield (0, msg)
 
 
+def check_assertisinstance(logical_line, filename):
+    if 'neutron/tests/' in filename:
+        if re.search(r"assertTrue\(\s*isinstance\(\s*[^,]*,\s*[^,]*\)\)",
+                     logical_line):
+            msg = ("N331: Use assertIsInstance(observed, type) instead "
+                   "of assertTrue(isinstance(observed, type))")
+            yield (0, msg)
+
+
 def factory(register):
     register(validate_log_translations)
     register(use_jsonutils)
@@ -233,3 +242,4 @@ def factory(register):
     register(no_mutable_default_args)
     register(check_assertfalse)
     register(check_assertempty)
+    register(check_assertisinstance)

diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py
index be65bd7e9e080bcfd5894244647819e3dc2c0393..91d9af3ae0fbe0f45ddc168bdb5384ffe7707af4 100644 (file)
--- a/neutron/tests/unit/hacking/test_checks.py
+++ b/neutron/tests/unit/hacking/test_checks.py
@@ -240,3 +240,23 @@ class HackingTestCase(base.BaseTestCase):
             self.assertEqual(
                 0, len(list(checks.check_assertfalse(pass_code2 % ec,
                                             "neutron/tests/test_assert.py"))))
+
+    def test_assertisinstance(self):
+        fail_code = """
+               self.assertTrue(isinstance(observed, ANY_TYPE))
+               """
+        pass_code1 = """
+               self.assertEqual(ANY_TYPE, type(observed))
+               """
+        pass_code2 = """
+               self.assertIsInstance(observed, ANY_TYPE)
+               """
+        self.assertEqual(
+            1, len(list(checks.check_assertisinstance(fail_code,
+                                        "neutron/tests/test_assert.py"))))
+        self.assertEqual(
+            0, len(list(checks.check_assertisinstance(pass_code1,
+                                            "neutron/tests/test_assert.py"))))
+        self.assertEqual(
+            0, len(list(checks.check_assertisinstance(pass_code2,
+                                            "neutron/tests/test_assert.py"))))