From 18de36a3c899839dbddcb5d648effe4df3fa7a9c Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 10 2024 01:42:03 +0000 Subject: [PATCH 1/11] more missing tearDowns --- diff --git a/tests/test_cli/test_watch_tasks.py b/tests/test_cli/test_watch_tasks.py index 5254750..9f09cc1 100644 --- a/tests/test_cli/test_watch_tasks.py +++ b/tests/test_cli/test_watch_tasks.py @@ -157,6 +157,9 @@ class TestWatchLogsCLI(utils.CliTestCase): %s: error: {message} """ % (self.progname, self.progname) + def tearDown(self): + mock.patch.stopall() + def test_handle_watch_task_help(self): self.assert_help( anon_handle_watch_task, diff --git a/tests/test_hub/test_get_user.py b/tests/test_hub/test_get_user.py index ba04035..bea2584 100644 --- a/tests/test_hub/test_get_user.py +++ b/tests/test_hub/test_get_user.py @@ -101,9 +101,13 @@ class TestGetUser(DBQueryTestCase): class TestGetUserByKrbPrincipal(unittest.TestCase): + def setUp(self): self.get_user = mock.patch('kojihub.kojihub.get_user').start() + def tearDown(self): + mock.patch.stopall() + def test_wrong_type_krb_principal(self): krb_principal = ['test-user'] with self.assertRaises(koji.GenericError) as cm: diff --git a/tests/test_hub/test_tag_operations.py b/tests/test_hub/test_tag_operations.py index 4429356..3cf0ba8 100644 --- a/tests/test_hub/test_tag_operations.py +++ b/tests/test_hub/test_tag_operations.py @@ -221,6 +221,9 @@ class TestGetTag(unittest.TestCase): self.queries = [] self.tagname = 'test-tag' + def tearDown(self): + mock.patch.stopall() + def test_get_tag_invalid_taginfo(self): taginfo = {'test-tag': 'value'} with self.assertRaises(koji.GenericError) as ex: From 92a1e0d719939ac90f8868745ad325843693f41f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 11 2024 15:29:09 +0000 Subject: [PATCH 2/11] fix tz issues in cli tests --- diff --git a/tests/test_cli/test_hostinfo.py b/tests/test_cli/test_hostinfo.py index 24fbc02..87bf57c 100644 --- a/tests/test_cli/test_hostinfo.py +++ b/tests/test_cli/test_hostinfo.py @@ -49,7 +49,7 @@ class TestHostinfo(utils.CliTestCase): del os.environ['TZ'] else: os.environ['TZ'] = self.original_timezone - time.tzset() + time.tzset() mock.patch.stopall() def test_hostinfo_without_option(self): diff --git a/tests/test_cli/test_list_hosts.py b/tests/test_cli/test_list_hosts.py index 987483f..698653a 100644 --- a/tests/test_cli/test_list_hosts.py +++ b/tests/test_cli/test_list_hosts.py @@ -45,7 +45,7 @@ class TestListHosts(utils.CliTestCase): del os.environ['TZ'] else: os.environ['TZ'] = self.original_timezone - time.tzset() + time.tzset() mock.patch.stopall() def __vm(self, result): diff --git a/tests/test_cli/test_list_pkgs.py b/tests/test_cli/test_list_pkgs.py index cf4a88c..5c7c28f 100644 --- a/tests/test_cli/test_list_pkgs.py +++ b/tests/test_cli/test_list_pkgs.py @@ -2,6 +2,8 @@ from __future__ import absolute_import import mock import copy +import os +import time from six.moves import StringIO import koji @@ -16,6 +18,9 @@ class TestListPkgs(utils.CliTestCase): self.session = mock.MagicMock() self.session.getAPIVersion.return_value = koji.API_VERSION self.ensure_connection_mock = mock.patch('koji_cli.commands.ensure_connection').start() + self.original_timezone = os.environ.get('TZ') + os.environ['TZ'] = 'UTC' + time.tzset() self.error_format = """Usage: %s list-pkgs [options] (Specify the --help global option for a list of other help options) @@ -54,6 +59,11 @@ class TestListPkgs(utils.CliTestCase): def tearDown(self): mock.patch.stopall() + if self.original_timezone is None: + del os.environ['TZ'] + else: + os.environ['TZ'] = self.original_timezone + time.tzset() def test_list_pkgs_non_exist_tag(self): self.session.getTag.return_value = None From cb6992ce035468b460fde002d238fd5939b501e1 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 11 2024 15:29:09 +0000 Subject: [PATCH 3/11] fix misuse of progname in unit tests this was breaking parallel testing --- diff --git a/tests/test_cli/test_mock_config.py b/tests/test_cli/test_mock_config.py index 9f58ad0..c054cab 100644 --- a/tests/test_cli/test_mock_config.py +++ b/tests/test_cli/test_mock_config.py @@ -84,7 +84,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' self.gen_config_mock.return_value = self.mock_output # buildroot check - arguments = ['--buildroot', 'root', self.progname] + arguments = ['--buildroot', 'root', 'ROOTNAME'] expected = self.format_error_message("Buildroot id must be an integer") self.assert_system_exit( anon_handle_mock_config, @@ -95,7 +95,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' self.ensure_connection_mock.assert_called_once_with(self.session, self.options) self.ensure_connection_mock.reset_mock() - arguments = self.common_args + ['--buildroot', '1', '--name', self.progname] + arguments = self.common_args + ['--buildroot', '1', '--name', 'ROOTNAME'] opts = self.common_opts.copy() opts.update({ 'repoid': buildroot_info['repo_id'], @@ -106,17 +106,17 @@ config_opts['macros']['%distribution'] = 'Koji Testing' del opts['topurl'] anon_handle_mock_config(self.options, self.session, arguments) self.assert_console_message(stdout, "%s\n" % self.gen_config_mock.return_value) - self.gen_config_mock.assert_called_with(self.progname, buildroot_info['arch'], **opts) + self.gen_config_mock.assert_called_with('ROOTNAME', buildroot_info['arch'], **opts) self.ensure_connection_mock.assert_called_once_with(self.session, self.options) self.ensure_connection_mock.reset_mock() arguments = self.common_args + ['--buildroot', '1', - '--name', self.progname, + '--name', 'ROOTNAME', '--latest'] opts['repoid'] = 'latest' anon_handle_mock_config(self.options, self.session, arguments) self.assert_console_message(stdout, "%s\n" % self.gen_config_mock.return_value) - self.gen_config_mock.assert_called_with(self.progname, buildroot_info['arch'], **opts) + self.gen_config_mock.assert_called_with('ROOTNAME', buildroot_info['arch'], **opts) self.ensure_connection_mock.assert_called_once_with(self.session, self.options) self.ensure_connection_mock.reset_mock() @@ -186,13 +186,13 @@ config_opts['macros']['%distribution'] = 'Koji Testing' }) del opts['topurl'] arguments = self.common_args + ['--task', str(task_id), - '--name', self.progname, + '--name', 'ROOTNAME', '--latest'] self.session.listBuildroots.return_value = [multi_broots[0]] self.gen_config_mock.return_value = self.mock_output anon_handle_mock_config(self.options, self.session, arguments) self.assert_console_message(stdout, "%s\n" % self.gen_config_mock.return_value) - self.gen_config_mock.assert_called_with(self.progname, multi_broots[0]['arch'], **opts) + self.gen_config_mock.assert_called_with('ROOTNAME', multi_broots[0]['arch'], **opts) self.ensure_connection_mock.assert_called_once_with(self.session, self.options) self.ensure_connection_mock.reset_mock() @@ -276,7 +276,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' arguments = self.common_args + ['--tag', tag['name'], '--arch', tag['arch'], - '--name', self.progname, + '--name', 'ROOTNAME', '--latest'] opts = self.common_opts.copy() opts.update({ @@ -300,7 +300,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' del opts['topurl'] anon_handle_mock_config(self.options, self.session, arguments) self.assert_console_message(stdout, "%s\n" % self.gen_config_mock.return_value) - self.gen_config_mock.assert_called_with(self.progname, tag['arch'], **opts) + self.gen_config_mock.assert_called_with('ROOTNAME', tag['arch'], **opts) self.ensure_connection_mock.assert_called_once_with(self.session, self.options) self.ensure_connection_mock.reset_mock() @@ -309,7 +309,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' warn_msg = '%s is not in the list of tag arches' % arch self.gen_config_mock.return_value = self.mock_output arguments = self.common_args + ['--tag', tag['name'], '--arch', arch, - '--name', self.progname, '--latest'] + '--name', 'ROOTNAME', '--latest'] anon_handle_mock_config(self.options, self.session, arguments) self.assert_console_message(stdout, "%s\n" % self.gen_config_mock.return_value) self.assert_console_message(stderr, "%s\n" % warn_msg) @@ -331,7 +331,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' warn_msg = 'Tag %s has an empty arch list' % tag['name'] self.gen_config_mock.return_value = self.mock_output arguments = self.common_args + ['--tag', tag['name'], '--arch', arch, - '--name', self.progname, '--latest'] + '--name', 'ROOTNAME', '--latest'] anon_handle_mock_config(self.options, self.session, arguments) self.assert_console_message(stdout, "%s\n" % self.gen_config_mock.return_value) self.assert_console_message(stderr, "%s\n" % warn_msg) @@ -390,7 +390,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' arguments = ['--distribution', 'fedora', '--topurl', '/top-url', '--yum-proxy', '/yum-proxy', '--target', target['name'], '--arch', arch, - '--name', self.progname] + '--name', 'ROOTNAME'] opts = self.common_opts.copy() opts.update({ 'repoid': 101, @@ -403,7 +403,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' self.gen_config_mock.return_value = self.mock_output anon_handle_mock_config(self.options, self.session, arguments) self.assert_console_message(stdout, "%s\n" % self.gen_config_mock.return_value) - self.gen_config_mock.assert_called_with(self.progname, arch, **opts) + self.gen_config_mock.assert_called_with('ROOTNAME', arch, **opts) self.ensure_connection_mock.assert_called_once_with(self.session, self.options) self.ensure_connection_mock.reset_mock() @@ -415,7 +415,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' anon_handle_mock_config(self.options, self.session, arguments) openf.assert_called_once() fobj.write.assert_called_once_with(self.mock_output) - self.gen_config_mock.assert_called_with(self.progname, arch, **opts) + self.gen_config_mock.assert_called_with('ROOTNAME', arch, **opts) self.ensure_connection_mock.assert_called_once_with(self.session, self.options) self.ensure_connection_mock.reset_mock() @@ -423,7 +423,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' arch = 'test' arguments = self.common_args + ['--target', target['name'], '--arch', arch, - '--name', self.progname] + '--name', 'ROOTNAME'] warn_msg = '%s is not in the list of tag arches' % arch self.gen_config_mock.return_value = self.mock_output anon_handle_mock_config(self.options, self.session, arguments) @@ -449,7 +449,7 @@ config_opts['macros']['%distribution'] = 'Koji Testing' self.ensure_connection_mock.reset_mock() # name is specified twice case - arguments = [self.progname, '--name', 'name'] + arguments = ['duplicate_buildroot_name', '--name', 'name'] expected = self.format_error_message("Name already specified via option") self.assert_system_exit( anon_handle_mock_config, From f3031c5726940fbc5f01ca1dc8fae5e4bebbbf75 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2024 15:29:09 +0000 Subject: [PATCH 4/11] more mocks related to context --- diff --git a/tests/test_cli/utils.py b/tests/test_cli/utils.py index 60d3cf6..3694f26 100644 --- a/tests/test_cli/utils.py +++ b/tests/test_cli/utils.py @@ -43,6 +43,8 @@ class CliTestCase(unittest.TestCase): STDOUT = sys.stdout STDERR = sys.stderr + def tearDown(self): + mock.patch.stopall() # # private methods # @@ -129,7 +131,7 @@ class CliTestCase(unittest.TestCase): # check callableObj callable self.__assert_callable(callableObj) - # these arguments are reseverd and used in assert_system_exit + # these arguments are reserved and used in assert_system_exit reserved = [ 'activate_session', 'stdout', 'stderr', 'assert_func', 'exit_code' diff --git a/tests/test_hub/test_add_external_repo_to_tag.py b/tests/test_hub/test_add_external_repo_to_tag.py index b208dfe..dca2171 100644 --- a/tests/test_hub/test_add_external_repo_to_tag.py +++ b/tests/test_hub/test_add_external_repo_to_tag.py @@ -9,6 +9,10 @@ class TestAddExternalRepoToTag(unittest.TestCase): def setUp(self): self.tag_name = 'test-tag' + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() self.get_tag = mock.patch('kojihub.kojihub.get_tag').start() self.get_external_repo = mock.patch('kojihub.kojihub.get_external_repo').start() self.get_tag_external_repos = mock.patch('kojihub.kojihub.get_tag_external_repos').start() diff --git a/tests/test_hub/test_add_group_member.py b/tests/test_hub/test_add_group_member.py index 3dcb20b..e891d34 100644 --- a/tests/test_hub/test_add_group_member.py +++ b/tests/test_hub/test_add_group_member.py @@ -10,6 +10,10 @@ class TestAddGroupMember(unittest.TestCase): def setUp(self): self.exports = kojihub.RootExports() + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() self.get_user = mock.patch('kojihub.kojihub.get_user').start() def tearDown(self): diff --git a/tests/test_hub/test_add_user_krb_principal.py b/tests/test_hub/test_add_user_krb_principal.py index 7d82b8b..2066ff5 100644 --- a/tests/test_hub/test_add_user_krb_principal.py +++ b/tests/test_hub/test_add_user_krb_principal.py @@ -8,6 +8,10 @@ import copy class TestAddUserKrbPrincipal(unittest.TestCase): def setUp(self): + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() self.get_user = mock.patch('kojihub.kojihub.get_user').start() self.verify_name_user = mock.patch('kojihub.kojihub.verify_name_user').start() self.get_user_by_krb_principal = mock.patch('kojihub.kojihub.get_user_by_krb_principal').start() diff --git a/tests/test_hub/test_create_image_build.py b/tests/test_hub/test_create_image_build.py index 864283a..6bd3ecb 100644 --- a/tests/test_hub/test_create_image_build.py +++ b/tests/test_hub/test_create_image_build.py @@ -13,6 +13,10 @@ class TestCreateImageBuild(unittest.TestCase): def setUp(self): self.get_build = mock.patch('kojihub.kojihub.get_build').start() self.exports = kojihub.RootExports() + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() self.InsertProcessor = mock.patch('kojihub.kojihub.InsertProcessor', side_effect=self.getInsert).start() self.inserts = [] diff --git a/tests/test_hub/test_delete_build_target.py b/tests/test_hub/test_delete_build_target.py index 9b5c227..e35348a 100644 --- a/tests/test_hub/test_delete_build_target.py +++ b/tests/test_hub/test_delete_build_target.py @@ -20,6 +20,10 @@ class TestDeleteBuildTarget(unittest.TestCase): def setUp(self): self.lookup_build_target = mock.patch('kojihub.kojihub.lookup_build_target').start() self.exports = kojihub.RootExports() + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor', side_effect=self.getUpdate).start() self.updates = [] diff --git a/tests/test_hub/test_disable_channel.py b/tests/test_hub/test_disable_channel.py index fcb4595..e14b749 100644 --- a/tests/test_hub/test_disable_channel.py +++ b/tests/test_hub/test_disable_channel.py @@ -18,6 +18,10 @@ class TestDisableChannel(unittest.TestCase): def setUp(self): self.exports = kojihub.RootExports() + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() self.get_channel = mock.patch('kojihub.kojihub.get_channel').start() self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor', side_effect=self.getUpdate).start() diff --git a/tests/test_hub/test_enable_channel.py b/tests/test_hub/test_enable_channel.py index 183cb2e..6b7a0c3 100644 --- a/tests/test_hub/test_enable_channel.py +++ b/tests/test_hub/test_enable_channel.py @@ -18,6 +18,11 @@ class TestEnableChannel(unittest.TestCase): def setUp(self): self.exports = kojihub.RootExports() + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() + self.get_channel = mock.patch('kojihub.kojihub.get_channel').start() self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor', side_effect=self.getUpdate).start() diff --git a/tests/test_hub/test_get_group_members.py b/tests/test_hub/test_get_group_members.py index e152e66..0274d00 100644 --- a/tests/test_hub/test_get_group_members.py +++ b/tests/test_hub/test_get_group_members.py @@ -11,6 +11,10 @@ class TestGetGroupMembers(DBQueryTestCase): super(TestGetGroupMembers, self).setUp() self.get_user = mock.patch('kojihub.kojihub.get_user').start() self.exports = kojihub.RootExports() + self.context = mock.patch('kojihub.kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() def tearDown(self): mock.patch.stopall() diff --git a/tests/test_hub/test_models/test_host.py b/tests/test_hub/test_models/test_host.py index 80daa53..343f098 100644 --- a/tests/test_hub/test_models/test_host.py +++ b/tests/test_hub/test_models/test_host.py @@ -132,7 +132,8 @@ class TestHost(unittest.TestCase): ) self.assertEqual(processor.call_args_list[2], update3) - def test_task_wait_check(self): + @mock.patch('kojihub.kojihub.context') + def test_task_wait_check(self, context): self.query_execute.return_value = [{'id': 1, 'state': 1}, {'id': 2, 'state': 2}, {'id': 3, 'state': 3}, diff --git a/tests/test_hub/test_write_signed_rpm.py b/tests/test_hub/test_write_signed_rpm.py index 2e0e431..1037db1 100644 --- a/tests/test_hub/test_write_signed_rpm.py +++ b/tests/test_hub/test_write_signed_rpm.py @@ -7,6 +7,7 @@ import kojihub class TestWriteSignedRPM(unittest.TestCase): def setUp(self): + self.context = mock.patch('kojihub.kojihub.context').start() self.get_rpm = mock.patch('kojihub.kojihub.get_rpm').start() def tearDown(self): From aadc45aa143bafa3471bc253198718bdb9995e40 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 11 2024 15:29:46 +0000 Subject: [PATCH 5/11] drop stray tearDown --- diff --git a/tests/test_cli/utils.py b/tests/test_cli/utils.py index 3694f26..3ac2b51 100644 --- a/tests/test_cli/utils.py +++ b/tests/test_cli/utils.py @@ -43,8 +43,6 @@ class CliTestCase(unittest.TestCase): STDOUT = sys.stdout STDERR = sys.stderr - def tearDown(self): - mock.patch.stopall() # # private methods # From 495418d311d97b3c4c5104749a68ede9877513ed Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 11 2024 18:52:17 +0000 Subject: [PATCH 6/11] fix context mocking --- diff --git a/tests/test_hub/test_add_btype.py b/tests/test_hub/test_add_btype.py index 2753f1e..04a473f 100644 --- a/tests/test_hub/test_add_btype.py +++ b/tests/test_hub/test_add_btype.py @@ -10,50 +10,49 @@ IP = kojihub.InsertProcessor class TestAddBType(unittest.TestCase): - @mock.patch('kojihub.kojihub.verify_name_internal') - @mock.patch('kojihub.kojihub.list_btypes') - @mock.patch('kojihub.kojihub.InsertProcessor') - def test_add_btype(self, InsertProcessor, list_btypes, verify_name_internal): - # Not sure why mock can't patch kojihub.context, so we do this - session = kojihub.kojihub.context.session = mock.MagicMock() - mocks = [InsertProcessor, list_btypes, session] + def setUp(self): + self.verify_name_internal = mock.patch('kojihub.kojihub.verify_name_internal').start() + self.list_btypes = mock.patch('kojihub.kojihub.list_btypes').start() + self.InsertProcessor = mock.patch('kojihub.kojihub.InsertProcessor').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.session = self.context.session # It seems MagicMock will not automatically handle attributes that # start with "assert" - session.assertPerm = mock.MagicMock() - verify_name_internal.return_value = None + self.session.assertPerm = mock.MagicMock() + self.verify_name_internal.return_value = None + def tearDown(self): + mock.patch.stopall() + + def test_add_btype(self): # expected case - list_btypes.return_value = None - insert = InsertProcessor.return_value + self.list_btypes.return_value = None + insert = self.InsertProcessor.return_value kojihub.add_btype('new_btype') - InsertProcessor.assert_called_once() + self.InsertProcessor.assert_called_once() insert.execute.assert_called_once() - args, kwargs = InsertProcessor.call_args + args, kwargs = self.InsertProcessor.call_args ip = IP(*args, **kwargs) self.assertEqual(ip.table, 'btype') self.assertEqual(ip.data, {'name': 'new_btype'}) self.assertEqual(ip.rawdata, {}) - session.assertPerm.assert_called_with('admin') - - for m in mocks: - m.reset_mock() - session.assertPerm = mock.MagicMock() + self.session.assertPerm.assert_called_with('admin') + def test_btype_exists(self): # already exists - list_btypes.return_value = True + self.list_btypes.return_value = True with self.assertRaises(koji.GenericError): kojihub.add_btype('new_btype') - InsertProcessor.assert_not_called() - session.assertPerm.assert_called_with('admin') + self.InsertProcessor.assert_not_called() + self.session.assertPerm.assert_called_with('admin') + def test_btype_badname(self): # name is longer as expected new_btype = 'new-btype+' - verify_name_internal.side_effect = koji.GenericError + self.verify_name_internal.side_effect = koji.GenericError with self.assertRaises(koji.GenericError): kojihub.add_btype(new_btype) - # not except regex rules - verify_name_internal.side_effect = koji.GenericError - with self.assertRaises(koji.GenericError): - kojihub.add_btype(new_btype) + +# the end diff --git a/tests/test_hub/test_add_external_repo_to_tag.py b/tests/test_hub/test_add_external_repo_to_tag.py index dca2171..cf872a4 100644 --- a/tests/test_hub/test_add_external_repo_to_tag.py +++ b/tests/test_hub/test_add_external_repo_to_tag.py @@ -17,6 +17,8 @@ class TestAddExternalRepoToTag(unittest.TestCase): self.get_external_repo = mock.patch('kojihub.kojihub.get_external_repo').start() self.get_tag_external_repos = mock.patch('kojihub.kojihub.get_tag_external_repos').start() self.parse_arches = mock.patch('koji.parse_arches').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() self.tag_info = {'id': 1, 'name': self.tag_name} self.external_repo_info = {'id': 123, 'name': 'test-repo'} self.priority = 11 diff --git a/tests/test_hub/test_add_group_member.py b/tests/test_hub/test_add_group_member.py index e891d34..e63b55f 100644 --- a/tests/test_hub/test_add_group_member.py +++ b/tests/test_hub/test_add_group_member.py @@ -15,6 +15,8 @@ class TestAddGroupMember(unittest.TestCase): # start with "assert" self.context.session.assertPerm = mock.MagicMock() self.get_user = mock.patch('kojihub.kojihub.get_user').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() def tearDown(self): mock.patch.stopall() diff --git a/tests/test_hub/test_add_user_krb_principal.py b/tests/test_hub/test_add_user_krb_principal.py index 2066ff5..ad13fdd 100644 --- a/tests/test_hub/test_add_user_krb_principal.py +++ b/tests/test_hub/test_add_user_krb_principal.py @@ -15,6 +15,8 @@ class TestAddUserKrbPrincipal(unittest.TestCase): self.get_user = mock.patch('kojihub.kojihub.get_user').start() self.verify_name_user = mock.patch('kojihub.kojihub.verify_name_user').start() self.get_user_by_krb_principal = mock.patch('kojihub.kojihub.get_user_by_krb_principal').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() self.username = 'testuser' self.krbprincipal = '%s@TEST.COM' % self.username self.userinfo = {'id': 1, 'name': self.username} diff --git a/tests/test_hub/test_create_image_build.py b/tests/test_hub/test_create_image_build.py index 6bd3ecb..2db4c64 100644 --- a/tests/test_hub/test_create_image_build.py +++ b/tests/test_hub/test_create_image_build.py @@ -12,6 +12,8 @@ class TestCreateImageBuild(unittest.TestCase): def setUp(self): self.get_build = mock.patch('kojihub.kojihub.get_build').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() self.exports = kojihub.RootExports() self.context = mock.patch('kojihub.kojihub.context').start() # It seems MagicMock will not automatically handle attributes that diff --git a/tests/test_hub/test_delete_build_target.py b/tests/test_hub/test_delete_build_target.py index e35348a..6e8f224 100644 --- a/tests/test_hub/test_delete_build_target.py +++ b/tests/test_hub/test_delete_build_target.py @@ -27,6 +27,8 @@ class TestDeleteBuildTarget(unittest.TestCase): self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor', side_effect=self.getUpdate).start() self.updates = [] + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() self.context_db = mock.patch('kojihub.db.context').start() # It seems MagicMock will not automatically handle attributes that # start with "assert" diff --git a/tests/test_hub/test_disable_channel.py b/tests/test_hub/test_disable_channel.py index e14b749..c8d900a 100644 --- a/tests/test_hub/test_disable_channel.py +++ b/tests/test_hub/test_disable_channel.py @@ -23,6 +23,8 @@ class TestDisableChannel(unittest.TestCase): # start with "assert" self.context.session.assertPerm = mock.MagicMock() self.get_channel = mock.patch('kojihub.kojihub.get_channel').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor', side_effect=self.getUpdate).start() self.updates = [] diff --git a/tests/test_hub/test_dist_repo.py b/tests/test_hub/test_dist_repo.py index 50bb4c7..dcf6bcf 100644 --- a/tests/test_hub/test_dist_repo.py +++ b/tests/test_hub/test_dist_repo.py @@ -105,18 +105,21 @@ class TestDistRepoInit(unittest.TestCase): class TestDistRepo(unittest.TestCase): - @mock.patch('kojihub.kojihub.assert_policy') - @mock.patch('kojihub.kojihub.dist_repo_init') - @mock.patch('kojihub.kojihub.make_task') - def test_DistRepo(self, make_task, dist_repo_init, assert_policy): - session = kojihub.context.session = mock.MagicMock() + def setUp(self): + self.assert_policy = mock.patch('kojihub.kojihub.assert_policy').start() + self.dist_repo_init = mock.patch('kojihub.kojihub.dist_repo_init').start() + self.make_task = mock.patch('kojihub.kojihub.make_task').start() + self.context = mock.patch('kojihub.kojihub.context').start() + + def tearDown(self): + mock.patch.stopall() + + def test_DistRepo(self): + session = self.context.session session.user_id = 123 - # It seems MagicMock will not automatically handle attributes that - # start with "assert" - session.hasPerm = mock.MagicMock() session.hasPerm.return_value = False - dist_repo_init.return_value = ('repo_id', 'event_id') - make_task.return_value = 'task_id' + self.dist_repo_init.return_value = ('repo_id', 'event_id') + self.make_task.return_value = 'task_id' exports = kojihub.RootExports() exports.getBuildConfig = mock.MagicMock() exports.getBuildConfig.return_value = {'extra': {}} @@ -124,10 +127,10 @@ class TestDistRepo(unittest.TestCase): ret = exports.distRepo('tag', 'keys') session.hasPerm.assert_has_calls([mock.call('dist-repo'), mock.call('admin')]) - assert_policy.assert_called_once_with('dist_repo', {'tag': 'tag'}) - dist_repo_init.assert_called_once() - make_task.assert_called_once() - self.assertEqual(ret, make_task.return_value) + self.assert_policy.assert_called_once_with('dist_repo', {'tag': 'tag'}) + self.dist_repo_init.assert_called_once() + self.make_task.assert_called_once() + self.assertEqual(ret, self.make_task.return_value) exports.getBuildConfig.assert_called_once_with('tag') @@ -216,6 +219,7 @@ class TestDistRepoMove(unittest.TestCase): self.get_build = mock.patch('kojihub.kojihub.get_build').start() self.get_rpm.side_effect = self.our_get_rpm self.get_build.side_effect = self.our_get_build + self.context = mock.patch('kojihub.kojihub.context').start() def tearDown(self): mock.patch.stopall() @@ -228,7 +232,7 @@ class TestDistRepoMove(unittest.TestCase): return self.builds[buildInfo] def test_distRepoMove(self): - session = kojihub.context.session = mock.MagicMock() + session = self.context.session session.user_id = 123 exports = kojihub.HostExports() exports.distRepoMove(self.rinfo['id'], self.uploadpath, self.arch) diff --git a/tests/test_hub/test_edit_build_target.py b/tests/test_hub/test_edit_build_target.py index 0f4fd7c..526af20 100644 --- a/tests/test_hub/test_edit_build_target.py +++ b/tests/test_hub/test_edit_build_target.py @@ -30,7 +30,8 @@ class TestEditBuildTarget(unittest.TestCase): self.target_info = {'id': 123, 'name': self.target_name} self.build_tag_info = {'id': 111, 'name': self.build_tag} self.dest_tag_info = {'id': 112, 'name': self.dest_tag} - self.session = kojihub.context.session = mock.MagicMock() + self.context = mock.patch('kojihub.kojihub.context').start() + self.session = self.context.session self.session.assertPerm = mock.MagicMock() self.QueryProcessor = mock.patch('kojihub.kojihub.QueryProcessor', side_effect=self.getQuery).start() diff --git a/tests/test_hub/test_massTag.py b/tests/test_hub/test_massTag.py index af84fb6..471cd2a 100644 --- a/tests/test_hub/test_massTag.py +++ b/tests/test_hub/test_massTag.py @@ -7,7 +7,7 @@ import kojihub class TestDeleteEventId(unittest.TestCase): @mock.patch('kojihub.kojihub.context') def test_delete_event_id(self, context): - kojihub.context.event_id = 123 + context.event_id = 123 kojihub._delete_event_id() self.assertFalse(hasattr(context, 'event_id')) diff --git a/tests/test_hub/test_multicall.py b/tests/test_hub/test_multicall.py index 3280b19..a83443b 100644 --- a/tests/test_hub/test_multicall.py +++ b/tests/test_hub/test_multicall.py @@ -15,17 +15,19 @@ class DummyExports(object): class TestMulticall(unittest.TestCase): + def setUp(self): + self.context = mock.patch('kojihub.kojixmlrpc.context').start() + self.context_db = mock.patch('kojihub.db.context').start() + self.kojihub = mock.patch('kojihub.kojixmlrpc.kojihub').start() + self.registry = HandlerRegistry() + self.exports = DummyExports() + self.registry.register_instance(self.exports) + def tearDown(self): mock.patch.stopall() def test_multicall(self): - self.context_db = mock.patch('kojihub.db.context').start() kojixmlrpc.kojihub = mock.MagicMock() - kojixmlrpc.context.opts = mock.MagicMock() - kojixmlrpc.context.session = mock.MagicMock() - self.registry = HandlerRegistry() - self.exports = DummyExports() - self.registry.register_instance(self.exports) calls = [{'methodName': 'foo', 'params': [1]}, {'methodName': 'non', 'params': [mock.ANY]}, {'methodName': 'foo', 'params': [2, Exception('with int arg', 1)]}, From 5d972a7598fdd79c1aad11ef26bf4cf98aa91f8d Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 11 2024 21:44:20 +0000 Subject: [PATCH 7/11] avoid leaving stray temp files --- diff --git a/tests/test_builder/test_build_notification.py b/tests/test_builder/test_build_notification.py index f525461..5f09b4b 100644 --- a/tests/test_builder/test_build_notification.py +++ b/tests/test_builder/test_build_notification.py @@ -8,6 +8,7 @@ import time import unittest import mock +import shutil import six import koji @@ -71,6 +72,7 @@ class TestBuildNotification(unittest.TestCase): os.environ['TZ'] = self.original_timezone time.tzset() mock.patch.stopall() + shutil.rmtree(self.tempdir) def test_build_notification(self): # force locale to compare 'message' value diff --git a/tests/test_builder/test_choose_taskarch.py b/tests/test_builder/test_choose_taskarch.py index a1c2c3f..b24e56c 100644 --- a/tests/test_builder/test_choose_taskarch.py +++ b/tests/test_builder/test_choose_taskarch.py @@ -1,6 +1,7 @@ from __future__ import absolute_import import mock import rpm +import shutil import tempfile import unittest import koji @@ -27,9 +28,9 @@ class TestChooseTaskarch(unittest.TestCase): self.session = mock.MagicMock() self.options = mock.MagicMock() self.options.literal_task_arches = '' - workdir = tempfile.mkdtemp() + self.workdir = tempfile.mkdtemp() self.handler = kojid.BuildTask(task_id, method, params, self.session, - self.options, workdir) + self.options, self.workdir) # mock some more things self.handler.event_id = 42 @@ -39,6 +40,9 @@ class TestChooseTaskarch(unittest.TestCase): self.session.getBuildConfig = self.getBuildConfig self.getBuildConfig.return_value = {'arches': 'armv7hl i686 x86_64 ppc64'} + def tearDown(self): + shutil.rmtree(self.workdir) + def test_binary_arches(self): for arch in ['i386', 'i686', 'x86_64', 'ppc', 'ppc64le', 's390', 's390x']: diff --git a/tests/test_builder/test_main.py b/tests/test_builder/test_main.py index 02e4a3b..e1480d6 100644 --- a/tests/test_builder/test_main.py +++ b/tests/test_builder/test_main.py @@ -1,7 +1,6 @@ from __future__ import absolute_import import mock import signal -import tempfile import unittest from six.moves import range @@ -23,7 +22,6 @@ class TestMain(unittest.TestCase): self.options.plugin = [] self.options.sleeptime = 1 self.options.pluginpath = '' - workdir = tempfile.mkdtemp() self.setup_rlimits = mock.patch('koji.util.setup_rlimits').start() self.TaskManager = mock.MagicMock() # the kojid import is weird, so we use patch.object diff --git a/tests/test_builder/test_volume_id.py b/tests/test_builder/test_volume_id.py index 2e557e6..fe81429 100644 --- a/tests/test_builder/test_volume_id.py +++ b/tests/test_builder/test_volume_id.py @@ -1,5 +1,6 @@ from __future__ import absolute_import import mock +import shutil import tempfile import unittest from .loadkojid import kojid @@ -75,9 +76,12 @@ class TestVolumeID(unittest.TestCase): self.session = mock.MagicMock() self.options = mock.MagicMock() self.options.literal_task_arches = '' - workdir = tempfile.mkdtemp() + self.workdir = tempfile.mkdtemp() self.handler = kojid.LiveCDTask(task_id, method, params, self.session, - self.options, workdir) + self.options, self.workdir) + + def tearDown(self): + shutil.rmtree(self.workdir) def test_volume_id_substitutions(self): """Check that volume ID is shorten corect by shortenVolID method.""" diff --git a/tests/test_cli/test_repoinfo.py b/tests/test_cli/test_repoinfo.py index cb54985..e175c5d 100644 --- a/tests/test_cli/test_repoinfo.py +++ b/tests/test_cli/test_repoinfo.py @@ -3,6 +3,7 @@ from __future__ import absolute_import import unittest import mock +import shutil import six import tempfile @@ -45,6 +46,7 @@ class TestRepoinfo(utils.CliTestCase): def tearDown(self): mock.patch.stopall() + shutil.rmtree(self.tempdir) @mock.patch('koji.formatTimeLong', return_value='Thu, 01 Jan 2000') @mock.patch('sys.stderr', new_callable=six.StringIO) diff --git a/tests/test_hub/test_dist_repo.py b/tests/test_hub/test_dist_repo.py index dcf6bcf..471ff14 100644 --- a/tests/test_hub/test_dist_repo.py +++ b/tests/test_hub/test_dist_repo.py @@ -39,6 +39,7 @@ class TestDistRepoInit(unittest.TestCase): def tearDown(self): mock.patch.stopall() + shutil.rmtree(self.tempdir) def test_simple_dist_repo_init(self): diff --git a/tests/test_lib/test_tasks.py b/tests/test_lib/test_tasks.py index 5444c56..7cb897c 100644 --- a/tests/test_lib/test_tasks.py +++ b/tests/test_lib/test_tasks.py @@ -1,13 +1,14 @@ from __future__ import absolute_import import mock +import os import random import shutil import six from six.moves import range +import tempfile import unittest from os import path, makedirs -from tempfile import gettempdir from mock import patch, MagicMock, Mock, call import requests_mock @@ -44,15 +45,6 @@ def get_fake_mounts_file(): ))) -def get_temp_dir_root(): - return path.join(gettempdir(), 'koji_tests') - - -def get_tmp_dir_path(folder_starts_with): - return path.join(get_temp_dir_root(), ( - '{0}{1}'.format(folder_starts_with, random.randint(1, 999999999999)))) - - class TaskTest(BaseTaskHandler): Methods = ['some_method'] _taskWeight = 5.2 @@ -74,11 +66,14 @@ class BadTask(BaseTaskHandler): class TasksTestCase(unittest.TestCase): + def setUp(self): + self.tempdir = tempfile.mkdtemp() + def tearDown(self): - temp_dir_root = get_temp_dir_root() + shutil.rmtree(self.tempdir) - if path.isdir(temp_dir_root): - shutil.rmtree(get_temp_dir_root()) + def get_tmp_dir_path(self, name): + return os.path.join(self.tempdir, name) def test_scan_mounts_results(self): """ Tests the scan_mounts function with a mocked /proc/mounts file. @@ -143,7 +138,7 @@ class TasksTestCase(unittest.TestCase): by the child class. """ obj = BadTask(123, 'some_method', ['random_arg'], None, None, - (get_tmp_dir_path('BadTask'))) + (self.get_tmp_dir_path('BadTask'))) try: obj.handler() raise Exception('The NotImplementedError exception was not raised') @@ -155,7 +150,7 @@ class TasksTestCase(unittest.TestCase): class' definition. """ obj = TaskNoWeightTest(123, 'some_method', ['random_arg'], None, None, - (get_tmp_dir_path('TaskNoWeightTest'))) + (self.get_tmp_dir_path('TaskNoWeightTest'))) self.assertEqual(obj.weight(), 1.0) def test_BaseTaskHandler_weight_set(self): @@ -163,14 +158,14 @@ class TasksTestCase(unittest.TestCase): child class' definition. """ obj = TaskTest(123, 'some_method', ['random_arg'], None, None, - (get_tmp_dir_path('TaskTest'))) + (self.get_tmp_dir_path('TaskTest'))) self.assertEqual(obj.weight(), 5.2) def test_BaseTaskHandler_createWorkdir_workdir_not_defined(self): """ Tests that the createWorkdir function does nothing when the workdir member variable is set to None. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(123, 'some_method', ['random_arg'], None, None, temp_path) obj.workdir = None obj.createWorkdir() @@ -182,17 +177,16 @@ class TasksTestCase(unittest.TestCase): """ Tests that the createWorkdir function creates a folder based on the path given to the workdir member variable. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(123, 'some_method', ['random_arg'], None, None, temp_path) obj.createWorkdir() self.assertEqual(path.isdir(temp_path), True) - shutil.rmtree(get_temp_dir_root()) def test_BaseTaskHandler_removeWorkdir(self): """ Tests that the removeWOrkdir function deletes a folder based on the path given to the workdir member variable. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(123, 'some_method', ['random_arg'], None, None, temp_path) makedirs(temp_path) self.assertEqual(path.isdir(temp_path), True) @@ -203,7 +197,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the wait function returns the subtask results of when the taskWait function returns only two finished tasks. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(12345678, 'some_method', ['random_arg'], None, None, temp_path) makedirs(temp_path) obj.session = Mock() @@ -238,7 +232,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the wait function returns the one finished subtask results of when the taskWait function returns one finished task and one unfinished """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(12345678, 'some_method', ['random_arg'], None, None, temp_path) makedirs(temp_path) obj.session = Mock() @@ -266,7 +260,7 @@ class TasksTestCase(unittest.TestCase): The taskWait function should first return one finished and one unfinished task, then the second time it should return two finished tasks. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(12345678, 'some_method', ['random_arg'], None, None, temp_path) makedirs(temp_path) obj.session = Mock() @@ -305,7 +299,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the wait function raises an exception when one of the subtask fails when the failany flag is set to True. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(12345678, 'some_method', ['random_arg'], None, None, temp_path) makedirs(temp_path) obj.session = Mock() @@ -326,7 +320,7 @@ class TasksTestCase(unittest.TestCase): @patch('signal.pause') def test_BaseTaskHandler_wait_timeout(self, pause, sigtimedwait, sleep, time): """Tests timeout behavior in the wait function""" - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(95, 'some_method', ['random_arg'], None, None, temp_path) makedirs(temp_path) obj.session = MagicMock() @@ -348,7 +342,7 @@ class TasksTestCase(unittest.TestCase): @patch('signal.pause') def test_BaseTaskHandler_wait_avoid_timeout(self, pause, sigtimedwait, sleep, time): """Tests that timeout does not happen if tasks finish in time""" - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(95, 'some_method', ['random_arg'], None, None, temp_path) makedirs(temp_path) obj.session = MagicMock() @@ -371,7 +365,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the getUploadDir function returns the appropriate path based on the id of the handler. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') obj = TaskTest(123, 'some_method', ['random_arg'], None, None, temp_path) self.assertEqual(obj.getUploadDir(), 'tasks/123/123') @@ -381,7 +375,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the uploadFile function calls the uploadWrapper function on the session member variable with the correct input. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) temp_file = path.join(temp_path, 'test.txt') with open(temp_file, 'wt') as temp_file_handler: @@ -399,7 +393,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the uploadFile function calls the uploadWrapper function on the session member variable without including empty files. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) temp_file = path.join(temp_path, 'test.txt') @@ -415,7 +409,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the uploadTree function calls the uploadFile function with the correct parameters. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) dummy_dir = path.join(temp_path, 'some_directory') @@ -441,7 +435,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the chownTree functions as expected on dummy files created in a temp directory """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) dummy_file = path.join(temp_path, 'test.txt') @@ -461,7 +455,7 @@ class TasksTestCase(unittest.TestCase): """ Tests the localPath function to ensure that when a file exists, it returns that path without trying to download it. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) local_folder = path.join(temp_path, 'local') @@ -479,7 +473,7 @@ class TasksTestCase(unittest.TestCase): def test_BaseTaskHandler_localPath_no_file(self, m_requests): """ """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) local_folder = path.join(temp_path, 'local') @@ -500,21 +494,21 @@ class TasksTestCase(unittest.TestCase): def test_BaseTaskHandler_localPath_no_topurl(self): """ Tests that the localPath function returns a path when options.topurl is not defined. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) options = Mock() options.topurl = None - options.topdir = get_temp_dir_root() + options.topdir = self.tempdir obj = TaskTest(123, 'some_method', ['random_arg'], None, options, temp_path) - self.assertEqual(obj.localPath('test.txt'), path.join(get_temp_dir_root(), 'test.txt')) + self.assertEqual(obj.localPath('test.txt'), path.join(self.tempdir, 'test.txt')) def test_BaseTaskHandler_find_arch(self): """ Tests that the find_arch function returns the input for arch when the input is not "noarch". """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) obj = TaskTest(123, 'some_method', ['random_arg'], None, None, temp_path) self.assertEqual(obj.find_arch('x86_64', None, None), 'x86_64') @@ -523,7 +517,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the find_arch function raises an exception when the host parameter doesn't contain a value for the arches key. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) host = {'arches': None, 'name': 'test.domain.local'} obj = TaskTest(123, 'some_method', ['random_arg'], None, None, temp_path) @@ -537,7 +531,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the find_arch function raises an exception when the tag parameter doesn't contain a value for the arches key. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) host = {'arches': 'x86_64', 'name': 'test.domain.local'} tag = {'arches': None, 'name': 'some_package-1.2-build'} @@ -552,7 +546,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the find_arch function finds a match of x86_64 when the host only supports x86_64 and the tag supports x86_64 and aarch64. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) host = {'arches': 'x86_64', 'name': 'test.domain.local'} tag = {'arches': 'x86_64 aarch64', 'name': 'some_package-1.2-build'} @@ -563,7 +557,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the find_arch function raises an exception when there isn't a common arch supported between the host and the tag. """ - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) host = {'arches': 'i386', 'name': 'test.domain.local'} tag = {'arches': 'x86_64 aarch64', 'name': 'some_package-1.2-build'} @@ -579,7 +573,7 @@ class TasksTestCase(unittest.TestCase): @patch('koji.util.RepoWatcher') def test_getRepo_no_wait_task(self, RepoWatcher): """ Tests that the getRepo method does not wait if repo is available""" - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) repo_dict = { @@ -606,7 +600,7 @@ class TasksTestCase(unittest.TestCase): @patch('koji.util.RepoWatcher') def test_getRepo_last_event(self, RepoWatcher): """ Tests that the getRepo method uses min_event='last' when requested""" - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) repo_dict = { @@ -634,7 +628,7 @@ class TasksTestCase(unittest.TestCase): @patch('koji.util.RepoWatcher') def test_getRepo_wait_task(self, RepoWatcher): """ Tests that the getRepo function waits for subtask if repo not immediately available""" - temp_path = get_tmp_dir_path('TaskTest') + temp_path = self.get_tmp_dir_path('TaskTest') makedirs(temp_path) repo_dict = { @@ -664,7 +658,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the FakeTest handler can be instantiated and returns 42 when run. """ obj = FakeTask(123, 'someMethod', ['random_arg'], None, None, - (get_tmp_dir_path('FakeTask'))) + (self.get_tmp_dir_path('FakeTask'))) self.assertEqual(obj.run(), 42) @patch('time.sleep') @@ -672,7 +666,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the SleepTask handler can be instantiated and runs appropriately based on the input. """ - obj = SleepTask(123, 'sleep', [5], None, None, (get_tmp_dir_path('SleepTask'))) + obj = SleepTask(123, 'sleep', [5], None, None, (self.get_tmp_dir_path('SleepTask'))) obj.run() mock_sleep.assert_called_once_with(5) @@ -681,7 +675,7 @@ class TasksTestCase(unittest.TestCase): """ Tests that the ForkTask handler can be instantiated and runs appropriately based on the input. """ - obj = ForkTask(123, 'fork', [1, 20], None, None, (get_tmp_dir_path('ForkTask'))) + obj = ForkTask(123, 'fork', [1, 20], None, None, (self.get_tmp_dir_path('ForkTask'))) obj.run() mock_spawnvp.assert_called_once_with(1, 'sleep', ['sleep', '20']) @@ -698,7 +692,7 @@ class TasksTestCase(unittest.TestCase): self.assertEqual(method, 'sleep') task_id = self.mock_subtask_id self.mock_subtask_id += 1 - obj = SleepTask(task_id, 'sleep', arglist, None, None, (get_tmp_dir_path('SleepTask'))) + obj = SleepTask(task_id, 'sleep', arglist, None, None, (self.get_tmp_dir_path('SleepTask'))) obj.run() return task_id @@ -712,7 +706,7 @@ class TasksTestCase(unittest.TestCase): if task_id == 4: raise koji.GenericError() - obj = WaitTestTask(123, 'waittest', [3], None, None, (get_tmp_dir_path('WaitTestTask'))) + obj = WaitTestTask(123, 'waittest', [3], None, None, (self.get_tmp_dir_path('WaitTestTask'))) obj.session = Mock() obj.session.host.subtask.side_effect = mock_subtask obj.session.getTaskResult.side_effect = mock_getTaskResult diff --git a/tests/test_lib/test_utils.py b/tests/test_lib/test_utils.py index 020a182..79aa414 100644 --- a/tests/test_lib/test_utils.py +++ b/tests/test_lib/test_utils.py @@ -1473,10 +1473,14 @@ class TestRmtree(unittest.TestCase): self.unlink.assert_not_called() self.isdir.assert_not_called() + @mock.patch('tempfile.mkstemp') # avoid stray temp file @mock.patch('koji.util._rmtree_nofork') @mock.patch('os.fork') @mock.patch('os._exit') - def test_rmtree_child(self, _exit, fork, rmtree_nofork): + def test_rmtree_child(self, _exit, fork, rmtree_nofork, mkstemp): + log = self.tempdir + '/rmtree-log.jsonl' + fd = os.open(log, os.O_RDWR | os.O_CREAT) + mkstemp.return_value = fd, log fork.return_value = 0 path = "/SOME_PATH" logger = "LOGGER" @@ -1492,12 +1496,17 @@ class TestRmtree(unittest.TestCase): rmtree_nofork.assert_called_once() self.assertEqual(rmtree_nofork.call_args[0][0], path) _exit.assert_called_once() + logger = rmtree_nofork.call_args.kwargs['logger'] + @mock.patch('tempfile.mkstemp') # avoid stray temp file @mock.patch('koji.util._rmtree_nofork') @mock.patch('os.fork') @mock.patch('os.waitpid') @mock.patch('os._exit') - def test_rmtree_child_fails(self, _exit, waitpid, fork, rmtree_nofork): + def test_rmtree_child_fails(self, _exit, waitpid, fork, rmtree_nofork, mkstemp): + log = self.tempdir + '/rmtree-log.jsonl' + fd = os.open(log, os.O_RDWR | os.O_CREAT) + mkstemp.return_value = fd, log fork.return_value = 0 path = "/SOME_PATH" logger = "LOGGER" @@ -1515,11 +1524,15 @@ class TestRmtree(unittest.TestCase): _exit.assert_called_once() waitpid.assert_not_called + @mock.patch('tempfile.mkstemp') # avoid stray temp file @mock.patch('koji.util._rmtree_nofork') @mock.patch('os.fork') @mock.patch('os.waitpid') @mock.patch('os._exit') - def test_rmtree_parent(self, _exit, waitpid, fork, rmtree_nofork): + def test_rmtree_parent(self, _exit, waitpid, fork, rmtree_nofork, mkstemp): + log = self.tempdir + '/rmtree-log.jsonl' + fd = os.open(log, os.O_RDWR | os.O_CREAT) + mkstemp.return_value = fd, log pid = 137 fork.return_value = pid waitpid.return_value = pid, 0 @@ -1530,13 +1543,17 @@ class TestRmtree(unittest.TestCase): rmtree_nofork.assert_not_called() _exit.assert_not_called() + @mock.patch('tempfile.mkstemp') # avoid stray temp file @mock.patch('koji.util.SimpleProxyLogger.send') @mock.patch('koji.util._rmtree_nofork') @mock.patch('os.fork') @mock.patch('os.unlink') @mock.patch('os.waitpid') @mock.patch('os._exit') - def test_rmtree_parent_logfail(self, _exit, waitpid, unlink, fork, rmtree_nofork, logsend): + def test_rmtree_parent_logfail(self, _exit, waitpid, unlink, fork, rmtree_nofork, logsend, mkstemp): + log = self.tempdir + '/rmtree-log.jsonl' + fd = os.open(log, os.O_RDWR | os.O_CREAT) + mkstemp.return_value = fd, log pid = 137 fork.return_value = pid waitpid.return_value = pid, 0 From bf78efbf2624d6cdeedd79ec1b2daffc23818221 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 15 2024 16:01:29 +0000 Subject: [PATCH 8/11] use pytest-dist for make test3 --- diff --git a/test-requirements.txt b/test-requirements.txt index 897624f..a527954 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -6,6 +6,8 @@ mock<=2.0.0 requests-mock coverage pytest +pytest-cov +pytest-xdist Cheetah3 psycopg2-binary python-multilib diff --git a/tox.ini b/tox.ini index d4776b7..3d9c004 100644 --- a/tox.ini +++ b/tox.ini @@ -36,7 +36,7 @@ commands_pre = [testenv:py3] deps = -r{toxinidir}/test-requirements.txt -allowlist_externals = coverage3 +allowlist_externals = coverage3,pytest setenv = {[testenv]setenv} PYTHONPATH=.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib @@ -44,9 +44,7 @@ commands_pre = {[testenv]commands_pre} coverage3 erase --rcfile .coveragerc3 commands = - coverage3 run --rcfile .coveragerc3 --source . -m pytest {posargs} - coverage3 report --rcfile .coveragerc3 - coverage3 html -d {toxinidir}/htmlcov/py3 --rcfile .coveragerc3 + pytest -n auto --cov --cov-config .coveragerc3 --cov-report=html [testenv:py2] deps = From 81271f34ec38bfb662efd9dcd087d442990a1a6f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 16 2024 13:33:45 +0000 Subject: [PATCH 9/11] use venv pytest, stop using ancient mock version dropping mock from the py3 reqs entirely so we use python's lib --- diff --git a/test-requirements.txt b/test-requirements.txt index a527954..62ebc50 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,7 +2,6 @@ defusedxml flake8 flake8-import-order -mock<=2.0.0 requests-mock coverage pytest diff --git a/tox.ini b/tox.ini index 3d9c004..e214641 100644 --- a/tox.ini +++ b/tox.ini @@ -36,7 +36,7 @@ commands_pre = [testenv:py3] deps = -r{toxinidir}/test-requirements.txt -allowlist_externals = coverage3,pytest +allowlist_externals = coverage3 setenv = {[testenv]setenv} PYTHONPATH=.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib @@ -44,7 +44,7 @@ commands_pre = {[testenv]commands_pre} coverage3 erase --rcfile .coveragerc3 commands = - pytest -n auto --cov --cov-config .coveragerc3 --cov-report=html + python -m pytest -n auto --cov --cov-config .coveragerc3 --cov-report=html [testenv:py2] deps = From 790512b04df33e36ee3d24e06352319e5e48fc20 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 16 2024 14:09:03 +0000 Subject: [PATCH 10/11] also use venv coverage --- diff --git a/tox.ini b/tox.ini index e214641..5b56a3a 100644 --- a/tox.ini +++ b/tox.ini @@ -36,13 +36,12 @@ commands_pre = [testenv:py3] deps = -r{toxinidir}/test-requirements.txt -allowlist_externals = coverage3 setenv = {[testenv]setenv} PYTHONPATH=.:plugins/hub/.:plugins/builder/.:plugins/cli/.:cli/.:www/lib commands_pre = {[testenv]commands_pre} - coverage3 erase --rcfile .coveragerc3 + python -m coverage erase --rcfile .coveragerc3 commands = python -m pytest -n auto --cov --cov-config .coveragerc3 --cov-report=html From fc0f55cb7590472d601498002632368a78fe1a90 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jul 16 2024 16:51:04 +0000 Subject: [PATCH 11/11] more coverage filters --- diff --git a/.coveragerc3 b/.coveragerc3 index 541f1cd..4086cb8 100644 --- a/.coveragerc3 +++ b/.coveragerc3 @@ -4,7 +4,9 @@ data_file = .coverage3 omit = setup.py /usr/* + /tmp/* tests/* + devtools/* .tox/* [report]