Add several new hacking rules

Rules:
 N311 - Validate that debug level logs are not translated
 N320 - Ensure that ``assertTrue(isinstance(A, B))``  is not used
 N321 - Ensure that ``assertEqual(type(A), B)`` is not used
 N322 - Ensure that ``assertEqual(A, None)`` and ``assertEqual(None, A)``
        are not used
 N323 - Ensure that ``assertTrue/assertFalse(A in/not in B)`` are not
        used with collection contents

Also, small bug related to logging in verification was fixed.

Change-Id: Ieabe1b23d0c02cc13bb6cad106ab855507359ddc

rally/verification/verifiers/tempest/tempest.py

@@ -19,7 +19,6 @@ import shutil
 import subprocess
 import sys
 
-from oslo.config import cfg
 from oslo.serialization import jsonutils
 
 from rally import exceptions
@@ -45,8 +44,7 @@ def check_output(*args, **kwargs):
         LOG.debug("error output: '%s'" % e.output)
         raise
 
-    if cfg.CONF.rally_debug:
+    LOG.debug(output)
-        print(output)
 
 
 class Tempest(object):

tests/hacking/README.rst

@@ -7,8 +7,15 @@ Rally Style Commandments
 
 Rally Specific Commandments
 ---------------------------
-
+* [N30x] - Reserved for rules related to ``mock`` library
-- [N301] Ensure that ``assert_*`` methods from ``mock`` library is used correctly
+ * [N301] - Ensure that ``assert_*`` methods from ``mock`` library is used correctly
-- [N302] Sub-error of N301, related to nonexistent "assert_called"
+ * [N302] - Ensure that nonexistent "assert_called" is not used
-- [N303] Sub-error of N301, related to nonexistent "assert_called_once"
+ * [N303] - Ensure that  nonexistent "assert_called_once" is not used
-- [N310] Ensure that ``rally.log`` is used instead of ``rally.openstack.common.log``
+* [N310-N314] - Reserved for rules related to logging
+ * [N310] - Ensure that ``rally.log`` is used instead of ``rally.openstack.common.log``
+ * [N311] - Validate that debug level logs are not translated
+* [N32x] - Reserved for rules related to assert* methods
+ * [N320] - Ensure that ``assertTrue(isinstance(A, B))``  is not used
+ * [N321] - Ensure that ``assertEqual(type(A), B)`` is not used
+ * [N322] - Ensure that ``assertEqual(A, None)`` and ``assertEqual(None, A)`` are not used
+ * [N323] - Ensure that ``assertTrue/assertFalse(A in/not in B)`` are not used with collection contents

tests/hacking/checks.py

