From 16774b6d33bda4fe356d14619c843e1d55722e40 Mon Sep 17 00:00:00 2001
From: Sergey Nikitin <snikitin@mirantis.com>
Date: Thu, 23 Oct 2014 16:32:33 +0400
Subject: [PATCH] Added hacking rule for assertEqual(a in b, True/False).

The following replacements were done in tests to have
clearer messages in case of failure:
- assertEqual(a in b, True) with assertIn(a, b)
- assertEqual(a in b, False) with assertNotIn(a, b)

The error message would now be like:
   'abc' not in ['a', 'b', 'c']
rather than:
   MismatchError: False != True

Change-Id: I514daca3a470feef5d332a1a319cba15256fc6ea
---
 HACKING.rst                         |  3 +++
 nova/hacking/checks.py              | 19 +++++++++++++++
 nova/tests/unit/test_hacking.py     | 37 +++++++++++++++++++++++++++++
 nova/tests/unit/test_nova_manage.py |  2 +-
 4 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/HACKING.rst b/HACKING.rst
index d8850bab6fe6..234acd4f05fe 100644
--- a/HACKING.rst
+++ b/HACKING.rst
@@ -49,6 +49,9 @@ Nova Specific Commandments
 - [N335] Check for usage of deprecated assertRaisesRegexp
 - [N336] Must use a dict comprehension instead of a dict constructor with a sequence of key-value pairs.
 - [N337] Don't import translation in tests
+- [N338] Change assertEqual(A in B, True), assertEqual(True, A in B),
+  assertEqual(A in B, False) or assertEqual(False, A in B) to the more specific
+  assertIn/NotIn(A, B)
 
 Creating Unit Tests
 -------------------
diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py
index 86f57cda4634..54a060d785a4 100644
--- a/nova/hacking/checks.py
+++ b/nova/hacking/checks.py
@@ -50,6 +50,10 @@ asse_trueinst_re = re.compile(
 asse_equal_type_re = re.compile(
                        r"(.)*assertEqual\(type\((\w|\.|\'|\"|\[|\])+\), "
                        "(\w|\.|\'|\"|\[|\])+\)")
+asse_equal_in_end_with_true_or_false_re = re.compile(r"assertEqual\("
+                    r"(\w|[][.'\"])+ in (\w|[][.'\", ])+, (True|False)\)")
+asse_equal_in_start_with_true_or_false_re = re.compile(r"assertEqual\("
+                    r"(True|False), (\w|[][.'\"])+ in (\w|[][.'\", ])+\)")
 asse_equal_end_with_none_re = re.compile(
                            r"assertEqual\(.*?,\s+None\)$")
 asse_equal_start_with_none_re = re.compile(
@@ -511,6 +515,20 @@ def dict_constructor_with_list_copy(logical_line):
         yield (0, msg)
 
 
+def assert_equal_in(logical_line):
+    """Check for assertEqual(A in B, True), assertEqual(True, A in B),
+    assertEqual(A in B, False) or assertEqual(False, A in B) sentences
+
+    N338
+    """
+    res = (asse_equal_in_start_with_true_or_false_re.search(logical_line) or
+           asse_equal_in_end_with_true_or_false_re.search(logical_line))
+    if res:
+        yield (0, "N338: Use assertIn/NotIn(A, B) rather than "
+                  "assertEqual(A in B, True/False) when checking collection "
+                  "contents.")
+
+
 def factory(register):
     register(import_no_db_in_virt)
     register(no_db_session_in_public_api)
@@ -536,3 +554,4 @@ def factory(register):
     register(check_oslo_namespace_imports)
     register(assert_true_or_false_with_in)
     register(dict_constructor_with_list_copy)
+    register(assert_equal_in)
diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py
index 78b994cccff4..494934184e6a 100644
--- a/nova/tests/unit/test_hacking.py
+++ b/nova/tests/unit/test_hacking.py
@@ -121,6 +121,43 @@ class HackingTestCase(test.NoDBTestCase):
         self.assertEqual(
             len(list(checks.assert_equal_type("self.assertTrue()"))), 0)
 
+    def test_assert_equal_in(self):
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(a in b, True)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual('str' in 'string', True)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(any(a==1 for a in b), True)"))), 0)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(True, a in b)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(True, 'str' in 'string')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(True, any(a==1 for a in b))"))), 0)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(a in b, False)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual('str' in 'string', False)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(any(a==1 for a in b), False)"))), 0)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(False, a in b)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(False, 'str' in 'string')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_in(
+            "self.assertEqual(False, any(a==1 for a in b))"))), 0)
+
     def test_assert_equal_none(self):
         self.assertEqual(len(list(checks.assert_equal_none(
             "self.assertEqual(A, None)"))), 1)
diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py
index c3321866dcf8..a53019e6c5fa 100644
--- a/nova/tests/unit/test_nova_manage.py
+++ b/nova/tests/unit/test_nova_manage.py
@@ -322,7 +322,7 @@ class ProjectCommandsTestCase(test.TestCase):
         sys.stdout = sys.__stdout__
         result = output.getvalue()
         print_format = "%-36s %-10s" % ('instances', 'unlimited')
-        self.assertEqual((print_format in result), True)
+        self.assertIn(print_format, result)
 
     def test_quota_update_invalid_key(self):
         self.assertEqual(2, self.commands.quota('admin', 'volumes1', '10'))