PR#4068: Rework mocking of QueryProcessor in DBQueryTestCase - koji

koji

#4068 Rework mocking of QueryProcessor in DBQueryTestCase

Merged a year ago by tkopecek. Opened a year ago by mikem.

mikem/koji fix-qp-mock into master

more missing tearDowns

Mike McLean • a year ago

25d6517

fix missing tearDown methods for numerous tests

Mike McLean • a year ago

c6272f2

rework DBQueryTestCase mocking

Mike McLean • a year ago

a5b7b88

tests/test_cli/test_add_channel.py

file modified

		`@@ -26,6 +26,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_add_channel(self, stdout):`
		`self.session.addChannel.return_value = self.channel_id`

tests/test_cli/test_add_external_repo.py

file modified

		`@@ -25,6 +25,9 @@`
		`self.tag = 'test-tag'`
		`self.priority = 10`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_add_external_repo_invalid_mode(self):`
		`mode = 'test-mode'`
		`arguments = ['--mode', mode]`

tests/test_cli/test_add_group.py

file modified

		`@@ -21,6 +21,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_add_group(self, stdout):`
		`tag = 'tag'`

tests/test_cli/test_add_host.py

file modified

		`@@ -19,6 +19,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_add_host(self, stdout):`
		`host = 'host'`

tests/test_cli/test_add_host_to_channel.py

file modified

		`@@ -19,6 +19,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_add_host_to_channel(self, stdout):`
		`host = 'host'`

tests/test_cli/test_add_notification.py

file modified

		`@@ -21,6 +21,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_add_notification(self):`
		`self.session.getPackageID.return_value = 1234`
		`self.session.getTagID.return_value = 4321`

tests/test_cli/test_add_pkg.py

file modified

		`@@ -27,6 +27,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_add_pkg(self, stdout):`
		`tag = 'tag'`

tests/test_cli/test_add_tag.py

file modified

		`@@ -21,6 +21,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_add_tag(self):`
		`"""Test handle_add_tag function"""`
		`# Case 1. no argument error`

tests/test_cli/test_add_tag_inheritance.py

file modified

		`@@ -56,6 +56,9 @@`
		`'perm': None,`
		`'perm_id': None}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_add_tag_inheritance_without_option(self):`
		`arguments = []`
		`expected = self.format_error_message(`

tests/test_cli/test_add_target.py

file modified

		`@@ -20,6 +20,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_add_target_without_option(self,):`
		`expected = self.format_error_message(`
		`"Please specify a target name, a build tag, and destination tag")`

tests/test_cli/test_add_user.py

file modified

		`@@ -25,6 +25,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_add_user(self, stdout):`
		`"""Test handle_add_user function"""`

tests/test_cli/test_add_volume.py

file modified

		`@@ -24,6 +24,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_add_volume(self, stdout):`
		`"""Test handle_add_volume function"""`

tests/test_cli/test_assign_task.py

file modified

		`@@ -24,6 +24,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_assign_task(self, stdout):`
		`hostname = "host"`

tests/test_cli/test_block_group.py

file modified

		`@@ -25,6 +25,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_block_group_nonexistent_tag(self):`
		`tag = 'nonexistent-tag'`
		`group = 'group'`

tests/test_cli/test_block_notification.py

file modified

		`@@ -21,6 +21,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_block_notification_non_exist_tag(self):`
		`tag = 'test-tag'`
		`arguments = ['--tag', tag]`

tests/test_cli/test_block_pkg.py

file modified

		`@@ -26,6 +26,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_block_pkg(self, stdout):`
		`tag = 'tag'`

tests/test_cli/test_build.py

file modified

		`@@ -48,6 +48,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_build_from_srpm(self, stdout):`
		`args = [self.target, self.source_srpm]`

tests/test_cli/test_call.py

file modified

		`@@ -29,6 +29,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_call(self, stdout):`
		`"""Test handle_call function"""`

tests/test_cli/test_cancel.py

file modified

		`@@ -35,6 +35,9 @@`
		`self.session.hub_version = (1, 33, 0)`
		`self.session.hub_version_str = '1.33.0'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_anon_cancel(self):`
		`args = ['123']`
		`self.activate_session_mock.side_effect = koji.GenericError`