@@ -24,6 +24,24 @@ Guidelines for writing new hacking checks
 
 """
 
+import re
+
+
+re_assert_true_instance = re.compile(
+    r"(.)*assertTrue\(isinstance\((\w|\.|\'|\"|\[|\])+, "
+    r"(\w|\.|\'|\"|\[|\])+\)\)")
+re_assert_equal_type = re.compile(
+    r"(.)*assertEqual\(type\((\w|\.|\'|\"|\[|\])+\), "
+    r"(\w|\.|\'|\"|\[|\])+\)")
+re_assert_equal_end_with_none = re.compile(r"assertEqual\(.*?,\s+None\)$")
+re_assert_equal_start_with_none = re.compile(r"assertEqual\(None,")
+re_assert_true_false_with_in_or_not_in = re.compile(
+    r"assert(True|False)\("
+    r"(\w|[][.'\"])+( not)? in (\w|[][.'\",])+(, .*)?\)")
+re_assert_true_false_with_in_or_not_in_spaces = re.compile(
+    r"assert(True|False)\((\w|[][.'\"])+( not)? in [\[|'|\"](\w|[][.'\", ])+"
+    r"[\[|'|\"](, .*)?\)")
+
 
 def _parse_assert_mock_str(line):
     point = line.find('.assert_')
@@ -45,8 +63,9 @@ def check_assert_methods_from_mock(logical_line, filename):
 
     correct_names = ["assert_any_call", "assert_called_once_with",
                      "assert_called_with", "assert_has_calls"]
+    ignored_files = ["./tests/unit/test_hacking.py"]
 
-    if 'tests/' in filename:
+    if filename.startswith("./tests") and filename not in ignored_files:
         pos, method_name, obj_name = _parse_assert_mock_str(logical_line)
 
         if pos:
@@ -80,7 +99,10 @@ def check_assert_methods_from_mock(logical_line, filename):
 
 
 def check_import_of_logging(logical_line, filename):
-    """Check correctness import of logging module N310."""
+    """Check correctness import of logging module
+
+    N310
+    """
 
     excluded_files = ["./rally/log.py", "./tests/unit/test_log.py"]
 
@@ -95,6 +117,78 @@ def check_import_of_logging(logical_line, filename):
                           "use `rally.log` instead.")
 
 
+def no_translate_debug_logs(logical_line):
+    """Check for 'LOG.debug(_('
+
+    As per our translation policy,
+    https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
+    we shouldn't translate debug level logs.
+
+    * This check assumes that 'LOG' is a logger.
+    * Use filename so we can start enforcing this in specific folders instead
+      of needing to do so all at once.
+
+    N311
+    """
+    if logical_line.startswith("LOG.debug(_("):
+        yield(0, "N311 Don't translate debug level logs")
+
+
+def assert_true_instance(logical_line):
+    """Check for assertTrue(isinstance(a, b)) sentences
+
+    N320
+    """
+    if re_assert_true_instance.match(logical_line):
+        yield (0, "N320 assertTrue(isinstance(a, b)) sentences not allowed, "
+                  "you should use assertIsInstance(a, b) instead.")
+
+
+def assert_equal_type(logical_line):
+    """Check for assertEqual(type(A), B) sentences
+
+    N321
+    """
+    if re_assert_equal_type.match(logical_line):
+        yield (0, "N321 assertEqual(type(A), B) sentences not allowed, "
+                  "you should use assertIsInstance(a, b) instead.")
+
+
+def assert_equal_none(logical_line):
+    """Check for assertEqual(A, None) or assertEqual(None, A) sentences
+
+    N322
+    """
+    res = (re_assert_equal_start_with_none.search(logical_line) or
+           re_assert_equal_end_with_none.search(logical_line))
+    if res:
+        yield (0, "N322 assertEqual(A, None) or assertEqual(None, A) "
+                  "sentences not allowed, you should use assertIsNone(A) "
+                  "instead.")
+
+
+def assert_true_or_false_with_in(logical_line):
+    """Check assertTrue/False(A in/not in B) with collection contents
+
+    Check for assertTrue/False(A in B), assertTrue/False(A not in B),
+    assertTrue/False(A in B, message) or assertTrue/False(A not in B, message)
+    sentences.
+
+    N323
+    """
+    res = (re_assert_true_false_with_in_or_not_in.search(logical_line) or
+           re_assert_true_false_with_in_or_not_in_spaces.search(logical_line))
+    if res:
+        yield (0, "N323 assertTrue/assertFalse(A in/not in B)sentences not "
+                  "allowed, you should use assertIn(A, B) or assertNotIn(A, B)"
+                  " instead.")
+
+
 def factory(register):
     register(check_assert_methods_from_mock)
     register(check_import_of_logging)
+    register(no_translate_debug_logs)
+    register(assert_true_instance)
+    register(assert_equal_type)
+    register(assert_equal_none)
+    register(assert_true_or_false_with_in)

tests/unit/benchmark/processing/test_utils.py

@@ -27,7 +27,7 @@ class MathTestCase(test.TestCase):
 
     def test_percentile_value_none(self):
         result = utils.percentile(None, 0.1)
-        self.assertEqual(result, None)
+        self.assertIsNone(result)
 
     def test_percentile_equal(self):
         lst = range(1, 101)

tests/unit/benchmark/scenarios/test_base.py

@@ -278,12 +278,10 @@ class ScenarioTestCase(test.TestCase):
                                 "Scenario `%s` has wrong context" % scenario)
 
     def test_RESOURCE_NAME_PREFIX(self):
-        self.assertTrue(isinstance(base.Scenario.RESOURCE_NAME_PREFIX,
+        self.assertIsInstance(base.Scenario.RESOURCE_NAME_PREFIX, basestring)
-                                   basestring))
 
     def test_RESOURCE_NAME_LENGTH(self):
-        self.assertTrue(
+        self.assertIsInstance(base.Scenario.RESOURCE_NAME_LENGTH, int)
-            isinstance(base.Scenario.RESOURCE_NAME_LENGTH, int))
         self.assertTrue(base.Scenario.RESOURCE_NAME_LENGTH > 4)
 
     @mock.patch(

tests/unit/cmd/commands/test_info.py

@@ -45,7 +45,7 @@ class InfoCommandsTestCase(test.TestCase):
         query = "Dummy"
         status = self.info.find(query)
         mock_get_by_name.assert_called_once_with(query)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(SCENARIO + ".get_scenario_by_name",
                 return_value=dummy.Dummy.dummy)
@@ -53,7 +53,7 @@ class InfoCommandsTestCase(test.TestCase):
         query = "Dummy.dummy"
         status = self.info.find(query)
         mock_get_scenario_by_name.assert_called_once_with(query)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(SCENARIO + ".get_scenario_by_name",
                 side_effect=exceptions.NoSuchScenario)
@@ -68,7 +68,7 @@ class InfoCommandsTestCase(test.TestCase):
         query = "FailureRate"
         status = self.info.find(query)
         mock_get_by_name.assert_called_once_with(query)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(ENGINE + ".get_by_name",
                 return_value=existing_cloud.ExistingCloud)
@@ -76,7 +76,7 @@ class InfoCommandsTestCase(test.TestCase):
         query = "ExistingCloud"
         status = self.info.find(query)
         mock_get_by_name.assert_called_once_with(query)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(PROVIDER + ".get_by_name",
                 return_value=existing_servers.ExistingServers)
@@ -84,7 +84,7 @@ class InfoCommandsTestCase(test.TestCase):
         query = "ExistingServers"
         status = self.info.find(query)
         mock_get_by_name.assert_called_once_with(query)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(UTILS + ".itersubclasses", return_value=[dummy.Dummy])
     def test_list(self, mock_itersubclasses):
@@ -94,29 +94,29 @@ class InfoCommandsTestCase(test.TestCase):
             mock.call(sla_base.SLA),
             mock.call(deploy.EngineFactory),
             mock.call(serverprovider.ProviderFactory)])
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(UTILS + ".itersubclasses", return_value=[dummy.Dummy])
     def test_BenchmarkScenarios(self, mock_itersubclasses):
         status = self.info.BenchmarkScenarios()
         mock_itersubclasses.assert_called_once_with(scenario_base.Scenario)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(UTILS + ".itersubclasses", return_value=[dummy.Dummy])
     def test_SLA(self, mock_itersubclasses):
         status = self.info.SLA()
         mock_itersubclasses.assert_called_once_with(sla_base.SLA)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(UTILS + ".itersubclasses", return_value=[dummy.Dummy])
     def test_DeployEngines(self, mock_itersubclasses):
         status = self.info.DeployEngines()
         mock_itersubclasses.assert_called_once_with(deploy.EngineFactory)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)
 
     @mock.patch(UTILS + ".itersubclasses", return_value=[dummy.Dummy])
     def test_ServerProviders(self, mock_itersubclasses):
         status = self.info.ServerProviders()
         mock_itersubclasses.assert_called_once_with(
                                             serverprovider.ProviderFactory)
-        self.assertEqual(None, status)
+        self.assertIsNone(status)

tests/unit/cmd/test_envutils.py

@@ -57,7 +57,7 @@ class EnvUtilsTestCase(test.TestCase):
     @mock.patch.dict(os.environ, values={}, clear=True)
     @mock.patch('rally.cmd.envutils.fileutils.load_env_file')
     def test_get_deployment_id_with_none(self, mock_file):
-        self.assertEqual(None, envutils.get_global(envutils.ENV_DEPLOYMENT))
+        self.assertIsNone(envutils.get_global(envutils.ENV_DEPLOYMENT))
         mock_file.assert_called_once_with(os.path.expanduser(
             '~/.rally/globals'))
 
@@ -77,7 +77,7 @@ class EnvUtilsTestCase(test.TestCase):
     @mock.patch.dict(os.environ, values={}, clear=True)
     @mock.patch('rally.cmd.envutils.fileutils.load_env_file')
     def test_get_task_id_with_none(self, mock_file):
-        self.assertEqual(None, envutils.get_global('RALLY_TASK'))
+        self.assertIsNone(envutils.get_global('RALLY_TASK'))
         mock_file.assert_called_once_with(os.path.expanduser(
             '~/.rally/globals'))
 

tests/unit/deploy/serverprovider/providers/test_openstack.py

@@ -93,7 +93,7 @@ class OpenStackProviderTestCase(test.TestCase):
         mock_clients.return_value.glance.side_effect = KeyError('image')
         cfg = self._get_valid_config()
         provider = OSProvider(mock.MagicMock(), cfg)
-        self.assertEqual(provider.glance, None)
+        self.assertIsNone(provider.glance)
 
     @mock.patch("rally.deploy.serverprovider.providers.openstack.osclients")
     def test_openstack_provider_init_with_invalid_conf_no_user(self,

tests/unit/deploy/serverprovider/providers/test_virsh.py

@@ -63,7 +63,7 @@ class VirshProviderTestCase(test.TestCase):
         mock_ipaddress.assert_called_once_with('10.0.0.1')
         self.assertEqual(server.host, '10.0.0.2')
         self.assertEqual(server.user, 'user')
-        self.assertEqual(server.key, None)
+        self.assertIsNone(server.key)
         self.assertEqual(server.password, 'password')
         self.provider.resources.create.assert_called_once_with({
             'name': 'name',

tests/unit/test_hacking.py

@@ -35,13 +35,13 @@ class HackingTestCase(test.TestCase):
         for name in correct_method_names:
             self.assertEqual(0, len(
                 list(checks.check_assert_methods_from_mock(
-                    'some_mock.%s(asd)' % name, 'tests/fake/test'))))
+                    'some_mock.%s(asd)' % name, './tests/fake/test'))))
 
     def test_wrong_usage_of_broad_assert_from_mock(self):
         fake_method = 'rtfm.assert_something()'
 
         actual_number, actual_msg = next(checks.check_assert_methods_from_mock(
-            fake_method, 'tests/fake/test'))
+            fake_method, './tests/fake/test'))
         self.assertEqual(4, actual_number)
         self.assertTrue(actual_msg.startswith('N301'))
 
@@ -49,7 +49,7 @@ class HackingTestCase(test.TestCase):
         fake_method = 'rtfm.assert_called()'
 
         actual_number, actual_msg = next(checks.check_assert_methods_from_mock(
-            fake_method, 'tests/fake/test'))
+            fake_method, './tests/fake/test'))
         self.assertEqual(4, actual_number)
         self.assertTrue(actual_msg.startswith('N302'))
 
@@ -57,7 +57,7 @@ class HackingTestCase(test.TestCase):
         fake_method = 'rtfm.assert_called_once()'
 
         actual_number, actual_msg = next(checks.check_assert_methods_from_mock(
-            fake_method, 'tests/fake/test'))
+            fake_method, './tests/fake/test'))
         self.assertEqual(4, actual_number)
         self.assertTrue(actual_msg.startswith('N303'))
 
@@ -82,3 +82,84 @@ class HackingTestCase(test.TestCase):
             checkres = checks.check_import_of_logging(fake_import,
                                                       "fakefile")
             self.assertEqual([], list(checkres))
+
+    def test_no_translate_debug_logs(self):
+        self.assertEqual(len(list(checks.no_translate_debug_logs(
+            "LOG.debug(_('foo'))"))), 1)
+
+        self.assertEqual(len(list(checks.no_translate_debug_logs(
+            "LOG.debug('foo')"))), 0)
+
+        self.assertEqual(len(list(checks.no_translate_debug_logs(
+            "LOG.info(_('foo'))"))), 0)
+
+    def test_assert_true_instance(self):
+        self.assertEqual(len(list(checks.assert_true_instance(
+            "self.assertTrue(isinstance(e, "
+            "exception.BuildAbortException))"))), 1)
+
+        self.assertEqual(
+            len(list(checks.assert_true_instance("self.assertTrue()"))), 0)
+
+    def test_assert_equal_type(self):
+        self.assertEqual(len(list(checks.assert_equal_type(
+            "self.assertEqual(type(als['QuicAssist']), list)"))), 1)
+
+        self.assertEqual(
+            len(list(checks.assert_equal_type("self.assertTrue()"))), 0)
+
+    def test_assert_equal_none(self):
+        self.assertEqual(len(list(checks.assert_equal_none(
+            "self.assertEqual(A, None)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_equal_none(
+            "self.assertEqual(None, A)"))), 1)
+
+        self.assertEqual(
+            len(list(checks.assert_equal_none("self.assertIsNone()"))), 0)
+
+    def test_assert_true_or_false_with_in_or_not_in(self):
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A in B)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertFalse(A in B)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A not in B)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertFalse(A not in B)"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A in B, 'some message')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertFalse(A in B, 'some message')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A not in B, 'some message')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertFalse(A not in B, 'some message')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A in 'some string with spaces')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A in 'some string with spaces')"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A in ['1', '2', '3'])"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(A in [1, 2, 3])"))), 1)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(any(A > 5 for A in B))"))), 0)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertTrue(any(A > 5 for A in B), 'some message')"))), 0)
+
+        self.assertEqual(len(list(checks.assert_true_or_false_with_in(
+            "self.assertFalse(some in list1 and some2 in list2)"))), 0)

tests/unit/test_osclients.py

@@ -49,7 +49,7 @@ class OSClientsTestCase(test.TestCase):
         super(OSClientsTestCase, self).tearDown()
 
     def test_keystone(self):
-        self.assertTrue("keystone" not in self.clients.cache)
+        self.assertNotIn("keystone", self.clients.cache)
         client = self.clients.keystone()
         self.assertEqual(client, self.fake_keystone)
         mgmt_url = urlparse.urlparse(self.endpoint.auth_url)
@@ -89,7 +89,7 @@ class OSClientsTestCase(test.TestCase):
         with mock.patch("rally.osclients.nova") as mock_nova:
             fake_nova = fakes.FakeNovaClient()
             mock_nova.Client = mock.MagicMock(return_value=fake_nova)
-            self.assertTrue("nova" not in self.clients.cache)
+            self.assertNotIn("nova", self.clients.cache)
             client = self.clients.nova()
             self.assertEqual(client, fake_nova)
             self.service_catalog.url_for.assert_called_once_with(
@@ -110,7 +110,7 @@ class OSClientsTestCase(test.TestCase):
     def test_neutron(self, mock_neutron):
         fake_neutron = fakes.FakeNeutronClient()
         mock_neutron.Client = mock.MagicMock(return_value=fake_neutron)
-        self.assertTrue("neutron" not in self.clients.cache)
+        self.assertNotIn("neutron", self.clients.cache)
         client = self.clients.neutron()
         self.assertEqual(client, fake_neutron)
         kw = {
@@ -130,7 +130,7 @@ class OSClientsTestCase(test.TestCase):
         with mock.patch("rally.osclients.glance") as mock_glance:
             fake_glance = fakes.FakeGlanceClient()
             mock_glance.Client = mock.MagicMock(return_value=fake_glance)
-            self.assertTrue("glance" not in self.clients.cache)
+            self.assertNotIn("glance", self.clients.cache)
             client = self.clients.glance()
             self.assertEqual(client, fake_glance)
             kw = {"endpoint": self.service_catalog.url_for.return_value,
@@ -149,7 +149,7 @@ class OSClientsTestCase(test.TestCase):
             fake_cinder = fakes.FakeCinderClient()
             fake_cinder.client = mock.MagicMock()
             mock_cinder.Client = mock.MagicMock(return_value=fake_cinder)
-            self.assertTrue("cinder" not in self.clients.cache)
+            self.assertNotIn("cinder", self.clients.cache)
             client = self.clients.cinder()
             self.assertEqual(client, fake_cinder)
             self.service_catalog.url_for.assert_called_once_with(
@@ -171,7 +171,7 @@ class OSClientsTestCase(test.TestCase):
             fake_ceilometer = fakes.FakeCeilometerClient()
             mock_ceilometer.Client = mock.MagicMock(
                 return_value=fake_ceilometer)
-            self.assertTrue("ceilometer" not in self.clients.cache)
+            self.assertNotIn("ceilometer", self.clients.cache)
             client = self.clients.ceilometer()
             self.assertEqual(client, fake_ceilometer)
             self.service_catalog.url_for.assert_called_once_with(
@@ -190,7 +190,7 @@ class OSClientsTestCase(test.TestCase):
     def test_ironic(self, mock_ironic):
         fake_ironic = fakes.FakeIronicClient()
         mock_ironic.get_client = mock.MagicMock(return_value=fake_ironic)
-        self.assertTrue("ironic" not in self.clients.cache)
+        self.assertNotIn("ironic", self.clients.cache)
         client = self.clients.ironic()
         self.assertEqual(client, fake_ironic)
         self.service_catalog.url_for.assert_called_once_with(
@@ -211,7 +211,7 @@ class OSClientsTestCase(test.TestCase):
     def test_sahara(self, mock_sahara):
         fake_sahara = fakes.FakeSaharaClient()
         mock_sahara.Client = mock.MagicMock(return_value=fake_sahara)
-        self.assertTrue("sahara" not in self.clients.cache)
+        self.assertNotIn("sahara", self.clients.cache)
         client = self.clients.sahara()
         self.assertEqual(client, fake_sahara)
         kw = {
@@ -227,7 +227,7 @@ class OSClientsTestCase(test.TestCase):
     def test_zaqar(self, mock_zaqar):
         fake_zaqar = fakes.FakeZaqarClient()
         mock_zaqar.Client = mock.MagicMock(return_value=fake_zaqar)
-        self.assertTrue("zaqar" not in self.clients.cache)
+        self.assertNotIn("zaqar", self.clients.cache)
         client = self.clients.zaqar()
         self.assertEqual(client, fake_zaqar)
         self.service_catalog.url_for.assert_called_once_with(

tests/unit/test_utils.py

@@ -128,7 +128,7 @@ class ImportModulesTestCase(test.TestCase):
     def test_try_append_module_into_sys_modules(self):
         modules = {}
         utils.try_append_module('rally.version', modules)
-        self.assertTrue('rally.version' in modules)
+        self.assertIn('rally.version', modules)
 
     def test_try_append_broken_module(self):
         modules = {}
@@ -139,8 +139,8 @@ class ImportModulesTestCase(test.TestCase):
 
     def test_import_modules_from_package(self):
         utils.import_modules_from_package('tests.unit.fixtures.import.package')
-        self.assertTrue('tests.unit.fixtures.import.package.a' in sys.modules)
+        self.assertIn('tests.unit.fixtures.import.package.a', sys.modules)
-        self.assertTrue('tests.unit.fixtures.import.package.b' in sys.modules)
+        self.assertIn('tests.unit.fixtures.import.package.b', sys.modules)
 
 
 class LogTestCase(test.TestCase):
@@ -241,7 +241,7 @@ class FirstIndexTestCase(test.TestCase):
 
     def test_list_with_non_existing_matching_element(self):
         lst = [1, 3, 5, 7]
-        self.assertEqual(utils.first_index(lst, lambda e: e == 2), None)
+        self.assertIsNone(utils.first_index(lst, lambda e: e == 2))
 
 
 class DocstringTestCase(test.TestCase):

tests/unit/verification/verifiers/test_tempest.py

@@ -92,7 +92,8 @@ class TempestUtilsTestCase(BaseTestCase):
         self.assertFalse(mock_sp.check_output.called)
 
     @mock.patch("os.path.isdir", return_value=False)
-    @mock.patch(TEMPEST_PATH + ".tempest.subprocess.check_output")
+    @mock.patch("%s.tempest.subprocess.check_output" % TEMPEST_PATH,
+                return_value="some_output")
     @testtools.skipIf(sys.version_info < (2, 7), "Incompatible Python Version")
     def test__venv_install_when_venv_not_exist(self, mock_sp, mock_isdir):
         self.verifier._install_venv()