From: lzklibj <lzklibj@cn.ibm.com>
Date: Wed, 6 Jan 2016 03:27:34 +0000 (+0800)
Subject: Unify using assertIsInstance
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d2a1d6fcb7e746a5340db0990a8f5d0fc786738d;p=openstack-build%2Fneutron-build.git

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 cc185ff4a..b1ecb523b 100644
--- 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 4254a6489..3441b68a0 100644
--- 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 be65bd7e9..91d9af3ae 100644
--- 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"))))