From a90d88c525a46d708dcb9de6c4a62c70c85b72e9 Mon Sep 17 00:00:00 2001 From: David Reed Date: Mon, 31 Jul 2023 15:35:09 -0600 Subject: [PATCH] Add create_pull_request_on_conflict option to automerge tasks (#3632) Co-authored-by: Ben French --- AUTHORS.rst | 1 + cumulusci/tasks/github/merge.py | 60 +++++++++----- cumulusci/tasks/github/tests/test_merge.py | 92 ++++++++++++++++++++++ 3 files changed, 132 insertions(+), 21 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 1ac2d6bda3..ba6dd22338 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -36,3 +36,4 @@ For example: * Ed Rivas (jerivas) * Gustavo Tandeciarz (dcinzona) +* Ben French (BenjaminFrench) diff --git a/cumulusci/tasks/github/merge.py b/cumulusci/tasks/github/merge.py index 9d9b7bfbd8..1cc32887fa 100644 --- a/cumulusci/tasks/github/merge.py +++ b/cumulusci/tasks/github/merge.py @@ -34,6 +34,9 @@ class MergeBranch(BaseGithubTask): "update_future_releases": { "description": "If true, then include release branches that are not the lowest release number even if they are not child branches. Defaults to False." }, + "create_pull_request_on_conflict": { + "description": "If true, then create a pull request when a merge conflict arises. Defaults to True." + }, } def _init_options(self, kwargs): @@ -58,6 +61,12 @@ def _init_options(self, kwargs): self.options["update_future_releases"] = process_bool_arg( self.options.get("update_future_releases") or False ) + if "create_pull_request_on_conflict" not in self.options: + self.options["create_pull_request_on_conflict"] = True + else: + self.options["create_pull_request_on_conflict"] = process_bool_arg( + self.options.get("create_pull_request_on_conflict") + ) def _init_task(self): super()._init_task() @@ -245,30 +254,39 @@ def _merge(self, branch_name, source, commit): if e.code != http.client.CONFLICT: raise - if branch_name in self._get_existing_prs( - self.options["source_branch"], self.options["branch_prefix"] - ): - self.logger.info( - f"Merge conflict on branch {branch_name}: merge PR already exists" - ) - return - - try: - pull = self.repo.create_pull( - title=f"Merge {source} into {branch_name}", - base=branch_name, - head=source, - body="This pull request was automatically generated because " - "an automated merge hit a merge conflict", - ) + if self.options["create_pull_request_on_conflict"]: + self._create_conflict_pull_request(branch_name, source) + else: self.logger.info( - f"Merge conflict on branch {branch_name}: created pull request #{pull.number}" - ) - except github3.exceptions.UnprocessableEntity as e: - self.logger.error( - f"Error creating merge conflict pull request to merge {source} into {branch_name}:\n{e.response.text}" + f"Merge conflict on branch {branch_name}: skipping pull request creation" ) + def _create_conflict_pull_request(self, branch_name, source): + """Attempt to create a pull request from source into branch_name if merge operation encounters a conflict""" + if branch_name in self._get_existing_prs( + self.options["source_branch"], self.options["branch_prefix"] + ): + self.logger.info( + f"Merge conflict on branch {branch_name}: merge PR already exists" + ) + return + + try: + pull = self.repo.create_pull( + title=f"Merge {source} into {branch_name}", + base=branch_name, + head=source, + body="This pull request was automatically generated because " + "an automated merge hit a merge conflict", + ) + self.logger.info( + f"Merge conflict on branch {branch_name}: created pull request #{pull.number}" + ) + except github3.exceptions.UnprocessableEntity as e: + self.logger.error( + f"Error creating merge conflict pull request to merge {source} into {branch_name}:\n{e.response.text}" + ) + def _is_source_branch_direct_descendent(self, branch_name): """Returns True if branch is a direct descendent of the source branch""" source_dunder_count = self.options["source_branch"].count("__") diff --git a/cumulusci/tasks/github/tests/test_merge.py b/cumulusci/tasks/github/tests/test_merge.py index f84b62f296..80743f7cb3 100644 --- a/cumulusci/tasks/github/tests/test_merge.py +++ b/cumulusci/tasks/github/tests/test_merge.py @@ -278,6 +278,45 @@ def test_task_output__feature_branch_merge_conflict(self): assert expected_log == actual_log assert 7 == len(responses.calls) + @responses.activate + def test_task_output__feature_branch_merge_conflict_skip_pull_requests(self): + self._mock_repo() + self._mock_branch(self.branch) + self.mock_pulls() + branch_name = "feature/a-test" + branches = [] + branches.append(self._get_expected_branch("main")) + branches.append(self._get_expected_branch(branch_name)) + branches = self._mock_branches(branches) + self._mock_compare( + base=branches[1]["name"], + head=self.project_config.repo_commit, + files=[{"filename": "test.txt"}], + ) + self._mock_merge(http.client.CONFLICT) + self._mock_pull_create(1, 2) + with LogCapture() as log: + task = self._create_task( + task_config={ + "options": { + "create_pull_request_on_conflict": "False", + } + } + ) + task() + actual_log = self._get_log_lines(log) + + expected_log = log_header() + [ + ("DEBUG", "Skipping branch main: is source branch"), + ("DEBUG", "Found descendents of main to update: ['feature/a-test']"), + ( + "INFO", + "Merge conflict on branch feature/a-test: skipping pull request creation", + ), + ] + assert expected_log == actual_log + assert 5 == len(responses.calls) + @responses.activate def test_merge__error_on_merge_conflict_pr(self): self._mock_repo() @@ -402,6 +441,59 @@ def test_task_output__main_parent_with_child_pr(self): assert expected_log == actual_log assert 7 == len(responses.calls) + @responses.activate + def test_task_output__main_parent_with_child_skip_pull_requests(self): + self._mock_repo() + self._mock_branch(self.branch) + # branches + parent_branch_name = "feature/a-test" + child_branch_name = "feature/a-test__a-child" + branches = [] + branches.append(self._get_expected_branch("main")) + branches.append(self._get_expected_branch(parent_branch_name)) + branches.append(self._get_expected_branch(child_branch_name)) + branches = self._mock_branches(branches) + # pull request + pull = self._get_expected_pull_request(1, 2) + pull["base"]["ref"] = parent_branch_name + pull["base"]["sha"] = branches[1]["commit"]["sha"] + pull["head"]["ref"] = child_branch_name + self.mock_pulls(pulls=[pull]) + # compare + self._mock_compare( + base=parent_branch_name, + head=self.project_config.repo_commit, + files=[{"filename": "test.txt"}], + ) + # merge + self._mock_merge(http.client.CONFLICT) + # create PR + self._mock_pull_create(1, 2) + with LogCapture() as log: + task = self._create_task( + task_config={ + "options": { + "create_pull_request_on_conflict": "False", + } + } + ) + task() + actual_log = self._get_log_lines(log) + expected_log = log_header() + [ + ("DEBUG", "Skipping branch main: is source branch"), + ( + "DEBUG", + "Skipping branch feature/a-test__a-child: is not a direct descendent of main", + ), + ("DEBUG", "Found descendents of main to update: ['feature/a-test']"), + ( + "INFO", + "Merge conflict on branch feature/a-test: skipping pull request creation", + ), + ] + assert expected_log == actual_log + assert 5 == len(responses.calls) + @responses.activate def test_task_output__main_merge_to_feature(self): """Tests that commits to the main branch are merged to the expected feature branches"""