From f6471d322ab5a13297590c531190e047e754e40b Mon Sep 17 00:00:00 2001 From: Dwayne Steinke Date: Mon, 8 Jan 2024 15:34:23 +0100 Subject: [PATCH] feat: Use original commit message on one commit updates (#1) If a template update only applies one commit to the target repository, the original commit message and body can be applied without modifications instead of the summary with all changes for bulk edits. This change introduces a detection if the template update only contains one commit and uses the original message accordingly. This will allow maintainers to update repositories in smaller increments, such that the commit history may partially mirror the commit history of the template. --------- Co-authored-by: Jan-Frederik Schmidt --- .../_update_projects.py | 38 ++++++++-- tests/unit/test_update_projects.py | 76 ++++++++++++++++--- 2 files changed, 99 insertions(+), 15 deletions(-) diff --git a/src/voraus_template_updater/_update_projects.py b/src/voraus_template_updater/_update_projects.py index 96b33eb..a3a8b69 100644 --- a/src/voraus_template_updater/_update_projects.py +++ b/src/voraus_template_updater/_update_projects.py @@ -180,14 +180,19 @@ def _update_project( cruft.update(Path(local_repo.working_dir), checkout=project.template_branch) + template_commit_messages = _get_template_commit_messages(project, github_access_token) local_repo.git.add(all=True) - local_repo.index.commit(PR_TITLE) + if len(template_commit_messages) == 1: + local_repo.index.commit(template_commit_messages[0]) + else: + local_repo.index.commit(PR_TITLE) local_repo.git.push("--set-upstream", "origin", branch) - pr_body = _get_pr_body(project, github_access_token) + pr_title = _get_pr_title(template_commit_messages) + pr_body = _get_pr_body(project, template_commit_messages) pull_request = remote_repo.create_pull( - base=remote_repo.default_branch, head=branch.name, title=PR_TITLE, body=pr_body + base=remote_repo.default_branch, head=branch.name, title=pr_title, body=pr_body ) _logger.info( @@ -198,7 +203,7 @@ def _update_project( return pull_request -def _get_pr_body(project: Project, github_access_token: str) -> str: +def _get_template_commit_messages(project: Project, github_access_token: str) -> List[str]: with TemporaryDirectory() as tmp_path: template_repo = _clone_repo(project.template_url, github_access_token, Path(tmp_path)) @@ -222,12 +227,33 @@ def _get_pr_body(project: Project, github_access_token: str) -> str: r"\(#(\d+)\)", lambda match: f"([PR]({link.format(match.groups()[0])}))", message ) + return commit_messages + + +def _get_pr_title(template_commit_messages: List[str]) -> str: + if len(template_commit_messages) == 1: + message = template_commit_messages[0] + return message.strip().splitlines()[0] + + return PR_TITLE + + +def _get_pr_body(project: Project, template_commit_messages: List[str]) -> str: + if len(template_commit_messages) == 1: + message = template_commit_messages[0] + lines = message.strip().splitlines() + if len(lines) > 1: + return "".join(message.strip().splitlines()[1:]) + return "" + # Indent all lines after the first line of a commit message by two spaces # This leads to nicer bullet points in the pull request body - commit_messages = ["\n ".join(commit_message.strip().splitlines()) for commit_message in commit_messages] + template_commit_messages = [ + "\n ".join(commit_message.strip().splitlines()) for commit_message in template_commit_messages + ] # Construct a pull request message containing the PR header and a bullet point list of changes and their explanation - return PR_BODY_HEADER.format(project.template_branch) + "- " + "\n\n- ".join(commit_messages) + "\n" + return PR_BODY_HEADER.format(project.template_branch) + "- " + "\n\n- ".join(template_commit_messages) + "\n" if __name__ == "__main__": # pragma: no cover diff --git a/tests/unit/test_update_projects.py b/tests/unit/test_update_projects.py index f178a4b..3a30b5c 100644 --- a/tests/unit/test_update_projects.py +++ b/tests/unit/test_update_projects.py @@ -18,6 +18,8 @@ _check_and_update_projects, _clone_repo, _get_cruft_config, + _get_pr_body, + _get_pr_title, ) ORGANIZATION = "dummy-organization" @@ -318,11 +320,13 @@ def test_up_to_date_project( assert summary.projects[0].status == Status.UP_TO_DATE +@pytest.mark.parametrize("is_incremental_update", [True, False], ids=["Incremental Update", "Bulk Update"]) @patch("voraus_template_updater._update_projects.cruft.update") @patch("voraus_template_updater._update_projects.cruft.check") def test_update_project_success( cruft_check_mock: MagicMock, cruft_update_mock: MagicMock, + is_incremental_update: bool, repo_mock: MagicMock, cloned_repo_mocks: List[MagicMock], cruft_config: CruftConfig, @@ -343,7 +347,8 @@ def _create_commit_mock(i: int) -> MagicMock: return commit_mock cloned_template_repo.git.rev_parse.return_value = "newest_commit" - cloned_template_repo.iter_commits.return_value = [_create_commit_mock(i) for i in range(2)] + number_new_commits = 1 if is_incremental_update else 2 + cloned_template_repo.iter_commits.return_value = [_create_commit_mock(i) for i in range(number_new_commits)] with caplog.at_level(logging.INFO): summary = _check_and_update_projects(ORGANIZATION) @@ -359,18 +364,31 @@ def _create_commit_mock(i: int) -> MagicMock: cloned_template_repo.iter_commits.assert_called_once_with(f"{cruft_config.commit}..newest_commit") cloned_project_repo.git.add.assert_called_with(all=True) - cloned_project_repo.index.commit.assert_called_with(PR_TITLE) + if is_incremental_update: + cloned_project_repo.index.commit.assert_called_with( + "Commit title ([PR](some-template-url/pull/0))\n\nDescription 0\n" + ) + else: + cloned_project_repo.index.commit.assert_called_with(PR_TITLE) cloned_project_repo.git.push.assert_called_with("--set-upstream", "origin", branch_mock) - expected_pr_body = ( - "Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch." - "\n\n" - "- Commit title ([PR](some-template-url/pull/0))\n \n Description 0\n\n" - "- Commit title ([PR](some-template-url/pull/1))\n \n Description 1\n" - ) + if is_incremental_update: + expected_pr_body = "Description 0" + else: + expected_pr_body = ( + "Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch." + "\n\n" + "- Commit title ([PR](some-template-url/pull/0))\n \n Description 0\n\n" + "- Commit title ([PR](some-template-url/pull/1))\n \n Description 1\n" + ) + + if is_incremental_update: + expected_pr_title = "Commit title ([PR](some-template-url/pull/0))" + else: + expected_pr_title = PR_TITLE repo_mock.create_pull.assert_called_once_with( - base=repo_mock.default_branch, head=branch_mock.name, title=PR_TITLE, body=expected_pr_body + base=repo_mock.default_branch, head=branch_mock.name, title=expected_pr_title, body=expected_pr_body ) assert caplog.record_tuples[1] == ( @@ -384,3 +402,43 @@ def _create_commit_mock(i: int) -> MagicMock: assert summary.projects[0].status == Status.UPDATED_THIS_RUN assert summary.projects[0].pull_request is not None assert summary.projects[0].pull_request.html_url == "https://some-pr.com" + + +@pytest.mark.parametrize( + argnames=["commit_messages", "expected_title"], + argvalues=[ + ([], PR_TITLE), + (["Commit Title"], "Commit Title"), + (["Commit Title\n\nCommit Body\n"], "Commit Title"), + (["Commit Title\nCommit Body\n"], "Commit Title"), + (["Commit Title 1\n\nCommit Body 1\n", "Commit Title 2\n Commit body 1.\n"], PR_TITLE), + ], +) +def test_get_pr_title(commit_messages: List[str], expected_title: str) -> None: + assert _get_pr_title(commit_messages) == expected_title + + +@pytest.mark.parametrize( + argnames=["commit_messages", "expected_body"], + argvalues=[ + ( + [], + "Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch." + "\n\n- \n", + ), # Empty commit should never happen. But this test shows, that we do not crash. + (["Commit Title"], ""), + (["Commit Title\n\nCommit Body"], "Commit Body"), + (["Commit Title\nCommit Body\n"], "Commit Body"), + ( + ["Commit Title", "Commit title\n\nCommit body.\n"], + "Contains the following changes to get up-to-date with the newest version of the template's 'dev' branch." + "\n\n" + "- Commit Title\n\n" + "- Commit title\n \n Commit body.\n", + ), + ], +) +def test_get_pr_body(commit_messages: List[str], expected_body: str) -> None: + project = MagicMock() + project.template_branch = "dev" + assert _get_pr_body(project, commit_messages) == expected_body