diff --git a/rally/verification/verifiers/tempest/tempest.py b/rally/verification/verifiers/tempest/tempest.py index 1b4358c918..0331cd900a 100644 --- a/rally/verification/verifiers/tempest/tempest.py +++ b/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: - print(output) + LOG.debug(output) class Tempest(object): diff --git a/tests/hacking/README.rst b/tests/hacking/README.rst index dc154d024b..73a7a94d90 100644 --- a/tests/hacking/README.rst +++ b/tests/hacking/README.rst @@ -7,8 +7,15 @@ Rally Style Commandments Rally Specific Commandments --------------------------- - -- [N301] Ensure that ``assert_*`` methods from ``mock`` library is used correctly -- [N302] Sub-error of N301, related to nonexistent "assert_called" -- [N303] Sub-error of N301, related to nonexistent "assert_called_once" -- [N310] Ensure that ``rally.log`` is used instead of ``rally.openstack.common.log`` +* [N30x] - Reserved for rules related to ``mock`` library + * [N301] - Ensure that ``assert_*`` methods from ``mock`` library is used correctly + * [N302] - Ensure that nonexistent "assert_called" is not used + * [N303] - Ensure that nonexistent "assert_called_once" is not used +* [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 diff --git a/tests/hacking/checks.py b/tests/hacking/checks.py index 2c63da6c53..7167d5f6ef 100644 --- a/tests/hacking/checks.py +++ b/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) diff --git a/tests/unit/benchmark/processing/test_utils.py b/tests/unit/benchmark/processing/test_utils.py index 80711e4ae3..7a443c928b 100644 --- a/tests/unit/benchmark/processing/test_utils.py +++ b/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) diff --git a/tests/unit/benchmark/scenarios/test_base.py b/tests/unit/benchmark/scenarios/test_base.py index 2991b6f990..8dc04c5c91 100644 --- a/tests/unit/benchmark/scenarios/test_base.py +++ b/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, - basestring)) + self.assertIsInstance(base.Scenario.RESOURCE_NAME_PREFIX, basestring) def test_RESOURCE_NAME_LENGTH(self): - self.assertTrue( - isinstance(base.Scenario.RESOURCE_NAME_LENGTH, int)) + self.assertIsInstance(base.Scenario.RESOURCE_NAME_LENGTH, int) self.assertTrue(base.Scenario.RESOURCE_NAME_LENGTH > 4) @mock.patch( diff --git a/tests/unit/cmd/commands/test_info.py b/tests/unit/cmd/commands/test_info.py index 2fcb8b7d8d..13e77d3775 100644 --- a/tests/unit/cmd/commands/test_info.py +++ b/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) diff --git a/tests/unit/cmd/test_envutils.py b/tests/unit/cmd/test_envutils.py index 237da6de17..e923d470b1 100644 --- a/tests/unit/cmd/test_envutils.py +++ b/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')) diff --git a/tests/unit/deploy/serverprovider/providers/test_openstack.py b/tests/unit/deploy/serverprovider/providers/test_openstack.py index 92fa9e3104..20527646e7 100644 --- a/tests/unit/deploy/serverprovider/providers/test_openstack.py +++ b/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, diff --git a/tests/unit/deploy/serverprovider/providers/test_virsh.py b/tests/unit/deploy/serverprovider/providers/test_virsh.py index f014b79149..4e3fe3dce2 100644 --- a/tests/unit/deploy/serverprovider/providers/test_virsh.py +++ b/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', diff --git a/tests/unit/test_hacking.py b/tests/unit/test_hacking.py index 7cdc847172..04eea27d03 100644 --- a/tests/unit/test_hacking.py +++ b/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) diff --git a/tests/unit/test_osclients.py b/tests/unit/test_osclients.py index 133b92c159..a5c5f01470 100644 --- a/tests/unit/test_osclients.py +++ b/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( diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 1bc105b657..033a89987f 100644 --- a/tests/unit/test_utils.py +++ b/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.assertTrue('tests.unit.fixtures.import.package.b' in sys.modules) + self.assertIn('tests.unit.fixtures.import.package.a', 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): diff --git a/tests/unit/verification/verifiers/test_tempest.py b/tests/unit/verification/verifiers/test_tempest.py index 5cb1dece3d..80bb4e6a39 100644 --- a/tests/unit/verification/verifiers/test_tempest.py +++ b/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()