Merge "Enable some off-by-default checks"

HACKING.rst

@@ -24,7 +24,6 @@ Cinder Specific Commandments
 - [C309] Unit tests should not perform logging.
 - [C310] Check for improper use of logging format arguments.
 - [C311] Check for proper naming and usage in option registration.
-- [C312] Check that assertIsNone(value) is used and not assertEqual(None, value).
 - [C313] Check that assertTrue(value) is used and not assertEqual(True, value).
 
 General

cinder/hacking/checks.py

@@ -448,13 +448,6 @@ def no_test_log(logical_line, filename, noqa):
         yield (0, msg)
 
 
-def validate_assertIsNone(logical_line):
-    if re.match(assert_None, logical_line):
-        msg = ("C312: Unit tests should use assertIsNone(value) instead"
-               " of using assertEqual(None, value).")
-        yield(0, msg)
-
-
 def validate_assertTrue(logical_line):
     if re.match(assert_True, logical_line):
         msg = ("C313: Unit tests should use assertTrue(value) instead"
@@ -479,5 +472,4 @@ def factory(register):
     register(no_log_warn)
     register(dict_constructor_with_list_copy)
     register(no_test_log)
-    register(validate_assertIsNone)
     register(validate_assertTrue)

cinder/tests/unit/objects/test_base.py

@@ -179,7 +179,7 @@ class TestCinderComparableObject(test_objects.BaseObjectsTestCase):
         self.assertTrue(obj1 == obj2)
         self.assertFalse(obj1 == obj3)
         self.assertFalse(obj1 == obj4)
-        self.assertNotEqual(obj1, None)
+        self.assertIsNotNone(obj1)
 
 
 @ddt.ddt

cinder/tests/unit/test_hacking.py

@@ -361,13 +361,6 @@ class HackingTestCase(test.TestCase):
         self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy(
             "      self._render_dict(xml, data_el, data.__dict__)"))))
 
-    def test_validate_assertIsNone(self):
-        test_value = None
-        self.assertEqual(0, len(list(checks.validate_assertIsNone(
-            "assertIsNone(None)"))))
-        self.assertEqual(1, len(list(checks.validate_assertIsNone(
-            "assertEqual(None, %s)" % test_value))))
-
     def test_validate_assertTrue(self):
         test_value = True
         self.assertEqual(0, len(list(checks.validate_assertTrue(

cinder/tests/unit/test_quota.py

@@ -1827,7 +1827,7 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
     def _mock_allocated_get_all_by_project(self, allocated_quota=False):
         def fake_qagabp(context, project_id, session=None):
             self.assertEqual('test_project', project_id)
-            self.assertNotEqual(session, None)
+            self.assertIsNotNone(session)
             if allocated_quota:
                 return dict(project_id=project_id, volumes=3,
                             gigabytes = 2 * 1024)

cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_7mode.py

@@ -533,8 +533,8 @@ class NetApp7modeClientTestCase(test.TestCase):
         self.assertEqual(
             fake.CG_SNAPSHOT_ID,
             actual_request.get_child_by_name('snapshot-name').get_content())
-        self.assertEqual(actual_request.get_child_by_name(
+        self.assertIsNone(actual_request.get_child_by_name(
-            'destination-exists'), None)
+            'destination-exists'))
         self.assertTrue(enable_tunneling)
 
     def test_clone_file_when_clone_fails(self):
@@ -589,8 +589,8 @@ class NetApp7modeClientTestCase(test.TestCase):
 
         self.assertEqual(expected_src_path, actual_src_path)
         self.assertEqual(expected_dest_path, actual_dest_path)
-        self.assertEqual(actual_request.get_child_by_name(
+        self.assertIsNone(actual_request.get_child_by_name(
-            'destination-exists'), None)
+            'destination-exists'))
         self.assertTrue(enable_tunneling)
 
         __, _args, _kwargs = self.connection.invoke_successfully.mock_calls[1]

cinder/tests/unit/volume/drivers/netapp/dataontap/client/test_client_cmode.py

@@ -1146,8 +1146,8 @@ class NetAppCmodeClientTestCase(test.TestCase):
         req_snapshot_child = actual_request.get_child_by_name('snapshot-name')
         self.assertEqual(fake.CG_SNAPSHOT_ID, req_snapshot_child.get_content())
 
-        self.assertEqual(actual_request.get_child_by_name(
+        self.assertIsNone(actual_request.get_child_by_name(
-            'destination-exists'), None)
+            'destination-exists'))
 
     def test_clone_file_when_destination_exists(self):
         expected_flex_vol = "fake_flex_vol"

tox.ini

@@ -129,6 +129,7 @@ usedevelop = False
 # E251 unexpected spaces around keyword / parameter equals
 # reason: no improvement in readability
 ignore = E251
+enable-extensions = H106,H203
 exclude = .git,.venv,.tox,dist,tools,doc/ext,*egg,build
 max-complexity=30