From 3117a196f7cd4692b4414b29eb4826afbb396163 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Sep 21 2018 19:48:32 +0000 Subject: Support remote PR and running pagure-ci Remote PR in pagure have no project_from since they are remote but up until now pagure-ci's code was relying on the fullname of the project_from. With this commit we add support for remote PR which, when trying to trigger a new CI run, can specify a PR UID instead of a project fullname. The logic will then retrieve the corresponding PR, check if it is a remote PR or not and depend on this use either the project_from or the project_to field of the pull-request object. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 8438423..89a27f4 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1367,7 +1367,7 @@ def add_pull_request_comment( and not request.project.private ): tasks_services.trigger_ci_build.delay( - project_name=request.project_from.fullname, + pr_uid=request.uid, cause=request.id, branch=request.branch_from, ci_type=request.project.ci_hook.ci_type, @@ -1390,7 +1390,7 @@ def add_pull_request_comment( and request.project.ci_hook.active_pr ): tasks_services.trigger_ci_build.delay( - project_name=request.project_from.fullname, + pr_uid=request.uid, cause=request.id, branch=request.branch_from, ci_type=request.project.ci_hook.ci_type, @@ -1929,7 +1929,7 @@ def new_pull_request( and not request.project.private ): tasks_services.trigger_ci_build.delay( - project_name=request.project_from.fullname, + pr_uid=request.uid, cause=request.id, branch=request.branch_from, ci_type=request.project.ci_hook.ci_type, diff --git a/pagure/lib/tasks_services.py b/pagure/lib/tasks_services.py index 8106fd0..cafcc6a 100644 --- a/pagure/lib/tasks_services.py +++ b/pagure/lib/tasks_services.py @@ -382,13 +382,26 @@ def load_json_commits_to_db( @conn.task(queue=pagure_config.get("CI_CELERY_QUEUE", None), bind=True) @pagure_task -def trigger_ci_build(self, session, project_name, cause, branch, ci_type): +def trigger_ci_build( + self, session, cause, branch, ci_type, project_name=None, pr_uid=None): """ Triggers a new run of the CI system on the specified pull-request. """ pagure.lib.plugins.get_plugin("Pagure CI") + if not pr_uid and not project_name: + _log.debug("No PR UID nor project name specified, can't trigger CI") + session.close() + return + + if pr_uid: + pr = pagure.lib.get_request_by_uid(session, pr_uid) + if pr.remote: + project_name = pr.project_to.fullname + else: + project_name = pr.project_from.fullname + user, namespace, project_name = split_project_fullname(project_name) _log.info("Pagure-CI: Looking for project: %s", project_name) diff --git a/tests/test_pagure_flask_ui_remote_pr.py b/tests/test_pagure_flask_ui_remote_pr.py index aae5428..ad0af76 100644 --- a/tests/test_pagure_flask_ui_remote_pr.py +++ b/tests/test_pagure_flask_ui_remote_pr.py @@ -184,7 +184,7 @@ class PagureRemotePRtests(tests.Modeltests): @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) def test_new_remote_pr_auth(self): - """ Test creating a new remote PR un-authenticated. """ + """ Test creating a new remote PR authenticated. """ tests.create_projects(self.session) tests.create_projects_git( @@ -268,7 +268,6 @@ class PagureRemotePRtests(tests.Modeltests): project = pagure.lib.get_authorized_project(self.session, 'test') self.assertEqual(len(project.requests), 1) - @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) def test_new_remote_pr_empty_target(self): """ Test creating a new remote PR authenticated against an empty @@ -411,6 +410,124 @@ class PagureRemotePRtests(tests.Modeltests): 'Overview - test - Pagure', output_text ) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + @patch('pagure.lib.tasks_services.trigger_ci_build') + def test_new_remote_pr_ci_off(self, trigger_ci): + """ Test creating a new remote PR when CI is not configured. """ + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + self.set_up_git_repo() + + # Before + self.session = pagure.lib.create_session(self.dbpath) + project = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(project.requests), 0) + + # Create a remote PR + user = tests.FakeUser(username='foo') + with tests.user_set(self.app.application, user): + + csrf_token = self.get_csrf() + data = { + 'csrf_token': csrf_token, + 'title': 'Remote PR title', + 'branch_from': 'feature', + 'branch_to': 'master', + 'git_repo': os.path.join(self.newpath, 'test'), + } + output = self.app.post( + '/test/diff/remote', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + + data['confirm'] = 1 + output = self.app.post( + '/test/diff/remote', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '#1', + output_text) + self.assertIn( + '
\n', output_text) + self.assertIn( + '
\n', output_text) + self.assertNotIn( + '
\n', output_text) + + # Remote PR Created + self.session = pagure.lib.create_session(self.dbpath) + project = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(project.requests), 1) + trigger_ci.assert_not_called() + + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + @patch('pagure.lib.tasks_services.trigger_ci_build') + def test_new_remote_pr_ci_on(self, trigger_ci): + """ Test creating a new remote PR when CI is configured. """ + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + self.set_up_git_repo() + + # Before + self.session = pagure.lib.create_session(self.dbpath) + project = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(project.requests), 0) + + # Create a remote PR + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + + # Activate CI hook + data = { + 'active_pr': 'y', + 'ci_url': 'https://jenkins.fedoraproject.org', + 'ci_job': 'test/job', + 'ci_type': 'jenkins', + 'csrf_token': csrf_token, + } + output = self.app.post( + '/test/settings/Pagure CI', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + + user = tests.FakeUser(username='foo') + with tests.user_set(self.app.application, user): + data = { + 'csrf_token': csrf_token, + 'title': 'Remote PR title', + 'branch_from': 'feature', + 'branch_to': 'master', + 'git_repo': os.path.join(self.newpath, 'test'), + } + output = self.app.post( + '/test/diff/remote', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + + data['confirm'] = 1 + output = self.app.post( + '/test/diff/remote', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '#1', + output_text) + self.assertIn( + '
\n', output_text) + self.assertIn( + '
\n', output_text) + self.assertNotIn( + '
\n', output_text) + + # Remote PR Created + self.session = pagure.lib.create_session(self.dbpath) + project = pagure.lib.get_authorized_project(self.session, 'test') + self.assertEqual(len(project.requests), 1) + trigger_ci.assert_not_called() + if __name__ == '__main__': unittest.main(verbosity=2)