tests/test_cli/test_chain_build.py

file modified

		`@@ -31,6 +31,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_chain_build(self, stdout):`
		`target = 'target'`

tests/test_cli/test_disable_channel.py

file modified

		`@@ -29,6 +29,9 @@`
		`'name': 'test-channel'}`
		`]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def __vm(self, result):`
		`m = koji.VirtualCall('mcall_method', [], {})`
		`if isinstance(result, dict) and result.get('faultCode'):`

tests/test_cli/test_disable_host.py

file modified

		`@@ -21,6 +21,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_disable_host_no_such_host(self):`
		`"""Test %s function""" % handle_disable_host.__name__`
		`self.session.getHost.return_value = None`

tests/test_cli/test_disable_user.py

file modified

		`@@ -24,6 +24,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_disable_user_no_argument(self):`
		`"""Test handle_disable_user function"""`
		`expected = self.format_error_message(`

tests/test_cli/test_edit_external_repo.py

file modified

		`@@ -24,6 +24,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_edit_external_repo_error(self):`
		`"""Test handle_edit_external_repo function"""`
		`# [(expected, args),...]`

tests/test_cli/test_edit_host.py

file modified

		`@@ -28,6 +28,9 @@`
		`self.description = 'description'`
		`self.comment = 'comment'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_edit_host(self, stdout):`
		`host_info = mock.ANY`

tests/test_cli/test_edit_notification.py

file modified

		`@@ -19,6 +19,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_edit_notification(self):`
		`self.session.getPackageID.return_value = 1234`
		`self.session.getTagID.return_value = 4321`

tests/test_cli/test_edit_permission.py

file modified

		`@@ -26,6 +26,9 @@`
		`self.perm = 'test-perm'`
		`self.description = 'test-description'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_edit_permission_argument_error(self):`
		`expected = self.format_error_message("Please specify a permission and a description")`
		`for args in [[], [self.perm]]:`

tests/test_cli/test_edit_tag.py

file modified

		`@@ -30,6 +30,9 @@`
		`self.extra = {'extraA': 'A', 'extraB': True}`
		`self.remove_extra = ['extraC', 'extraD']`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_edit_tag_1(self, stdout):`
		`args = [self.tag]`

tests/test_cli/test_edit_target.py

file modified

		`@@ -42,6 +42,9 @@`
		`self.new_dest_tag = 'new-dest-tag'`
		`self.new_build_tag = 'new-build-tag'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_edit_target_without_option(self):`
		`expected = self.format_error_message("Please specify a build target")`
		`self.assert_system_exit(`

tests/test_cli/test_edit_user.py

file modified

		`@@ -26,6 +26,9 @@`
		`self.user = 'user'`
		`self.rename = 'user2'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_edit_user(self, stdout):`
		`args = [self.user]`

tests/test_cli/test_enable_channel.py

file modified

		`@@ -30,6 +30,9 @@`
		`'name': 'test-channel'}`
		`]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def __vm(self, result):`
		`m = koji.VirtualCall('mcall_method', [], {})`
		`if isinstance(result, dict) and result.get('faultCode'):`

tests/test_cli/test_enable_host.py

file modified

		`@@ -22,6 +22,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_enable_host_no_such_host(self):`
		`"""Test %s function""" % handle_enable_host.__name__`

tests/test_cli/test_enable_user.py

file modified

		`@@ -21,6 +21,9 @@`
		`""" % (self.progname, self.progname)`
		`self.username = 'user'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_enable_user_no_argument(self):`
		`"""Test handle_enable_user function"""`
		`expected = self.format_error_message("You must specify the username of the user to enable")`

tests/test_cli/test_free_task.py

file modified

		`@@ -19,6 +19,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_free_task_without_arg(self):`
		`expected = self.format_error_message('please specify at least one task_id')`
		`self.assert_system_exit(`

tests/test_cli/test_grant_cg_access.py

file modified

		`@@ -22,6 +22,9 @@`
		`self.cg = 'cg'`
		`self.user = 'user'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_grant_cg_access_arg_error(self):`
		`"""Test handle_grant_cg_access function"""`
		`expected = self.format_error_message("Please specify a user and content generator")`

tests/test_cli/test_grant_permission.py

file modified

		`@@ -26,6 +26,9 @@`
		`self.perm = 'createuser'`
		`self.user = 'user'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_grant_permission_argument_error(self):`
		`expected = self.format_error_message(`
		`"Please specify a permission and at least one user")`

tests/test_cli/test_hello.py

file modified

		`@@ -39,6 +39,9 @@`
		`""" % (self.progname, self.progname)`
		`self.huburl = "https://%s.local/%shub" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`@mock.patch('koji_cli.commands._printable_unicode')`
		`def test_handle_moshimoshi(self, print_unicode_mock, stdout):`

