From cbb815631105120a00660eaaaf5492c9f9903555 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Dec 07 2018 10:37:36 +0000 Subject: Add tests for use of anonymous koji session This patch also clean and rewrite some tests that are relative to use anonymous koji session. Signed-off-by: Chenxiong Qi --- diff --git a/module_build_service/scheduler/producer.py b/module_build_service/scheduler/producer.py index 83f28db..b96ff6b 100644 --- a/module_build_service/scheduler/producer.py +++ b/module_build_service/scheduler/producer.py @@ -67,7 +67,7 @@ class MBSProducer(PollingProducer): if conf.system == 'koji': # We don't do this on behalf of users - koji_session = KojiModuleBuilder.get_session(conf, None, login=True) + koji_session = KojiModuleBuilder.get_session(conf, None, login=False) log.info('Querying tasks for statuses:') res = models.ComponentBuild.query.filter_by( state=koji.BUILD_STATES['BUILDING']).options( @@ -302,7 +302,7 @@ class MBSProducer(PollingProducer): now = datetime.utcnow() - koji_session = KojiModuleBuilder.get_session(config, None, login=False) + koji_session = KojiModuleBuilder.get_session(config, None) for target in koji_session.getBuildTargets(): koji_tag = target["dest_tag_name"] module = session.query(models.ModuleBuild).filter_by( diff --git a/tests/test_builder/test_koji.py b/tests/test_builder/test_koji.py index 2888664..8a92855 100644 --- a/tests/test_builder/test_koji.py +++ b/tests/test_builder/test_koji.py @@ -95,6 +95,16 @@ class TestKojiBuilder: self.config.koji_repository_url = conf.koji_repository_url self.module = module_build_service.models.ModuleBuild.query.filter_by(id=2).one() + self.p_read_config = patch('koji.read_config', return_value={ + 'authtype': 'kerberos', + 'timeout': 60, + 'server': 'http://koji.example.com/' + }) + self.mock_read_config = self.p_read_config.start() + + def teardown_method(self, test_method): + self.p_read_config.stop() + def test_tag_to_repo(self): """ Test that when a repo msg hits us and we have no match, that we do nothing gracefully. @@ -314,9 +324,9 @@ class TestKojiBuilder: expected_calls = [mock.call(1, 'foo'), mock.call(2, 'foo'), mock.call(1, 'bar')] assert mock_session.untagBuild.mock_calls == expected_calls - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_build_weights(self, get_session): - session = MagicMock() + @patch('koji.ClientSession') + def test_get_build_weights(self, ClientSession): + session = ClientSession.return_value session.getLoggedInUser.return_value = {"id": 123} session.multiCall.side_effect = [ # getPackageID response @@ -327,7 +337,6 @@ class TestKojiBuilder: [[{'1': [], '2': [], '3': [{'weight': 1.0}, {'weight': 1.0}]}], [{'1': [], '2': [], '3': [{'weight': 1.0}, {'weight': 1.0}]}]] ] - get_session.return_value = session weights = KojiModuleBuilder.get_build_weights(["httpd", "apr"]) assert weights == {"httpd": 2, "apr": 2} @@ -335,9 +344,12 @@ class TestKojiBuilder: expected_calls = [mock.call(456), mock.call(789)] assert session.getTaskDescendents.mock_calls == expected_calls - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_build_weights_no_task_id(self, get_session): - session = MagicMock() + # getLoggedInUser requires to a logged-in session + session.krb_login.assert_called_once() + + @patch('koji.ClientSession') + def test_get_build_weights_no_task_id(self, ClientSession): + session = ClientSession.return_value session.getLoggedInUser.return_value = {"id": 123} session.multiCall.side_effect = [ # getPackageID response @@ -348,17 +360,17 @@ class TestKojiBuilder: [[{'1': [], '2': [], '3': [{'weight': 1.0}, {'weight': 1.0}]}]] ] session.getAverageBuildDuration.return_value = None - get_session.return_value = session weights = KojiModuleBuilder.get_build_weights(["httpd", "apr"]) assert weights == {"httpd": 2, "apr": 1.5} expected_calls = [mock.call(456)] assert session.getTaskDescendents.mock_calls == expected_calls + session.krb_login.assert_called_once() - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_build_weights_no_build(self, get_session): - session = MagicMock() + @patch('koji.ClientSession') + def test_get_build_weights_no_build(self, ClientSession): + session = ClientSession.return_value session.getLoggedInUser.return_value = {"id": 123} session.multiCall.side_effect = [ # getPackageID response @@ -369,21 +381,20 @@ class TestKojiBuilder: [[{'1': [], '2': [], '3': [{'weight': 1.0}, {'weight': 1.0}]}]] ] session.getAverageBuildDuration.return_value = None - get_session.return_value = session weights = KojiModuleBuilder.get_build_weights(["httpd", "apr"]) assert weights == {"httpd": 2, "apr": 1.5} expected_calls = [mock.call(456)] assert session.getTaskDescendents.mock_calls == expected_calls + session.krb_login.assert_called_once() - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_build_weights_listBuilds_failed(self, get_session): - session = MagicMock() + @patch('koji.ClientSession') + def test_get_build_weights_listBuilds_failed(self, ClientSession): + session = ClientSession.return_value session.getLoggedInUser.return_value = {"id": 123} session.multiCall.side_effect = [[[1], [2]], []] session.getAverageBuildDuration.return_value = None - get_session.return_value = session weights = KojiModuleBuilder.get_build_weights(["httpd", "apr"]) assert weights == {"httpd": 1.5, "apr": 1.5} @@ -393,14 +404,14 @@ class TestKojiBuilder: mock.call(packageID=2, userID=123, state=1, queryOpts={'limit': 1, 'order': '-build_id'})] assert session.listBuilds.mock_calls == expected_calls + session.krb_login.assert_called_once() - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_build_weights_getPackageID_failed(self, get_session): - session = MagicMock() + @patch('koji.ClientSession') + def test_get_build_weights_getPackageID_failed(self, ClientSession): + session = ClientSession.return_value session.getLoggedInUser.return_value = {"id": 123} session.multiCall.side_effect = [[], []] session.getAverageBuildDuration.return_value = None - get_session.return_value = session weights = KojiModuleBuilder.get_build_weights(["httpd", "apr"]) assert weights == {"httpd": 1.5, "apr": 1.5} @@ -408,13 +419,15 @@ class TestKojiBuilder: expected_calls = [mock.call("httpd"), mock.call("apr")] assert session.getPackageID.mock_calls == expected_calls - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_build_weights_getLoggedInUser_failed(self, get_session): - session = MagicMock() + session.krb_login.assert_called_once() + + @patch('koji.ClientSession') + def test_get_build_weights_getLoggedInUser_failed(self, ClientSession): + session = ClientSession.return_value session.getAverageBuildDuration.return_value = None - get_session.return_value = session weights = KojiModuleBuilder.get_build_weights(["httpd", "apr"]) assert weights == {"httpd": 1.5, "apr": 1.5} + session.krb_login.assert_called_once() @patch.object(conf, 'base_module_arches', new={"platform:xx": ["x86_64", "i686"]}) @@ -533,9 +546,9 @@ class TestKojiBuilder: expected_calls = [] assert session.packageListBlock.mock_calls == expected_calls - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_built_rpms_in_module_build(self, get_session): - session = MagicMock() + @patch('koji.ClientSession') + def test_get_built_rpms_in_module_build(self, ClientSession): + session = ClientSession.return_value session.listTaggedRPMS.return_value = ([ {'build_id': 735939, 'name': 'tar', 'extra': None, 'arch': 'ppc64le', 'buildtime': 1533299221, 'id': 6021394, 'epoch': 2, 'version': '1.30', @@ -547,7 +560,6 @@ class TestKojiBuilder: 'metadata_only': False, 'release': '4.el8+1308+551bfa71', 'buildroot_id': 4321122, 'payloadhash': '0621ab2091256d21c47dcac868e7fc2a', 'size': 878684}], []) - get_session.return_value = session # Module builds generated by init_data uses generic modulemd file and # the module's name/stream/version/context does not have to match it. @@ -562,6 +574,7 @@ class TestKojiBuilder: ret = KojiModuleBuilder.get_built_rpms_in_module_build(mmd) assert set(ret) == set( ['bar-2:1.30-4.el8+1308+551bfa71', 'tar-2:1.30-4.el8+1308+551bfa71']) + session.assert_not_called() @pytest.mark.parametrize('br_filtered_rpms,expected', ( ( @@ -588,9 +601,9 @@ class TestKojiBuilder: [] ), )) - @patch('module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session') - def test_get_filtered_rpms_on_self_dep(self, get_session, br_filtered_rpms, expected): - session = MagicMock() + @patch('koji.ClientSession') + def test_get_filtered_rpms_on_self_dep(self, ClientSession, br_filtered_rpms, expected): + session = ClientSession.return_value session.listTaggedRPMS.return_value = ( [ { @@ -633,11 +646,11 @@ class TestKojiBuilder: } ] ) - get_session.return_value = session reuse_component_init_data() current_module = module_build_service.models.ModuleBuild.query.get(3) rv = KojiModuleBuilder._get_filtered_rpms_on_self_dep(current_module, br_filtered_rpms) assert set(rv) == set(expected) + session.assert_not_called() @pytest.mark.parametrize('cg_enabled,cg_devel_enabled', [ (False, False), @@ -666,6 +679,18 @@ class TestKojiBuilder: else: mock_koji_cg.koji_import.assert_not_called() + @patch('koji.ClientSession') + def test_get_anonymous_session(self, ClientSession): + mbs_config = mock.Mock(koji_profile='koji', koji_config='conf/koji.conf') + session = KojiModuleBuilder.get_session(mbs_config, 'someone', login=False) + assert ClientSession.return_value == session + assert ClientSession.return_value.krb_login.assert_not_called + + @patch('koji.ClientSession') + def test_ensure_builder_use_a_logged_in_koji_session(self, ClientSession): + builder = KojiModuleBuilder('owner', MagicMock(), conf, 'module-tag', []) + builder.koji_session.krb_login.assert_called_once() + class TestGetDistTagSRPM: """Test KojiModuleBuilder.get_disttag_srpm""" diff --git a/tests/test_content_generator.py b/tests/test_content_generator.py index 3c3835e..144f732 100644 --- a/tests/test_content_generator.py +++ b/tests/test_content_generator.py @@ -31,7 +31,7 @@ import module_build_service.messaging import module_build_service.scheduler.handlers.repos # noqa from module_build_service import models, conf, build_logs, Modulemd, glib -from mock import patch, Mock, MagicMock, call, mock_open +from mock import patch, Mock, call, mock_open import kobo.rpmlib from tests import init_data @@ -56,6 +56,13 @@ class TestBuild: module.cg_build_koji_tag = "f27-module-candidate" self.cg = KojiContentGenerator(module, conf) + self.p_read_config = patch('koji.read_config', return_value={ + 'authtype': 'kerberos', + 'timeout': 60, + 'server': 'http://koji.example.com/' + }) + self.mock_read_config = self.p_read_config.start() + # Ensure that there is no build log from other tests try: file_path = build_logs.path(self.cg.module) @@ -64,6 +71,8 @@ class TestBuild: pass def teardown_method(self, test_method): + self.p_read_config.stop() + # Necessary to restart the twisted reactor for the next test. import sys del sys.modules['twisted.internet.reactor'] @@ -76,7 +85,7 @@ class TestBuild: except OSError: pass - @patch("module_build_service.builder.KojiContentGenerator.get_session") + @patch("koji.ClientSession") @patch("subprocess.Popen") @patch("subprocess.check_output", return_value='1.4') @patch("pkg_resources.get_distribution") @@ -86,12 +95,11 @@ class TestBuild: "_koji_rpms_in_tag")) @pytest.mark.parametrize("devel", (False, True)) def test_get_generator_json(self, rpms_in_tag, machine, distro, pkg_res, coutput, popen, - get_session, devel): + ClientSession, devel): """ Test generation of content generator json """ - koji_session = MagicMock() + koji_session = ClientSession.return_value koji_session.getUser.return_value = GET_USER_RV koji_session.getTag.return_value = {"arches": ""} - get_session.return_value = koji_session distro.return_value = ("Fedora", "25", "Twenty Five") machine.return_value = "i686" pkg_res.return_value = Mock() @@ -131,7 +139,10 @@ class TestBuild: assert ret["build"]["name"] == "nginx-devel" assert ret["build"]["extra"]["typeinfo"]["module"]["name"] == "nginx-devel" - @patch("module_build_service.builder.KojiContentGenerator.get_session") + # Ensure an anonymous Koji session works + koji_session.krb_login.assert_not_called() + + @patch("koji.ClientSession") @patch("subprocess.Popen") @patch("subprocess.check_output", return_value='1.4') @patch("pkg_resources.get_distribution") @@ -140,12 +151,11 @@ class TestBuild: @patch(("module_build_service.builder.KojiContentGenerator.KojiContentGenerator." "_koji_rpms_in_tag")) def test_get_generator_json_no_log(self, rpms_in_tag, machine, distro, pkg_res, coutput, popen, - get_session): + ClientSession): """ Test generation of content generator json """ - koji_session = MagicMock() + koji_session = ClientSession.return_value koji_session.getUser.return_value = GET_USER_RV koji_session.getTag.return_value = {"arches": ""} - get_session.return_value = koji_session distro.return_value = ("Fedora", "25", "Twenty Five") machine.return_value = "i686" pkg_res.return_value = Mock() @@ -174,6 +184,9 @@ class TestBuild: rpms_in_tag.assert_called_once() assert expected_output == ret + # Anonymous koji session should work well. + koji_session.krb_login.assert_not_called() + def test_prepare_file_directory(self): """ Test preparation of directory with output files """ dir_path = self.cg._prepare_file_directory() @@ -193,26 +206,27 @@ class TestBuild: with open(path.join(dir_path, "modulemd.i686.txt")) as mmd: assert len(mmd.read()) == 255 - @patch("module_build_service.builder.KojiContentGenerator.get_session") - def test_tag_cg_build(self, get_session): + @patch("koji.ClientSession") + def test_tag_cg_build(self, ClientSession): """ Test that the CG build is tagged. """ - koji_session = MagicMock() + koji_session = ClientSession.return_value koji_session.getUser.return_value = GET_USER_RV koji_session.getTag.return_value = {'id': 123} - get_session.return_value = koji_session self.cg._tag_cg_build() koji_session.getTag.assert_called_once_with(self.cg.module.cg_build_koji_tag) koji_session.tagBuild.assert_called_once_with(123, "nginx-0-2.10e50d06") - @patch("module_build_service.builder.KojiContentGenerator.get_session") - def test_tag_cg_build_fallback_to_default_tag(self, get_session): + # tagBuild requires logging into a session in advance. + koji_session.krb_login.assert_called_once() + + @patch("koji.ClientSession") + def test_tag_cg_build_fallback_to_default_tag(self, ClientSession): """ Test that the CG build is tagged to default tag. """ - koji_session = MagicMock() + koji_session = ClientSession.return_value koji_session.getUser.return_value = GET_USER_RV koji_session.getTag.side_effect = [{}, {'id': 123}] - get_session.return_value = koji_session self.cg._tag_cg_build() @@ -221,30 +235,35 @@ class TestBuild: call(conf.koji_cg_default_build_tag)] koji_session.tagBuild.assert_called_once_with(123, "nginx-0-2.10e50d06") - @patch("module_build_service.builder.KojiContentGenerator.get_session") - def test_tag_cg_build_no_tag_set(self, get_session): + # tagBuild requires logging into a session in advance. + koji_session.krb_login.assert_called_once() + + @patch("koji.ClientSession") + def test_tag_cg_build_no_tag_set(self, ClientSession): """ Test that the CG build is not tagged when no tag set. """ - koji_session = MagicMock() + koji_session = ClientSession.return_value koji_session.getUser.return_value = GET_USER_RV koji_session.getTag.side_effect = [{}, {'id': 123}] - get_session.return_value = koji_session self.cg.module.cg_build_koji_tag = None self.cg._tag_cg_build() koji_session.tagBuild.assert_not_called() + # tagBuild requires logging into a session in advance. + koji_session.krb_login.assert_called_once() - @patch("module_build_service.builder.KojiContentGenerator.get_session") - def test_tag_cg_build_no_tag_available(self, get_session): + @patch("koji.ClientSession") + def test_tag_cg_build_no_tag_available(self, ClientSession): """ Test that the CG build is not tagged when no tag available. """ - koji_session = MagicMock() + koji_session = ClientSession.return_value koji_session.getUser.return_value = GET_USER_RV koji_session.getTag.side_effect = [{}, {}] - get_session.return_value = koji_session self.cg._tag_cg_build() koji_session.tagBuild.assert_not_called() + # tagBuild requires logging into a session in advance. + koji_session.krb_login.assert_called_once() @patch("module_build_service.builder.KojiContentGenerator.open", create=True) def test_get_arch_mmd_output(self, patched_open): @@ -313,9 +332,9 @@ class TestBuild: 'type': 'file' } - @patch("module_build_service.builder.KojiContentGenerator.get_session") - def test_koji_rpms_in_tag(self, get_session): - koji_session = MagicMock() + @patch("koji.ClientSession") + def test_koji_rpms_in_tag(self, ClientSession): + koji_session = ClientSession.return_value koji_session.getUser.return_value = GET_USER_RV koji_session.getTag.return_value = {"arches": "x86_64"} @@ -379,7 +398,6 @@ class TestBuild: [{'license': 'MIT'}], [{'license': 'GPL'}]] ] - get_session.return_value = koji_session rpms = self.cg._koji_rpms_in_tag("tag") for rpm in rpms: @@ -391,6 +409,9 @@ class TestBuild: assert rpm["exclusivearch"] == ["x86_64"] assert rpm["license"] == "GPL" + # Listing tagged RPMs does not require to log into a session + koji_session.krb_login.assert_not_called() + def _add_test_rpm(self, nevra, srpm_name=None, multilib=None, koji_srpm_name=None, excludearch=None, exclusivearch=None, license=None): diff --git a/tests/test_scheduler/test_module_wait.py b/tests/test_scheduler/test_module_wait.py index a1c2635..86b04fb 100644 --- a/tests/test_scheduler/test_module_wait.py +++ b/tests/test_scheduler/test_module_wait.py @@ -88,12 +88,11 @@ class TestModuleWait: @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", return_value={'build': [], 'srpm-build': []}) - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") @patch("module_build_service.builder.GenericBuilder.create_from_module") @patch('module_build_service.resolver.DBResolver') @patch('module_build_service.resolver.GenericResolver') def test_new_repo_called_when_macros_reused( - self, generic_resolver, resolver, create_builder, koji_get_session, dbg): + self, generic_resolver, resolver, create_builder, dbg): """ Test that newRepo is called when module-build-macros build is reused. """ @@ -101,7 +100,6 @@ class TestModuleWait: scheduler_init_data() koji_session = mock.MagicMock() koji_session.newRepo.return_value = 123456 - koji_get_session.return_value = koji_session builder = mock.MagicMock() builder.koji_session = koji_session @@ -130,12 +128,11 @@ class TestModuleWait: @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", return_value={'build': [], 'srpm-build': []}) - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") @patch("module_build_service.builder.GenericBuilder.create_from_module") @patch('module_build_service.resolver.DBResolver') @patch('module_build_service.resolver.GenericResolver') def test_new_repo_not_called_when_macros_not_reused( - self, generic_resolver, resolver, create_builder, koji_get_session, dbg): + self, generic_resolver, resolver, create_builder, dbg): """ Test that newRepo is called everytime for module-build-macros """ @@ -143,7 +140,6 @@ class TestModuleWait: scheduler_init_data() koji_session = mock.MagicMock() koji_session.newRepo.return_value = 123456 - koji_get_session.return_value = koji_session builder = mock.MagicMock() builder.koji_session = koji_session @@ -166,12 +162,11 @@ class TestModuleWait: @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", return_value={'build': [], 'srpm-build': []}) - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") @patch("module_build_service.builder.GenericBuilder.create_from_module") @patch('module_build_service.resolver.DBResolver') @patch('module_build_service.resolver.GenericResolver') def test_set_cg_build_koji_tag_fallback_to_default( - self, generic_resolver, resolver, create_builder, koji_get_session, dbg): + self, generic_resolver, resolver, create_builder, dbg): """ Test that build.cg_build_koji_tag fallbacks to default tag. """ @@ -183,7 +178,6 @@ class TestModuleWait: scheduler_init_data() koji_session = mock.MagicMock() koji_session.newRepo.return_value = 123456 - koji_get_session.return_value = koji_session builder = mock.MagicMock() builder.koji_session = koji_session @@ -213,14 +207,13 @@ class TestModuleWait: ]) @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", return_value={'build': [], 'srpm-build': []}) - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") @patch("module_build_service.builder.GenericBuilder.create_from_module") @patch('module_build_service.resolver.DBResolver') @patch('module_build_service.resolver.GenericResolver') @patch("module_build_service.config.Config.base_module_names", new_callable=mock.PropertyMock, return_value=set(["base-runtime", "platform"])) def test_set_cg_build_koji_tag( - self, cfg, generic_resolver, resolver, create_builder, koji_get_session, dbg, + self, cfg, generic_resolver, resolver, create_builder, dbg, koji_cg_tag_build, expected_cg_koji_build_tag): """ Test that build.cg_build_koji_tag is set. @@ -233,7 +226,6 @@ class TestModuleWait: scheduler_init_data() koji_session = mock.MagicMock() koji_session.newRepo.return_value = 123456 - koji_get_session.return_value = koji_session builder = mock.MagicMock() builder.koji_session = koji_session diff --git a/tests/test_scheduler/test_poller.py b/tests/test_scheduler/test_poller.py index 2477d85..e1f0858 100644 --- a/tests/test_scheduler/test_poller.py +++ b/tests/test_scheduler/test_poller.py @@ -32,21 +32,27 @@ from datetime import datetime, timedelta @patch("module_build_service.builder.GenericBuilder.default_buildroot_groups", return_value={'build': [], 'srpm-build': []}) @patch("module_build_service.scheduler.consumer.get_global_consumer") -@patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") @patch("module_build_service.builder.GenericBuilder.create_from_module") class TestPoller: def setup_method(self, test_method): reuse_component_init_data() + self.p_read_config = patch('koji.read_config', return_value={ + 'authtype': 'kerberos', + 'timeout': 60, + 'server': 'http://koji.example.com/' + }) + self.mock_read_config = self.p_read_config.start() + def teardown_method(self, test_method): + self.p_read_config.stop() clean_database() @pytest.mark.parametrize('fresh', [True, False]) @patch('module_build_service.utils.batches.start_build_component') def test_process_paused_module_builds(self, start_build_component, create_builder, - koji_get_session, global_consumer, - dbg, fresh): + global_consumer, dbg, fresh): """ Tests general use-case of process_paused_module_builds. """ @@ -54,9 +60,6 @@ class TestPoller: consumer.incoming = queue.Queue() global_consumer.return_value = consumer - koji_session = mock.MagicMock() - koji_get_session.return_value = koji_session - builder = mock.MagicMock() create_builder.return_value = builder @@ -95,9 +98,9 @@ class TestPoller: assert len(start_build_component.mock_calls) == expected_build_calls - def test_trigger_new_repo_when_failed(self, create_builder, - koji_get_session, global_consumer, - dbg): + @patch("koji.ClientSession") + def test_trigger_new_repo_when_failed( + self, ClientSession, create_builder, global_consumer, dbg): """ Tests that we call koji_sesion.newRepo when newRepo task failed. """ @@ -105,11 +108,10 @@ class TestPoller: consumer.incoming = queue.Queue() global_consumer.return_value = consumer - koji_session = mock.MagicMock() + koji_session = ClientSession.return_value koji_session.getTag = lambda tag_name: {'name': tag_name} koji_session.getTaskInfo.return_value = {'state': koji.TASK_STATES['FAILED']} koji_session.newRepo.return_value = 123456 - koji_get_session.return_value = koji_session builder = mock.MagicMock() builder.buildroot_ready.return_value = False @@ -129,9 +131,9 @@ class TestPoller: koji_session.newRepo.assert_called_once_with( "module-testmodule-master-20170219191323-c40c156c-build") - def test_trigger_new_repo_when_succeded(self, create_builder, - koji_get_session, global_consumer, - dbg): + @patch('koji.ClientSession') + def test_trigger_new_repo_when_succeeded( + self, ClientSession, create_builder, global_consumer, dbg): """ Tests that we do not call koji_sesion.newRepo when newRepo task succeeded. @@ -140,11 +142,10 @@ class TestPoller: consumer.incoming = queue.Queue() global_consumer.return_value = consumer - koji_session = mock.MagicMock() + koji_session = ClientSession.return_value koji_session.getTag = lambda tag_name: {'name': tag_name} koji_session.getTaskInfo.return_value = {'state': koji.TASK_STATES['CLOSED']} koji_session.newRepo.return_value = 123456 - koji_get_session.return_value = koji_session builder = mock.MagicMock() builder.buildroot_ready.return_value = False @@ -169,7 +170,7 @@ class TestPoller: assert module_build.new_repo_task_id == 0 def test_process_paused_module_builds_waiting_for_repo( - self, create_builder, koji_get_session, global_consumer, dbg): + self, create_builder, global_consumer, dbg): """ Tests that process_paused_module_builds does not start new batch when we are waiting for repo. @@ -178,9 +179,6 @@ class TestPoller: consumer.incoming = queue.Queue() global_consumer.return_value = consumer - koji_session = mock.MagicMock() - koji_get_session.return_value = koji_session - builder = mock.MagicMock() create_builder.return_value = builder @@ -205,72 +203,161 @@ class TestPoller: for component in components: assert component.state is None - def test_delete_old_koji_targets( - self, create_builder, koji_get_session, global_consumer, dbg): - """ - Tests that we delete koji target when time_completed is older than - koji_target_delete_time value. - """ + @patch('koji.ClientSession') + def test_old_build_targets_are_not_associated_with_any_module_builds( + self, ClientSession, create_builder, global_consumer, dbg): consumer = mock.MagicMock() consumer.incoming = queue.Queue() global_consumer.return_value = consumer - for state_name, state in models.BUILD_STATES.items(): - koji_session = mock.MagicMock() - koji_session.getBuildTargets.return_value = [ - {'dest_tag_name': 'module-tag', 'id': 852, 'name': 'module-tag'}, - {'dest_tag_name': 'f26', 'id': 853, 'name': 'f26'}, - {'dest_tag_name': 'module-tag2', 'id': 853, 'name': 'f26'}] - koji_get_session.return_value = koji_session + koji_session = ClientSession.return_value + # No created module build has any of these tags. + koji_session.getBuildTargets.return_value = [ + {'dest_tag_name': 'module-xxx-1'}, + {'dest_tag_name': 'module-yyy-2'}, + ] + + hub = mock.MagicMock() + poller = MBSProducer(hub) + poller.delete_old_koji_targets(conf, db.session) - builder = mock.MagicMock() - create_builder.return_value = builder + koji_session.deleteBuildTarget.assert_not_called() - # Change the batch to 2, so the module build is in state where - # it is not building anything, but the state is "build". - module_build = models.ModuleBuild.query.filter_by(id=3).one() - module_build.state = state - module_build.koji_tag = "module-tag" - module_build.time_completed = datetime.utcnow() - module_build.new_repo_task_id = 123456 - db.session.commit() + @patch('koji.ClientSession') + def test_dont_delete_base_module_build_target( + self, ClientSession, create_builder, global_consumer, dbg): + module_build = models.ModuleBuild.query.filter_by(id=3).one() + + koji_session = ClientSession.return_value + # No created module build has any of these tags. + koji_session.getBuildTargets.return_value = [ + {'dest_tag_name': module_build.koji_tag}, + ] + + consumer = mock.MagicMock() + consumer.incoming = queue.Queue() + global_consumer.return_value = consumer + + # If module build's name is one of base module names, build target + # should not be deleted. + with patch.object(conf, 'base_module_names', new=[module_build.name]): - # Poll :) hub = mock.MagicMock() poller = MBSProducer(hub) poller.delete_old_koji_targets(conf, db.session) - module_build = models.ModuleBuild.query.filter_by(id=3).one() - db.session.refresh(module_build) - module_build.time_completed = datetime.utcnow() - timedelta(hours=23) - db.session.commit() - poller.delete_old_koji_targets(conf, db.session) + koji_session.deleteBuildTarget.assert_not_called() + + @patch('koji.ClientSession') + def test_dont_delete_build_target_for_unfinished_module_builds( + self, ClientSession, create_builder, global_consumer, dbg): + module_build = models.ModuleBuild.query.filter_by(id=3).one() - # deleteBuildTarget should not be called, because time_completed is - # set to "now". - assert not koji_session.deleteBuildTarget.called + koji_session = ClientSession.return_value + # No created module build has any of these tags. + koji_session.getBuildTargets.return_value = [ + {'dest_tag_name': module_build.koji_tag}, + ] - # Try removing non-modular target - should not happen - db.session.refresh(module_build) - module_build.koji_tag = "module-tag2" - module_build.time_completed = datetime.utcnow() - timedelta(hours=25) + consumer = mock.MagicMock() + consumer.incoming = queue.Queue() + global_consumer.return_value = consumer + + # Each time when a module build is in one of these state, build target + # should not be deleted. + for state in ['init', 'wait', 'build']: + module_build.state = state db.session.commit() + + hub = mock.MagicMock() + poller = MBSProducer(hub) poller.delete_old_koji_targets(conf, db.session) - assert not koji_session.deleteBuildTarget.called - # Refresh our module_build object and set time_completed 25 hours ago - db.session.refresh(module_build) - module_build.time_completed = datetime.utcnow() - timedelta(hours=25) - module_build.koji_tag = "module-tag" - db.session.commit() + koji_session.deleteBuildTarget.assert_not_called() + + @patch('koji.ClientSession') + def test_only_delete_build_target_with_allowed_koji_tag_prefix( + self, ClientSession, create_builder, global_consumer, dbg): + module_build_2 = models.ModuleBuild.query.filter_by(id=2).one() + module_build_3 = models.ModuleBuild.query.filter_by(id=3).one() + + # Only module build 1's build target should be deleted. + module_build_2.koji_tag = 'module-tag1' + module_build_2.state = models.BUILD_STATES['done'] + # Ensure to exceed the koji_target_delete_time easily later for deletion + module_build_2.time_completed = datetime.utcnow() - timedelta(hours=24) + module_build_3.koji_tag = 'f28' + db.session.commit() + db.session.refresh(module_build_2) + db.session.refresh(module_build_3) + + koji_session = ClientSession.return_value + # No created module build has any of these tags. + koji_session.getBuildTargets.return_value = [ + { + 'id': 1, + 'dest_tag_name': module_build_2.koji_tag, + 'name': module_build_2.koji_tag + }, + { + 'id': 2, + 'dest_tag_name': module_build_3.koji_tag, + 'name': module_build_3.koji_tag + }, + ] + + consumer = mock.MagicMock() + consumer.incoming = queue.Queue() + global_consumer.return_value = consumer + with patch.object(conf, 'koji_tag_prefixes', + new=['module', 'another-prefix']): + with patch.object(conf, 'koji_target_delete_time', new=60): + hub = mock.MagicMock() + poller = MBSProducer(hub) + poller.delete_old_koji_targets(conf, db.session) + + koji_session.deleteBuildTarget.assert_called_once_with(1) + koji_session.krb_login.assert_called_once() + + @patch('koji.ClientSession') + def test_cant_delete_build_target_if_not_reach_delete_time( + self, ClientSession, create_builder, global_consumer, dbg): + module_build_2 = models.ModuleBuild.query.filter_by(id=2).one() + + # Only module build 1's build target should be deleted. + module_build_2.koji_tag = 'module-tag1' + module_build_2.state = models.BUILD_STATES['done'] + # Ensure to exceed the koji_target_delete_time easily later for deletion + module_build_2.time_completed = datetime.utcnow() - timedelta(minutes=5) + db.session.commit() + db.session.refresh(module_build_2) + + koji_session = ClientSession.return_value + # No created module build has any of these tags. + koji_session.getBuildTargets.return_value = [ + { + 'id': 1, + 'dest_tag_name': module_build_2.koji_tag, + 'name': module_build_2.koji_tag + }, + ] + + consumer = mock.MagicMock() + consumer.incoming = queue.Queue() + global_consumer.return_value = consumer + + with patch.object(conf, 'koji_tag_prefixes', new=['module']): + # Use default koji_target_delete_time in config. That time is long + # enough for test. + hub = mock.MagicMock() + poller = MBSProducer(hub) poller.delete_old_koji_targets(conf, db.session) - if state_name in ["done", "ready", "failed"]: - koji_session.deleteBuildTarget.assert_called_once_with(852) + koji_session.deleteBuildTarget.assert_not_called() - def test_process_waiting_module_build(self, create_builder, koji_get_session, - global_consumer, dbg): + def test_process_waiting_module_build( + self, create_builder, global_consumer, dbg): """ Test that processing old waiting module builds works. """ consumer = mock.MagicMock() @@ -300,8 +387,8 @@ class TestPoller: # ensure the time_modified was changed. assert module_build.time_modified > original - def test_process_waiting_module_build_not_old_enough(self, create_builder, koji_get_session, - global_consumer, dbg): + def test_process_waiting_module_build_not_old_enough( + self, create_builder, global_consumer, dbg): """ Test that we do not process young waiting builds. """ consumer = mock.MagicMock() @@ -329,8 +416,8 @@ class TestPoller: # Ensure we did *not* process the 9 minute-old build. assert consumer.incoming.qsize() == 0 - def test_process_waiting_module_build_none_found(self, create_builder, koji_get_session, - global_consumer, dbg): + def test_process_waiting_module_build_none_found( + self, create_builder, global_consumer, dbg): """ Test nothing happens when no module builds are waiting. """ consumer = mock.MagicMock() @@ -349,8 +436,8 @@ class TestPoller: # Ensure we did *not* process any of the non-waiting builds. assert consumer.incoming.qsize() == 0 - def test_cleanup_stale_failed_builds(self, create_builder, koji_get_session, - global_consumer, dbg): + def test_cleanup_stale_failed_builds( + self, create_builder, global_consumer, dbg): """ Test that one of the two module builds gets to the garbage state when running cleanup_stale_failed_builds. """ @@ -402,8 +489,8 @@ class TestPoller: 'module-build-macros-0.1-1.module+0+d027b723' ]) - def test_cleanup_stale_failed_builds_no_components(self, create_builder, koji_get_session, - global_consumer, dbg): + def test_cleanup_stale_failed_builds_no_components( + self, create_builder, global_consumer, dbg): """ Test that a module build without any components built gets to the garbage state when running cleanup_stale_failed_builds. """ @@ -445,8 +532,8 @@ class TestPoller: @pytest.mark.parametrize('test_state', [models.BUILD_STATES[state] for state in conf.cleanup_stuck_builds_states]) - def test_cancel_stuck_module_builds(self, create_builder, koji_get_session, global_consumer, - dbg, test_state): + def test_cancel_stuck_module_builds( + self, create_builder, global_consumer, dbg, test_state): module_build1 = models.ModuleBuild.query.get(1) module_build1.state = test_state diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 01a615e..38fd247 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -25,7 +25,7 @@ from datetime import datetime import module_build_service.scm -from mock import patch, PropertyMock, Mock +from mock import patch, PropertyMock from shutil import copyfile from os import path, mkdir from os.path import dirname @@ -417,8 +417,8 @@ class TestViews: for key, part in zip(nsvc_keys, nsvc_parts): assert item[key] == part - @patch("module_build_service.builder.KojiModuleBuilder.KojiModuleBuilder.get_session") - def test_query_builds_with_binary_rpm(self, mock_get_session): + @patch("koji.ClientSession") + def test_query_builds_with_binary_rpm(self, ClientSession): """ Test for querying MBS with the binary rpm filename. MBS should return all the modules, which contain the rpm. @@ -431,13 +431,17 @@ class TestViews: {"name": "non-module-tag"}, {"name": "module-testmodule-master-20170109091357-78e4a6fd"}] - mock_session = Mock() + mock_session = ClientSession.return_value mock_session.getRPM.return_value = mock_rpm_md mock_session.listTags.return_value = mock_tags - mock_get_session.return_value = mock_session rpm = quote('module-build-macros-0.1-1.testmodule_master_20170303190726.src.rpm') - rv = self.client.get('/module-build-service/1/module-builds/?rpm=%s' % rpm) + with patch('koji.read_config', return_value={ + 'authtype': 'kerberos', + 'timeout': 60, + 'server': 'http://koji.example.com/' + }): + rv = self.client.get('/module-build-service/1/module-builds/?rpm=%s' % rpm) results = json.loads(rv.data)['items'] assert len(results) == 2 @@ -448,6 +452,8 @@ class TestViews: "module-build-macros-0.1-1.testmodule_master_20170303190726.src.rpm") mock_session.listTags.assert_called_once_with(mock_rpm_md["build_id"]) + mock_session.krb_login.assert_not_called() + @patch('module_build_service.config.Config.system', new_callable=PropertyMock, return_value="invalid_builder") def test_query_builds_with_binary_rpm_not_koji(self, mock_builder):