From 9bb22b480ab931d580939c447df95c505400ffa6 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Jul 17 2016 04:53:42 +0000 Subject: [PATCH 1/2] Get the test suite working again. --- diff --git a/tests/test_hub/test_delete_build.py b/tests/test_hub/test_delete_build.py index 727f2ec..d6fc103 100644 --- a/tests/test_hub/test_delete_build.py +++ b/tests/test_hub/test_delete_build.py @@ -11,7 +11,7 @@ class TestDeleteBuild(unittest.TestCase): @mock.patch('kojihub.context') @mock.patch('kojihub.get_build') def test_delete_build_raise_error(self, build, context): - + context.session.assertPerm = mock.MagicMock() references = ['tags', 'rpms', 'archives', 'images'] for ref in references: context = mock.MagicMock() @@ -27,7 +27,7 @@ class TestDeleteBuild(unittest.TestCase): @mock.patch('kojihub.context') @mock.patch('kojihub.get_build') def test_delete_build_return_false(self, build, context): - + context.session.assertPerm = mock.MagicMock() references = ['tags', 'rpms', 'archives', 'images'] for ref in references: context = mock.MagicMock() @@ -42,7 +42,7 @@ class TestDeleteBuild(unittest.TestCase): @mock.patch('kojihub.context') @mock.patch('kojihub.get_build') def test_delete_build_check_last_used_raise_error(self, build, context): - + context.session.assertPerm = mock.MagicMock() references = ['tags', 'rpms', 'archives', 'images', 'last_used'] for ref in references: context = mock.MagicMock() @@ -59,7 +59,7 @@ class TestDeleteBuild(unittest.TestCase): @mock.patch('kojihub.context') @mock.patch('kojihub.get_build') def test_delete_build_check_last_used_raise_error(self, build, context): - + context.session.assertPerm = mock.MagicMock() references = ['tags', 'rpms', 'archives', 'images', 'last_used'] for ref in references: context = mock.MagicMock() diff --git a/tests/test_hub/test_import_image_internal.py b/tests/test_hub/test_import_image_internal.py index 287c172..212d368 100644 --- a/tests/test_hub/test_import_image_internal.py +++ b/tests/test_hub/test_import_image_internal.py @@ -22,6 +22,9 @@ class TestImportImageInternal(unittest.TestCase): @mock.patch('kojihub.Task') @mock.patch('kojihub.context') def test_basic(self, context, Task, get_build, get_archive_type, import_archive, work): + task = mock.MagicMock() + task.assertHost = mock.MagicMock() + Task.return_value = task imgdata = { 'arch': 'x86_64', 'task_id': 1, @@ -51,6 +54,9 @@ class TestImportImageInternal(unittest.TestCase): @mock.patch('kojihub.Task') @mock.patch('kojihub.context') def test_with_rpm(self, context, Task, get_build, get_archive_type, import_archive, build, work, get_rpm): + task = mock.MagicMock() + task.assertHost = mock.MagicMock() + Task.return_value = task rpm = { #'location': 'foo', 'id': 6, diff --git a/tests/test_hub/test_insert_processor.py b/tests/test_hub/test_insert_processor.py index 202d914..1771681 100644 --- a/tests/test_hub/test_insert_processor.py +++ b/tests/test_hub/test_insert_processor.py @@ -33,6 +33,7 @@ class TestInsertProcessor(unittest.TestCase): def test_make_create(self, context): cursor = mock.MagicMock() context.cnx.cursor.return_value = cursor + context.session.assertLogin = mock.MagicMock() proc = kojihub.InsertProcessor('sometable', data={'foo': 'bar'}) proc.make_create(event_id=1, user_id=2) self.assertEquals(proc.data['create_event'], 1) @@ -54,6 +55,7 @@ class TestInsertProcessor(unittest.TestCase): def test_dup_check(self, context): cursor = mock.MagicMock() context.cnx.cursor.return_value = cursor + context.session.assertLogin = mock.MagicMock() proc = kojihub.InsertProcessor('sometable', data={'foo': 'bar'}) proc.dup_check() diff --git a/tests/test_plugins/test_runroot_hub.py b/tests/test_plugins/test_runroot_hub.py index 97fd1cf..dd13289 100644 --- a/tests/test_plugins/test_runroot_hub.py +++ b/tests/test_plugins/test_runroot_hub.py @@ -8,6 +8,7 @@ class TestRunrootHub(unittest.TestCase): @mock.patch('kojihub.make_task') @mock.patch('runroot_hub.context') def test_basic_invocation(self, context, make_task): + context.session.assertPerm = mock.MagicMock() runroot_hub.runroot( tagInfo='some_tag', arch='x86_64', From 013da77d01fa2f36a65ccdaca60350be7a9f8ce3 Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Jul 17 2016 04:54:57 +0000 Subject: [PATCH 2/2] Move 'build' creation earlier in the build task. Currently, when you submit a new build, the hub creates a build task', which gets picked up by a builder. Off of that 'build task', a `buildSRPMFromSCM` subtask is created. Only after that subtask succeeds do we then create a top-level `build` object to record this work before proceeding to build all of the rpms and do any necessary tagging. That all works fine. The problem is when `buildSRPMFromSCM` fails for some reason (user error, network failure, etc..). When it fails, we never create a top-level build object and therefore the `postBuildStateChange` callback never gets called (which cascades as silent failure to other systems that might be listening for that callback to know if the build fails). This commit primarily moves the creation of that top-level build object earlier in the initial build task, to just before the `buildSRPMFromSCM` subtask is created. Secondly, all the code surrounding the `buildSRPMFromSCM` task is indented to be inside a `try:` block, which will catch any failures there and propagate them to be recorded as a failure in the top-level build object. *Caveat emptor*: I haven't tested this code, but it seems reasonable to me. Is there a reason that srpm creation was included in the build task but left out of the build object? --- diff --git a/builder/kojid b/builder/kojid index 240b5ef..6fe6bf3 100755 --- a/builder/kojid +++ b/builder/kojid @@ -830,32 +830,33 @@ class BuildTask(BaseTaskHandler): if not repo_info: repo_info = self.getRepo(build_tag) #(subtask) self.event_id = self.session.getLastEvent()['id'] - srpm = self.getSRPM(src, build_tag, repo_info['id']) - h = self.readSRPMHeader(srpm) - data = koji.get_header_fields(h,['name','version','release','epoch']) - data['task_id'] = self.id - extra_arches = None - self.logger.info("Reading package config for %(name)s" % data) - pkg_cfg = self.session.getPackageConfig(dest_tag,data['name'],event=self.event_id) - self.logger.debug("%r" % pkg_cfg) - if pkg_cfg is not None: - extra_arches = pkg_cfg.get('extra_arches') - if not self.opts.get('skip_tag') and not self.opts.get('scratch'): - # Make sure package is on the list for this tag - if pkg_cfg is None: - raise koji.BuildError, "package %s not in list for tag %s" \ - % (data['name'], target_info['dest_tag_name']) - elif pkg_cfg['blocked']: - raise koji.BuildError, "package %s is blocked for tag %s" \ - % (data['name'], target_info['dest_tag_name']) - # TODO - more pre tests - archlist = self.getArchList(build_tag, h, extra=extra_arches) #let the system know about the build we're attempting if not self.opts.get('scratch'): #scratch builds do not get imported build_id = self.session.host.initBuild(data) #(initBuild raises an exception if there is a conflict) try: + srpm = self.getSRPM(src, build_tag, repo_info['id']) + h = self.readSRPMHeader(srpm) + data = koji.get_header_fields(h,['name','version','release','epoch']) + data['task_id'] = self.id + extra_arches = None + self.logger.info("Reading package config for %(name)s" % data) + pkg_cfg = self.session.getPackageConfig(dest_tag,data['name'],event=self.event_id) + self.logger.debug("%r" % pkg_cfg) + if pkg_cfg is not None: + extra_arches = pkg_cfg.get('extra_arches') + if not self.opts.get('skip_tag') and not self.opts.get('scratch'): + # Make sure package is on the list for this tag + if pkg_cfg is None: + raise koji.BuildError, "package %s not in list for tag %s" \ + % (data['name'], target_info['dest_tag_name']) + elif pkg_cfg['blocked']: + raise koji.BuildError, "package %s is blocked for tag %s" \ + % (data['name'], target_info['dest_tag_name']) + # TODO - more pre tests + archlist = self.getArchList(build_tag, h, extra=extra_arches) + self.extra_information = { "src": src, "data": data, "target": target } srpm,rpms,brmap,logs = self.runBuilds(srpm,build_tag,archlist,repo_info['id'])