tests/test_cli/test_import.py

file modified

		`@@ -84,6 +84,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def __do_import_test(self, options, session, arguments, **kwargs):`
		`expected = kwargs.get('expected', None)`
		`rpm_header = kwargs.get('rpm_header', {})`

tests/test_cli/test_import_archive.py

file modified

		`@@ -23,6 +23,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_import_archive_without_option(self):`
		`expected = self.format_error_message(`
		`"You must specify a build ID or N-V-R and an archive to import")`

tests/test_cli/test_import_cg.py

file modified

		`@@ -34,6 +34,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`@mock.patch('koji_cli.commands._progress_callback')`
		`@mock.patch('koji.json')`

tests/test_cli/test_import_comps.py

file modified

		`@@ -38,6 +38,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`@mock.patch('koji_cli.commands.libcomps')`
		`@mock.patch('koji_cli.commands._import_comps')`

tests/test_cli/test_import_sig.py

file modified

		`@@ -76,6 +76,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stderr', new_callable=six.StringIO)`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`@mock.patch('koji.rip_rpm_sighdr')`

tests/test_cli/test_list_api.py

file modified

		`@@ -19,6 +19,9 @@`
		`self.options = mock.MagicMock()`
		`self.ensure_connection = mock.patch('koji_cli.commands.ensure_connection').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_anon_handle_list_api_all_method(self, stdout):`
		`"""Test anon_handle_list_api function"""`

tests/test_cli/test_list_buildroot.py

file modified

		`@@ -22,6 +22,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_list_buildroot_without_args(self):`
		`self.assert_system_exit(`
		`anon_handle_list_buildroot,`

tests/test_cli/test_list_channels.py

file modified

		`@@ -64,6 +64,9 @@`
		`]]`
		`]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=StringIO)`
		`def test_list_channels_not_quiet(self, stdout):`
		`self.session.listChannels.return_value = self.list_channels`

tests/test_cli/test_list_notifications.py

file modified

		`@@ -23,6 +23,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=StringIO)`
		`def test_list_notifications(self, stdout):`
		`self.session.getBuildNotifications.return_value = [`

tests/test_cli/test_list_permissions.py

file modified

		`@@ -36,6 +36,9 @@`
		`self.user = 'tester'`
		`self.userinfo = {'id': 101, 'name': self.user}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_list_permissions_arg_error(self):`
		`"""Test handle_list_permissions argument error (no argument is required)"""`
		`expected = self.format_error_message("This command takes no arguments")`

tests/test_cli/test_list_pkgs.py

file modified

		`@@ -52,6 +52,9 @@`
		`'perm': None,`
		`'perm_id': None}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_list_pkgs_non_exist_tag(self):`
		`self.session.getTag.return_value = None`
		`self.assert_system_exit(`

tests/test_cli/test_list_signed.py

file modified

		`@@ -20,6 +20,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def __vm(self, result):`
		`m = koji.VirtualCall('mcall_method', [], {})`
		`if isinstance(result, dict) and result.get('faultCode'):`

tests/test_cli/test_list_tags.py

file modified

		`@@ -38,6 +38,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_list_tags_non_exist_package(self):`
		`pkg = 'test-pkg'`
		`self.session.getPackage.return_value = None`

tests/test_cli/test_mock_config.py

file modified

		`@@ -65,6 +65,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_mock_config_buildroot_option(self, stdout):`
		`"""Test anon_handle_mock_config buildroot options"""`

tests/test_cli/test_remove_external_repo.py

file modified

		`@@ -18,6 +18,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_remove_external_repo_help(self):`
		`self.assert_help(`
		`handle_remove_external_repo,`

tests/test_cli/test_remove_pkg.py

file modified

		`@@ -28,6 +28,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stderr', new_callable=six.StringIO)`
		`def test_handle_remove_pkg(self, stderr):`
		`tag = 'tag'`

tests/test_cli/test_remove_tag_inheritance.py

file modified

		`@@ -50,6 +50,9 @@`
		`'perm': None,`
		`'perm_id': None}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_remove_tag_inheritance_without_option(self):`
		`expected = self.format_error_message(`
		`"This command takes at least one argument: a tag name or ID")`

tests/test_cli/test_restart_host.py

file modified

		`@@ -24,6 +24,9 @@`
		`self.watch_tasks_mock = mock.patch('koji_cli.commands.watch_tasks').start()`
		`self.task_id = 101`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_restart_hosts_force_options(self):`
		`"""Test %s function with --force option""" % handle_restart_hosts.__name__`
		`arguments = ['--force']`

tests/test_cli/test_set_task_priority.py

file modified

		`@@ -25,6 +25,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('sys.stderr', new_callable=six.StringIO)`
		`@mock.patch('sys.stdout', new_callable=six.StringIO)`
		`def test_handle_set_task_priority(self, stdout, stderr):`

tests/test_cli/test_show_groups.py

file modified

		`@@ -52,6 +52,9 @@`
		`'group_id': 3}`
		`]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_show_groups_incorrect_num_of_args(self):`
		`arguments = []`
		`self.assert_system_exit(`

tests/test_cli/test_untag_build.py

file modified

		`@@ -34,6 +34,9 @@`
		`'name': 'test-package', 'release': '1.f35', 'tag.name': 'test-tag',`
		`'tag_id': 460, 'version': '1.1'}]}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def __vm(self, result):`
		`m = koji.VirtualCall('mcall_method', [], {})`
		`if isinstance(result, dict) and result.get('faultCode'):`

tests/test_cli/test_watch_logs.py

file modified

		`@@ -21,6 +21,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_handle_watch_logs_help(self):`
		`self.assert_help(`
		`anon_handle_watch_logs,`

tests/test_cli/test_win_build.py

file modified

		`@@ -39,6 +39,9 @@`
		`%s: error: {message}`
		`""" % (self.progname, self.progname)`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_win_build_without_option(self):`
		`self.assert_system_exit(`
		`handle_win_build,`

tests/test_hub/test_add_group_member.py

file modified

		`@@ -12,6 +12,9 @@`
		`self.exports = kojihub.RootExports()`
		`self.get_user = mock.patch('kojihub.kojihub.get_user').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_user(self):`
		`data = [{'id': 3,`
		`'name': 'test-group',`

tests/test_hub/test_add_volume.py

file modified

		`@@ -19,6 +19,9 @@`
		`self.context.session.assertPerm = mock.MagicMock()`
		`self.context.session.assertLogin = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_add_volume_wrong_format(self):`
		`volume_name = 'volume-name+'`

tests/test_hub/test_create_image_build.py

file modified

		`@@ -18,6 +18,9 @@`
		`self.inserts = []`
		`self.insert_execute = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getInsert(self, args, *kwargs):`
		`insert = IP(args, *kwargs)`
		`insert.execute = self.insert_execute`

tests/test_hub/test_create_maven_build.py

file modified

		`@@ -32,6 +32,9 @@`
		`}`
		`self.build_info = 'test-build-11-12'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getInsert(self, args, *kwargs):`
		`insert = IP(args, *kwargs)`
		`insert.execute = self.insert_execute`

tests/test_hub/test_create_user.py

file modified

		`@@ -24,6 +24,9 @@`
		`self.user_info_krb = {'id': 1, 'krb_principals': ['test_user@fedora.org'],`
		`'name': self.user_name, 'status': 0, 'usertype': 0}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_create_user_wrong_format(self):`
		`user_name = 'test-user+'`

tests/test_hub/test_create_win_build.py

file modified

		`@@ -32,6 +32,9 @@`
		`}`
		`self.build_info = 'test-build-11-12'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getInsert(self, args, *kwargs):`
		`insert = IP(args, *kwargs)`
		`insert.execute = self.insert_execute`

tests/test_hub/test_delete_build_target.py

file modified

		`@@ -28,6 +28,9 @@`
		`# start with "assert"`
		`self.context_db.session.assertLogin = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_target(self):`
		`build_target = 'build-target'`
		`self.lookup_build_target.return_value = None`

tests/test_hub/test_disable_channel.py

file modified

		`@@ -24,6 +24,9 @@`
		`self.updates = []`
		`self.channelname = 'test-channel'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_channel(self):`
		`self.get_channel.return_value = None`
		`with self.assertRaises(koji.GenericError) as cm:`

tests/test_hub/test_disable_user.py

file modified

		`@@ -12,6 +12,9 @@`
		`self.exports = kojihub.RootExports()`
		`self.get_user = mock.patch('kojihub.kojihub.get_user').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_user(self):`
		`username = 'test-user'`
		`self.get_user.return_value = None`

tests/test_hub/test_edit_permission.py

file modified

		`@@ -24,6 +24,9 @@`
		`self.perm_info = {'id': 1, 'name': self.perm_name}`
		`self.description = 'test-description'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_edit_permission_non_exist_permission(self):`
		`self.lookup_perm.side_effect = koji.GenericError`
		`with self.assertRaises(koji.GenericError):`

tests/test_hub/test_enable_channel.py

file modified

		`@@ -24,6 +24,9 @@`
		`self.updates = []`
		`self.channelname = 'test-channel'`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_channel(self):`

		`self.get_channel.return_value = None`

tests/test_hub/test_enable_user.py

file modified

		`@@ -12,6 +12,9 @@`
		`self.exports = kojihub.RootExports()`
		`self.get_user = mock.patch('kojihub.kojihub.get_user').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_user(self):`
		`username = 'test-user'`
		`self.get_user.return_value = None`

tests/test_hub/test_find_build_id.py

file modified

		`@@ -24,6 +24,9 @@`
		`self.queries = []`
		`self.query_singleValue = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_build_dict(self):`
		`build = {`
		`'name': 'test_name',`

tests/test_hub/test_getRPMDeps.py

file modified

		`@@ -11,6 +11,9 @@`
		`self.get_rpm = mock.patch('kojihub.kojihub.get_rpm').start()`
		`self.get_build = mock.patch('kojihub.kojihub.get_build').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_getRPMDeps_no_rpminfo(self):`
		`def mock_get_rpm(rpmID, strict=False):`
		`if strict:`

tests/test_hub/test_get_build_notifications.py

file modified

		`@@ -10,6 +10,9 @@`
		`self.get_user = mock.patch('kojihub.kojihub.get_user').start()`
		`self.get_build_notifications = mock.patch('kojihub.kojihub.get_build_notifications').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_loggedin_user(self):`
		`self.get_user.return_value = {'id': 1}`
		`kojihub.RootExports().getBuildNotifications(None)`

tests/test_hub/test_get_buildroot.py

file modified

		`@@ -10,6 +10,9 @@`
		`self.query_buildroots = mock.patch('kojihub.kojihub.query_buildroots').start()`
		`self.buildroot_id = 1`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_empty_buildroots_without_strict(self):`
		`self.query_buildroots.return_value = []`
		`rv = kojihub.get_buildroot(self.buildroot_id, strict=False)`

tests/test_hub/test_get_changelog_entries.py

file modified

		`@@ -17,6 +17,9 @@`
		`self.cursor = mock.MagicMock()`
		`self.os_path_exists = mock.patch('os.path.exists').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_build(self):`
		`build_id = 1`
		`self.cursor.fetchone.return_value = None`

tests/test_hub/test_get_external_repo.py

file modified

		`@@ -12,6 +12,9 @@`
		`self.get_external_repos = mock.patch('kojihub.kojihub.get_external_repos').start()`
		`self.exports = kojihub.RootExports()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_repo_with_strict(self):`
		`repo = 'test-repo'`
		`self.get_external_repos.return_value = []`

tests/test_hub/test_get_volume.py

file modified

		`@@ -11,6 +11,9 @@`
		`self.exports = kojihub.RootExports()`
		`self.lookup_name = mock.patch('kojihub.kojihub.lookup_name').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_non_exist_volume_with_strict(self):`
		`volume = ['test-volume']`
		`self.lookup_name.return_value = None`

tests/test_hub/test_grant_permissions.py

file modified

		`@@ -32,6 +32,9 @@`
		`'status': 0, 'usertype': 0}`
		`self.perm_info = {'id': 1, 'name': self.perms_name}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_grant_permission_wrong_format(self):`
		`perms_name = 'test-perms+'`

tests/test_hub/test_group_pkg_operations.py

file modified

		`@@ -61,6 +61,9 @@`
		`self.context_db.event_id = 42`
		`self.context_db.session.user_id = 24`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_grp_pkg_add_previous_changed(self):`
		`self.lookup_tag.return_value = self.taginfo`
		`self.lookup_group.return_value = self.groupinfo`

tests/test_hub/test_group_req_operations.py

file modified

		`@@ -61,6 +61,9 @@`
		`self.context_db.event_id = 42`
		`self.context_db.session.user_id = 24`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_grp_req_add_not_previous(self):`
		`self.lookup_tag.return_value = self.taginfo`
		`self.lookup_group.side_effect = [self.groupinfo, self.reqinfo]`

tests/test_hub/test_import_image_internal.py

file modified

		`@@ -22,6 +22,7 @@`

		`def tearDown(self):`
		`shutil.rmtree(self.tempdir)`
		`+ mock.patch.stopall()`

		`def test_basic(self):`
		`task = mock.MagicMock()`

tests/test_hub/test_list_builds.py

file modified

		`@@ -86,6 +86,9 @@`
		`'volume_name': 'DEFAULT',`
		`'draft': False},]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_wrong_package(self):`
		`package = 'test-package'`
		`self.get_package_id.return_value = None`

tests/test_hub/test_list_tagged.py

file modified

		`@@ -27,6 +27,9 @@`
		`'release': '1.fc35', 'state': 1, 'tag_id': 1, 'tag_name': 'test-tag',`
		`'task_id': None, 'version': '2.52', 'volume_id': 0, 'volume_name': 'DEFAULT'}]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getQuery(self, args, *kwargs):`
		`query = QP(args, *kwargs)`
		`query.execute = mock.MagicMock()`

tests/test_hub/test_list_tagged_archives.py

file modified

		`@@ -48,6 +48,9 @@`
		`'volume_name': 'DEFAULT'}]`
		`]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getQuery(self, args, *kwargs):`
		`query = QP(args, *kwargs)`
		`query.execute = mock.MagicMock()`

tests/test_hub/test_list_tagged_rpms.py

file modified

		`@@ -47,6 +47,9 @@`
		`'volume_name': 'DEFAULT'}]`
		`]`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getQuery(self, args, *kwargs):`
		`query = QP(args, *kwargs)`
		`query.execute = mock.MagicMock()`

tests/test_hub/test_models/test_host.py

file modified

		`@@ -32,6 +32,9 @@`
		`self.queries = []`
		`self.query_execute = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`@mock.patch('kojihub.kojihub.context')`
		`def test_instantiation_not_a_host(self, context):`
		`context.session.getHostId.return_value = None`

tests/test_hub/test_multicall.py

file modified

		`@@ -15,6 +15,9 @@`

		`class TestMulticall(unittest.TestCase):`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_multicall(self):`
		`self.context_db = mock.patch('kojihub.db.context').start()`
		`kojixmlrpc.kojihub = mock.MagicMock()`

tests/test_hub/test_new_maven_build.py

file modified

		`@@ -26,6 +26,9 @@`
		`'build_id': 2,`
		`}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getInsert(self, args, *kwargs):`
		`insert = IP(args, *kwargs)`
		`insert.execute = self.insert_execute`

tests/test_hub/test_new_win_build.py

file modified

		`@@ -31,6 +31,9 @@`
		`'extra': {'extra_key': 'extra_value'},`
		`}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def getInsert(self, args, *kwargs):`
		`insert = IP(args, *kwargs)`
		`insert.execute = self.insert_execute`

tests/test_hub/test_query_buildroots.py

file modified

		`@@ -20,6 +20,9 @@`
		`self.queries = []`
		`self.query_execute = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_query_buildroots(self):`
		`self.query_execute.side_effect = [[7], [7], [7], []]`
		`self.repo_references.return_value = [{'id': 7, 'host_id': 1, 'create_event': 333,`

tests/test_hub/test_query_rpm_sigs.py

file modified

		`@@ -35,6 +35,9 @@`
		`'size': 25644,`
		`'version': '3.3'}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_rpm_dict(self):`
		`rinfo_dict = {`
		`'arch': 'x86_64',`

tests/test_hub/test_restart_hosts.py

file modified

		`@@ -14,6 +14,9 @@`
		`self.context.session.assertPerm = mock.MagicMock()`
		`self.make_task = mock.patch('kojihub.kojihub.make_task').start()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_options_is_none(self):`
		`self.make_task.return_value = 13`
		`rv = self.exports.restartHosts()`

tests/test_hub/test_set_user_status.py

file modified

		`@@ -23,6 +23,9 @@`
		`self.context.session.assertPerm = mock.MagicMock()`
		`self.update_execute = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_wrong_status(self):`
		`status = 111`
		`with self.assertRaises(koji.GenericError) as cm:`

tests/test_hub/test_verify_names.py

file modified

		`@@ -15,6 +15,9 @@`
		`self.context.opts = {'MaxNameLengthInternal': 15,`
		`'RegexNameInternal.compiled': re.compile('^[A-Za-z0-9/_.+-]+$')}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_verify_name_internal_integer_type(self):`
		`expected_error = "Name should be string"`
		`with self.assertRaises(koji.GenericError) as cm:`
		`@@ -43,6 +46,9 @@`
		`self.context.opts = {'MaxNameLengthInternal': 15,`
		`'RegexUserName.compiled': re.compile('^[A-Za-z0-9/_.@-]+$')}`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_verify_user_type_name(self):`
		`expected_error = "Name should be string"`
		`with self.assertRaises(koji.GenericError) as cm:`

tests/test_hub/utils.py

file modified

+25 -20

		`@@ -2,46 +2,51 @@`
		`import unittest`

		`import kojihub`
		`+ import kojihub.db`

		`- QP = kojihub.QueryProcessor`
		`+`
		`+ def get_qp_init(testcase):`
		`+ orig_qp_init = kojihub.db.QueryProcessor.__init__`
		`+`
		`+ def my_qp_init(_query, a, *kw):`
		`+ _query.execute = mock.MagicMock()`
		`+ _query.execute.return_value = testcase.qp_execute_return_value`
		`+ _query.execute.side_effect = testcase.qp_execute_side_effect`
		`+ _query.executeOne = mock.MagicMock()`
		`+ _query.executeOne.return_value = testcase.qp_execute_one_return_value`
		`+ _query.executeOne.side_effect = testcase.qp_execute_one_side_effect`
		`+ _query.singleValue = mock.MagicMock()`
		`+ _query.singleValue.return_value = testcase.qp_single_value_return_value`
		`+ _query.iterate = mock.MagicMock()`
		`+ _query.iterate.return_value = testcase.qp_iterate_return_value`
		`+ testcase.queries.append(_query)`
		`+ return orig_qp_init(_query, a, *kw)`
		`+`
		`+ return my_qp_init`


		`class DBQueryTestCase(unittest.TestCase):`

		`def setUp(self):`
		`- mock.patch.stopall()`
		`self.qp_execute_return_value = []`
		`self.qp_execute_side_effect = None`
		`self.qp_execute_one_return_value = []`
		`self.qp_execute_one_side_effect = None`
		`self.qp_single_value_return_value = None`
		`self.qp_iterate_return_value = None`
		`- self.QueryProcessor = mock.patch('kojihub.kojihub.QueryProcessor',`
		`- side_effect=self.get_query).start()`
		`+`
		`+ # patch init to catch queries regardless of how QP is imported`
		`+ new_init = get_qp_init(self)`
		`+ self.qp_init = mock.patch('kojihub.db.QueryProcessor.__init__', new=new_init).start()`
		`+`
		`self.queries = []`

		`def tearDown(self):`
		`mock.patch.stopall()`
		`- self.reset_query()`

		`def reset_query(self):`
		`del self.queries[:]`

		`- def get_query(self, args, *kwargs):`
		`- query = QP(args, *kwargs)`
		`- query.execute = mock.MagicMock()`
		`- query.execute.return_value = self.qp_execute_return_value`
		`- query.execute.side_effect = self.qp_execute_side_effect`
		`- query.executeOne = mock.MagicMock()`
		`- query.executeOne.return_value = self.qp_execute_one_return_value`
		`- query.executeOne.side_effect = self.qp_execute_one_side_effect`
		`- query.singleValue = mock.MagicMock()`
		`- query.singleValue.return_value = self.qp_single_value_return_value`
		`- query.iterate = mock.MagicMock()`
		`- query.iterate.return_value = self.qp_iterate_return_value`
		`- self.queries.append(query)`
		`- return query`
		`-`
		`def assertQueryEqual(self, query, **kwargs):`
		`for k, v in kwargs.items():`
		`self.assertEqual(getattr(query, k, None), v)`

tests/test_lib/test_auth.py

file modified

		`@@ -44,6 +44,9 @@`
		`# start with "assert"`
		`self.context.session.assertLogin = mock.MagicMock()`

		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`def test_instance(self):`
		`"""Simple kojihub.auth.Session instance"""`
		`self.context.opts = {`

mikem commented a year ago

The previous only worked for code from kojihub.py. Any hub module that imported QueryProcessor differently would not see the mock. This approach mocks the class in place so that all imports will see it.

While here, I noticed this bit of test code:

    def setUp(self):
        mock.patch.stopall()

It makes no sense to stop mock patches in setUp. At this point, there should be no active patches unless other tests are misbehaving. As it turns out, a great many of our tests were failing to properly clean up. Removing the above stopall broke several tests, which was probably what prompted #2758.

Having stray mocks lingering after a unit test is completed could potentially mask a later unit test, or as seen here break one in a hard to debug way.

Fixes: https://pagure.io/koji/issue/4071
Related: https://pagure.io/koji/issue/2758

tkopecek commented a year ago

:thumbsup:
Maybe changing test_cli.utils.CliTestCase.tearDown and test_hub.utils.DBQueryTestCase.tearDown to do this by default?

Metadata Update from @tkopecek:
- Pull-request tagged with: no_qe

a year ago

mikem commented a year ago

Maybe changing test_cli.utils.CliTestCase.tearDown and test_hub.utils.DBQueryTestCase.tearDown to do this by default?

DBQueryTestCase.tearDown does mock.patch.stopall(). Part of the issue here is that other test cases that did not use this class did not.

CliTestCase doesn't have a setUp or tearDown method and uses decorators or context managers for all its mocking. I suppose we could add a tearDown anyway, but that feels like masking a problem.

I looked, but didn't see a way to declare any sort of global setup or teardown in the unit test framework

mikem commented a year ago

You're right though that there could be cli test cases that don't properly clean up. I only checked for / fixed the hub ones.

tkopecek commented a year ago

Commit f5c9ca9 fixes this pull-request

Pull-Request has been merged by tkopecek

a year ago

Metadata

Assignee

None

Tags

koji

Source Code

Documentation

#4068 Rework mocking of QueryProcessor in DBQueryTestCase Merged a year ago by tkopecek. Opened a year ago by mikem. mikem/koji fix-qp-mock into master

Metadata

no_qe

Changes Summary 98

#4068 Rework mocking of QueryProcessor in DBQueryTestCase

Merged a year ago by tkopecek. Opened a year ago by mikem.

mikem/koji fix-qp-mock into master