From eb93ac35b3278fbda61478192b1f6a71231e8117 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 26 Feb 2024 15:03:20 +0000 Subject: [PATCH 01/32] feat: get commit message from googleapis --- library_generation/test/unit_tests.py | 16 ++++++++++++++++ library_generation/utilities.py | 26 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 625aaf8ffd..6af90332c5 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -30,6 +30,7 @@ from library_generation.model.gapic_inputs import parse as parse_build_file from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig +from library_generation.utilities import get_commit_messages script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -471,6 +472,21 @@ def test_repo_level_post_process_success(self): actual=f"{repository_path}/gapic-libraries-bom/pom.xml", ) + def test_get_commit_message_success(self): + repo_url = "https://github.com/googleapis/googleapis.git" + new_committish = "5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" + old_committish = "e04130efabc98c20c56675b0c0af603087681f48" + message = get_commit_messages( + repo_url=repo_url, + new_committish=new_committish, + old_committish=old_committish, + ) + self.assertTrue("5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" in message) + self.assertTrue("8df1cd698e2fe0b7d1c298dabafdf13aa01c4d39" in message) + self.assertTrue("87a71a910ce60009cc578eb183062fb7d62e664f" in message) + self.assertTrue("cfda64661b5e005730e9c7f24745ff2e40680647" in message) + self.assertFalse("e04130efabc98c20c56675b0c0af603087681f48" in message) + def __compare_files(self, expect: str, actual: str): with open(expect, "r") as f: expected_lines = f.readlines() diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 8af797b792..78c3e9f16e 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -17,6 +17,7 @@ import os import shutil import re +from git import Repo from pathlib import Path from lxml import etree from library_generation.model.bom_config import BomConfig @@ -485,3 +486,28 @@ def get_version_from( for line in f.readlines(): if artifact_id in line: return line.split(":")[index].strip() + + +def get_commit_messages(repo_url: str, new_committish: str, old_committish: str) -> str: + """ + Get commit messages of a repository from new_committish to + old_committish. + :param repo_url: the url of the repository. + :param new_committish: + :param old_committish: + :return: + """ + tmp_dir = "/tmp/repo" + shutil.rmtree(tmp_dir, ignore_errors=True) + os.mkdir(tmp_dir) + messages = [] + repo = Repo.clone_from(repo_url, tmp_dir) + commit = repo.commit(new_committish) + while str(commit.hexsha) != old_committish: + messages.append(f"{commit.hexsha}\n{commit.message}") + commit_parents = commit.parents + if len(commit_parents) == 0: + break + commit = commit_parents[0] + shutil.rmtree(tmp_dir, ignore_errors=True) + return "\n\n".join(messages) From df8fe507934d67821bb460de099c4c1f03d0dcba Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 1 Mar 2024 13:39:11 +0000 Subject: [PATCH 02/32] add comments --- library_generation/utilities.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 78c3e9f16e..bae726e8ec 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -492,10 +492,12 @@ def get_commit_messages(repo_url: str, new_committish: str, old_committish: str) """ Get commit messages of a repository from new_committish to old_committish. + Note that old_committish should be an ancestor of new_committish. + :param repo_url: the url of the repository. :param new_committish: :param old_committish: - :return: + :return: commit messages. """ tmp_dir = "/tmp/repo" shutil.rmtree(tmp_dir, ignore_errors=True) From 9a1be4c17d6cca9b1b18b5840d1cacceaf570179 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 1 Mar 2024 13:57:03 +0000 Subject: [PATCH 03/32] move to integration test --- library_generation/test/integration_tests.py | 16 ++++++++++++++++ library_generation/test/unit_tests.py | 16 ---------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index c552a52e4a..8fc7dd2ebb 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -26,6 +26,7 @@ from library_generation.generate_repo import generate_from_yaml from library_generation.model.generation_config import from_yaml from library_generation.test.compare_poms import compare_xml +from library_generation.utilities import get_commit_messages from library_generation.utilities import ( get_library_name, sh_util as shell_call, @@ -45,6 +46,21 @@ class IntegrationTest(unittest.TestCase): + def test_get_commit_message_success(self): + repo_url = "https://github.com/googleapis/googleapis.git" + new_committish = "5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" + old_committish = "e04130efabc98c20c56675b0c0af603087681f48" + message = get_commit_messages( + repo_url=repo_url, + new_committish=new_committish, + old_committish=old_committish, + ) + self.assertTrue("5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" in message) + self.assertTrue("8df1cd698e2fe0b7d1c298dabafdf13aa01c4d39" in message) + self.assertTrue("87a71a910ce60009cc578eb183062fb7d62e664f" in message) + self.assertTrue("cfda64661b5e005730e9c7f24745ff2e40680647" in message) + self.assertFalse("e04130efabc98c20c56675b0c0af603087681f48" in message) + def test_generate_repo(self): shutil.rmtree(f"{golden_dir}", ignore_errors=True) os.makedirs(f"{golden_dir}", exist_ok=True) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 6af90332c5..625aaf8ffd 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -30,7 +30,6 @@ from library_generation.model.gapic_inputs import parse as parse_build_file from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig -from library_generation.utilities import get_commit_messages script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -472,21 +471,6 @@ def test_repo_level_post_process_success(self): actual=f"{repository_path}/gapic-libraries-bom/pom.xml", ) - def test_get_commit_message_success(self): - repo_url = "https://github.com/googleapis/googleapis.git" - new_committish = "5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" - old_committish = "e04130efabc98c20c56675b0c0af603087681f48" - message = get_commit_messages( - repo_url=repo_url, - new_committish=new_committish, - old_committish=old_committish, - ) - self.assertTrue("5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" in message) - self.assertTrue("8df1cd698e2fe0b7d1c298dabafdf13aa01c4d39" in message) - self.assertTrue("87a71a910ce60009cc578eb183062fb7d62e664f" in message) - self.assertTrue("cfda64661b5e005730e9c7f24745ff2e40680647" in message) - self.assertFalse("e04130efabc98c20c56675b0c0af603087681f48" in message) - def __compare_files(self, expect: str, actual: str): with open(expect, "r") as f: expected_lines = f.readlines() From c1f556c9941e8c025d7df7ed50c6dd051dbd27c7 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 1 Mar 2024 14:36:26 +0000 Subject: [PATCH 04/32] add paths from generation_config --- library_generation/test/integration_tests.py | 3 ++ library_generation/utilities.py | 45 ++++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 8fc7dd2ebb..5053b7595b 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -27,6 +27,7 @@ from library_generation.model.generation_config import from_yaml from library_generation.test.compare_poms import compare_xml from library_generation.utilities import get_commit_messages +from library_generation.utilities import get_file_paths from library_generation.utilities import ( get_library_name, sh_util as shell_call, @@ -50,10 +51,12 @@ def test_get_commit_message_success(self): repo_url = "https://github.com/googleapis/googleapis.git" new_committish = "5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" old_committish = "e04130efabc98c20c56675b0c0af603087681f48" + paths = get_file_paths(f"{config_dir}/google-cloud-java/{config_name}") message = get_commit_messages( repo_url=repo_url, new_committish=new_committish, old_committish=old_committish, + paths=paths, ) self.assertTrue("5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" in message) self.assertTrue("8df1cd698e2fe0b7d1c298dabafdf13aa01c4d39" in message) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index bae726e8ec..d2a15ef85b 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -17,11 +17,12 @@ import os import shutil import re -from git import Repo +from git import Commit, Repo from pathlib import Path from lxml import etree from library_generation.model.bom_config import BomConfig from library_generation.model.generation_config import GenerationConfig +from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig from typing import List from jinja2 import Environment, FileSystemLoader @@ -34,6 +35,13 @@ group_id_tag = "groupId" artifact_tag = "artifactId" version_tag = "version" +common_protos = { + "google/api", + "google/longrunning", + "google/rpc", + "google/shopping/type", + "google/type", +} def __render(template_name: str, output_name: str, **kwargs): @@ -488,15 +496,35 @@ def get_version_from( return line.split(":")[index].strip() -def get_commit_messages(repo_url: str, new_committish: str, old_committish: str) -> str: +def get_file_paths(path_to_yaml: str) -> set[str]: + """ + Get versioned proto_path from configuration file, plus known paths of + common protos. + + :param path_to_yaml: the path to the configuration file + :return: versioned proto_path plus paths of common protos + """ + config = from_yaml(path_to_yaml) + paths = set() + for library in config.libraries: + for gapic_config in library.gapic_configs: + paths.add(gapic_config.proto_path) + return paths.union(common_protos) + + +def get_commit_messages( + repo_url: str, new_committish: str, old_committish: str, paths: set[str] +) -> str: """ Get commit messages of a repository from new_committish to - old_committish. + old_committish. Only messages of a commit which affects files in a + pre-defined set will be added. Note that old_committish should be an ancestor of new_committish. :param repo_url: the url of the repository. :param new_committish: :param old_committish: + :param paths: a set of file paths :return: commit messages. """ tmp_dir = "/tmp/repo" @@ -506,10 +534,19 @@ def get_commit_messages(repo_url: str, new_committish: str, old_committish: str) repo = Repo.clone_from(repo_url, tmp_dir) commit = repo.commit(new_committish) while str(commit.hexsha) != old_committish: - messages.append(f"{commit.hexsha}\n{commit.message}") + if __is_relevant_commit(paths, commit): + messages.append(f"{commit.hexsha}\n{commit.message}") commit_parents = commit.parents if len(commit_parents) == 0: break commit = commit_parents[0] shutil.rmtree(tmp_dir, ignore_errors=True) return "\n\n".join(messages) + + +def __is_relevant_commit(paths: set[str], commit: Commit) -> bool: + for file in commit.stats.files.keys(): + idx = file.rfind("/") + if file[:idx] in paths: + return True + return False From ccb1a2123303ae0e774536248f5a026dc50e8e4e Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sat, 2 Mar 2024 00:29:04 +0000 Subject: [PATCH 05/32] add generate_pr_description.py --- library_generation/generate_pr_description.py | 86 +++++++++++++++++++ library_generation/utilities.py | 42 ++++----- 2 files changed, 107 insertions(+), 21 deletions(-) create mode 100644 library_generation/generate_pr_description.py diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py new file mode 100644 index 0000000000..2fc9c48bd3 --- /dev/null +++ b/library_generation/generate_pr_description.py @@ -0,0 +1,86 @@ +#!/usr/bin/env python3 +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import click + +from library_generation.model.generation_config import from_yaml +from library_generation.utilities import get_commit_messages +from library_generation.utilities import get_file_paths + + +@click.group(invoke_without_command=False) +@click.pass_context +@click.version_option(message="%(version)s") +def main(ctx): + pass + + +@main.command() +@click.option( + "--generation-config-yaml", + required=True, + type=str, + help=""" + Path to generation_config.yaml that contains the metadata about + library generation. + """, +) +@click.option( + "--baseline-commit", + required=True, + type=str, + help=""" + The baseline commit from which the commit message is considered. + This commit should be an ancestor of googleapis commit in configuration. + """, +) +@click.option( + "--repo-url", + type=str, + default="https://github.com/googleapis/googleapis.git", + show_default=True, + help=""" + GitHub repository URL. + """, +) +def generate( + generation_config_yaml: str, + repo_url: str, + baseline_commit: str, +) -> str: + return generate_pr_descriptions( + generation_config_yaml=generation_config_yaml, + repo_url=repo_url, + baseline_commit=baseline_commit, + ) + + +def generate_pr_descriptions( + generation_config_yaml: str, + repo_url: str, + baseline_commit: str, +) -> str: + config = from_yaml(generation_config_yaml) + paths = get_file_paths(config) + return get_commit_messages( + repo_url=repo_url, + latest_commit=config.googleapis_commitish, + baseline_commit=baseline_commit, + paths=paths, + ) + + +if __name__ == "__main__": + main() diff --git a/library_generation/utilities.py b/library_generation/utilities.py index d2a15ef85b..73fb7b6fea 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -22,7 +22,6 @@ from lxml import etree from library_generation.model.bom_config import BomConfig from library_generation.model.generation_config import GenerationConfig -from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig from typing import List from jinja2 import Environment, FileSystemLoader @@ -143,6 +142,14 @@ def __handle_special_bom( ] +def __is_relevant_commit(paths: set[str], commit: Commit) -> bool: + for file in commit.stats.files.keys(): + idx = file.rfind("/") + if file[:idx] in paths: + return True + return False + + def create_argument(arg_key: str, arg_container: object) -> List[str]: """ Generates a list of two elements [argument, value], or returns @@ -496,15 +503,14 @@ def get_version_from( return line.split(":")[index].strip() -def get_file_paths(path_to_yaml: str) -> set[str]: +def get_file_paths(config: GenerationConfig) -> set[str]: """ Get versioned proto_path from configuration file, plus known paths of common protos. - :param path_to_yaml: the path to the configuration file + :param config: :return: versioned proto_path plus paths of common protos """ - config = from_yaml(path_to_yaml) paths = set() for library in config.libraries: for gapic_config in library.gapic_configs: @@ -513,17 +519,19 @@ def get_file_paths(path_to_yaml: str) -> set[str]: def get_commit_messages( - repo_url: str, new_committish: str, old_committish: str, paths: set[str] + repo_url: str, latest_commit: str, baseline_commit: str, paths: set[str] ) -> str: """ - Get commit messages of a repository from new_committish to - old_committish. Only messages of a commit which affects files in a - pre-defined set will be added. - Note that old_committish should be an ancestor of new_committish. + Combine commit messages of a repository from latest_commit to + baseline_commit. Only commits which change files in a pre-defined + file paths will be considered. + Note that baseline_commit should be an ancestor of latest_commit. :param repo_url: the url of the repository. - :param new_committish: - :param old_committish: + :param latest_commit: the newest commit to be considered in + selecting commit message. + :param baseline_commit: the oldest commit to be considered in + selecting commit message. This commit should be an ancestor of :param paths: a set of file paths :return: commit messages. """ @@ -532,8 +540,8 @@ def get_commit_messages( os.mkdir(tmp_dir) messages = [] repo = Repo.clone_from(repo_url, tmp_dir) - commit = repo.commit(new_committish) - while str(commit.hexsha) != old_committish: + commit = repo.commit(latest_commit) + while str(commit.hexsha) != baseline_commit: if __is_relevant_commit(paths, commit): messages.append(f"{commit.hexsha}\n{commit.message}") commit_parents = commit.parents @@ -542,11 +550,3 @@ def get_commit_messages( commit = commit_parents[0] shutil.rmtree(tmp_dir, ignore_errors=True) return "\n\n".join(messages) - - -def __is_relevant_commit(paths: set[str], commit: Commit) -> bool: - for file in commit.stats.files.keys(): - idx = file.rfind("/") - if file[:idx] in paths: - return True - return False From 057da9185d5d43c9d42e4e404ddae2b48f3a868d Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sat, 2 Mar 2024 00:29:23 +0000 Subject: [PATCH 06/32] add an integration test --- library_generation/test/integration_tests.py | 29 ++++++++++--------- .../google-cloud-java/generation_config.yaml | 14 +++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 5053b7595b..746bb029e6 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -23,11 +23,11 @@ from pathlib import Path from typing import List from typing import Dict + +from library_generation.generate_pr_description import generate_pr_descriptions from library_generation.generate_repo import generate_from_yaml from library_generation.model.generation_config import from_yaml from library_generation.test.compare_poms import compare_xml -from library_generation.utilities import get_commit_messages -from library_generation.utilities import get_file_paths from library_generation.utilities import ( get_library_name, sh_util as shell_call, @@ -49,20 +49,21 @@ class IntegrationTest(unittest.TestCase): def test_get_commit_message_success(self): repo_url = "https://github.com/googleapis/googleapis.git" - new_committish = "5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" - old_committish = "e04130efabc98c20c56675b0c0af603087681f48" - paths = get_file_paths(f"{config_dir}/google-cloud-java/{config_name}") - message = get_commit_messages( + baseline_commit = "a17d4caf184b050d50cacf2b0d579ce72c31ce74" + description = generate_pr_descriptions( + f"{config_dir}/google-cloud-java/{config_name}", repo_url=repo_url, - new_committish=new_committish, - old_committish=old_committish, - paths=paths, + baseline_commit=baseline_commit, ) - self.assertTrue("5c5ecf0eb9bd8acf5bed57fe1ce0df1770466089" in message) - self.assertTrue("8df1cd698e2fe0b7d1c298dabafdf13aa01c4d39" in message) - self.assertTrue("87a71a910ce60009cc578eb183062fb7d62e664f" in message) - self.assertTrue("cfda64661b5e005730e9c7f24745ff2e40680647" in message) - self.assertFalse("e04130efabc98c20c56675b0c0af603087681f48" in message) + # There are five commits from googleapis commitish to baseline commit, + # inclusively. Only two of the commits contain changes in proto_path + # that are in configuration. Therefore, only two commit messages should + # be included in the PR description. + self.assertFalse("a17d4caf184b050d50cacf2b0d579ce72c31ce74" in description) + self.assertFalse("9063da8b4d45339db4e2d7d92a27c6708620e694" in description) + self.assertTrue("7a9a855287b5042410c93e5a510f40efd4ce6cb1" in description) + self.assertTrue("c7fd8bd652ac690ca84f485014f70b52eef7cb9e" in description) + self.assertFalse("a17d4caf184b050d50cacf2b0d579ce72c31ce74" in description) def test_generate_repo(self): shutil.rmtree(f"{golden_dir}", ignore_errors=True) diff --git a/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml b/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml index a13860a8ca..d55a07fa8d 100644 --- a/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml +++ b/library_generation/test/resources/integration/google-cloud-java/generation_config.yaml @@ -48,3 +48,17 @@ libraries: - proto_path: google/cloud/alloydb/connectors/v1 - proto_path: google/cloud/alloydb/connectors/v1alpha - proto_path: google/cloud/alloydb/connectors/v1beta + + - api_shortname: documentai + name_pretty: Document AI + product_documentation: https://cloud.google.com/compute/docs/documentai/ + api_description: allows developers to unlock insights from your documents with machine + learning. + library_name: document-ai + release_level: stable + issue_tracker: https://issuetracker.google.com/savedsearches/559755 + GAPICs: + - proto_path: google/cloud/documentai/v1 + - proto_path: google/cloud/documentai/v1beta1 + - proto_path: google/cloud/documentai/v1beta2 + - proto_path: google/cloud/documentai/v1beta3 From da3fc4c1ae5d0828f96848039683bcddcd2b1017 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sat, 2 Mar 2024 00:29:36 +0000 Subject: [PATCH 07/32] add an unit test --- library_generation/test/unit_tests.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 625aaf8ffd..b3ef31c25b 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -30,6 +30,7 @@ from library_generation.model.gapic_inputs import parse as parse_build_file from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig +from library_generation.utilities import get_file_paths script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -214,6 +215,24 @@ def test_from_yaml_succeeds(self): self.assertEqual("google/cloud/asset/v1p5beta1", gapics[3].proto_path) self.assertEqual("google/cloud/asset/v1p7beta1", gapics[4].proto_path) + def test_get_file_paths_from_yaml_success(self): + paths = get_file_paths(f"{test_config_dir}/generation_config.yaml") + self.assertEqual( + { + "google/cloud/asset/v1", + "google/cloud/asset/v1p1beta1", + "google/cloud/asset/v1p2beta1", + "google/cloud/asset/v1p5beta1", + "google/cloud/asset/v1p7beta1", + "google/api", + "google/longrunning", + "google/rpc", + "google/shopping/type", + "google/type", + }, + paths, + ) + @parameterized.expand( [ ("BUILD_no_additional_protos.bazel", " "), From 1e6ab5e407ad2683fc25d6ce769d868af1cdd915 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sat, 2 Mar 2024 00:33:48 +0000 Subject: [PATCH 08/32] fix unit tests --- library_generation/test/unit_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index b3ef31c25b..f4a548dcc2 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -216,7 +216,7 @@ def test_from_yaml_succeeds(self): self.assertEqual("google/cloud/asset/v1p7beta1", gapics[4].proto_path) def test_get_file_paths_from_yaml_success(self): - paths = get_file_paths(f"{test_config_dir}/generation_config.yaml") + paths = get_file_paths(from_yaml(f"{test_config_dir}/generation_config.yaml")) self.assertEqual( { "google/cloud/asset/v1", From 939c513e6b12f3630b876f32b0e52e0494ac241e Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Sun, 3 Mar 2024 13:39:04 +0000 Subject: [PATCH 09/32] change func name --- library_generation/utilities.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 73fb7b6fea..3ed4168407 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -142,7 +142,7 @@ def __handle_special_bom( ] -def __is_relevant_commit(paths: set[str], commit: Commit) -> bool: +def __is_qualified_commit(paths: set[str], commit: Commit) -> bool: for file in commit.stats.files.keys(): idx = file.rfind("/") if file[:idx] in paths: @@ -542,7 +542,7 @@ def get_commit_messages( repo = Repo.clone_from(repo_url, tmp_dir) commit = repo.commit(latest_commit) while str(commit.hexsha) != baseline_commit: - if __is_relevant_commit(paths, commit): + if __is_qualified_commit(paths, commit): messages.append(f"{commit.hexsha}\n{commit.message}") commit_parents = commit.parents if len(commit_parents) == 0: From 707932752a9b84847b36407e3f8113a7a505f3ee Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 4 Mar 2024 16:28:57 +0000 Subject: [PATCH 10/32] compare result with golden file --- library_generation/test/integration_tests.py | 15 ++++++++++----- .../integration/pr-description-golden.txt | 10 ++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 library_generation/test/resources/integration/pr-description-golden.txt diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 746bb029e6..49e2cb4bae 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -17,6 +17,7 @@ import unittest from distutils.dir_util import copy_tree from distutils.file_util import copy_file +from filecmp import cmp from filecmp import dircmp from git import Repo @@ -55,15 +56,19 @@ def test_get_commit_message_success(self): repo_url=repo_url, baseline_commit=baseline_commit, ) + description_file = f"{config_dir}/pr-description.txt" + if os.path.isfile(f"{description_file}"): + os.remove(f"{description_file}") + with open(f"{description_file}", "w+") as f: + f.write(description) # There are five commits from googleapis commitish to baseline commit, # inclusively. Only two of the commits contain changes in proto_path # that are in configuration. Therefore, only two commit messages should # be included in the PR description. - self.assertFalse("a17d4caf184b050d50cacf2b0d579ce72c31ce74" in description) - self.assertFalse("9063da8b4d45339db4e2d7d92a27c6708620e694" in description) - self.assertTrue("7a9a855287b5042410c93e5a510f40efd4ce6cb1" in description) - self.assertTrue("c7fd8bd652ac690ca84f485014f70b52eef7cb9e" in description) - self.assertFalse("a17d4caf184b050d50cacf2b0d579ce72c31ce74" in description) + self.assertTrue( + cmp(f"{config_dir}/pr-description-golden.txt", f"{description_file}") + ) + os.remove(f"{description_file}") def test_generate_repo(self): shutil.rmtree(f"{golden_dir}", ignore_errors=True) diff --git a/library_generation/test/resources/integration/pr-description-golden.txt b/library_generation/test/resources/integration/pr-description-golden.txt new file mode 100644 index 0000000000..6229629b98 --- /dev/null +++ b/library_generation/test/resources/integration/pr-description-golden.txt @@ -0,0 +1,10 @@ +7a9a855287b5042410c93e5a510f40efd4ce6cb1 +feat: expose model_type in v1 processor, so that user can see the model_type after get or list processor version + +PiperOrigin-RevId: 603727585 + + +c7fd8bd652ac690ca84f485014f70b52eef7cb9e +feat: add model_type in v1beta3 processor proto + +PiperOrigin-RevId: 603726122 From b77aa1d3961653177812c0e1dc0f3b7751d3d378 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Mon, 4 Mar 2024 16:46:57 +0000 Subject: [PATCH 11/32] fix integration tests --- library_generation/test/integration_tests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index 76aa6f4729..e7f77db17d 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -23,7 +23,6 @@ from git import Repo from pathlib import Path from typing import List -from typing import Dict from library_generation.generate_pr_description import generate_pr_descriptions from library_generation.generate_repo import generate_from_yaml @@ -175,7 +174,7 @@ def __pull_repo_to(cls, default_dest: Path, repo: str, committish: str) -> str: repo = Repo(dest) else: dest = default_dest - repo_dest = f"{golden_dir}/{repo}" + shutil.rmtree(dest, ignore_errors=True) repo_url = f"{repo_prefix}/{repo}" print(f"Cloning repository {repo_url}") repo = Repo.clone_from(repo_url, dest) @@ -194,6 +193,8 @@ def __get_library_names_from_config(cls, config: GenerationConfig) -> List[str]: def __get_config_files(cls, path: str) -> List[tuple[str, str]]: config_files = [] for sub_dir in Path(path).resolve().iterdir(): + if sub_dir.is_file(): + continue repo = sub_dir.name if repo == "golden": continue From 259c4ef3501b3e2b042491c04771fe1387570677 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 01:52:05 +0000 Subject: [PATCH 12/32] move get_commit_messages to surface --- library_generation/utilities.py | 43 --------------------------------- 1 file changed, 43 deletions(-) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 3446fe0552..402c0eed1e 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -17,7 +17,6 @@ import os import shutil import re -from git import Commit, Repo from pathlib import Path from lxml import etree from library_generation.model.bom_config import BomConfig @@ -142,14 +141,6 @@ def __handle_special_bom( ] -def __is_qualified_commit(paths: set[str], commit: Commit) -> bool: - for file in commit.stats.files.keys(): - idx = file.rfind("/") - if file[:idx] in paths: - return True - return False - - def create_argument(arg_key: str, arg_container: object) -> List[str]: """ Generates a list of two elements [argument, value], or returns @@ -514,37 +505,3 @@ def get_file_paths(config: GenerationConfig) -> set[str]: for gapic_config in library.gapic_configs: paths.add(gapic_config.proto_path) return paths.union(common_protos) - - -def get_commit_messages( - repo_url: str, latest_commit: str, baseline_commit: str, paths: set[str] -) -> str: - """ - Combine commit messages of a repository from latest_commit to - baseline_commit. Only commits which change files in a pre-defined - file paths will be considered. - Note that baseline_commit should be an ancestor of latest_commit. - - :param repo_url: the url of the repository. - :param latest_commit: the newest commit to be considered in - selecting commit message. - :param baseline_commit: the oldest commit to be considered in - selecting commit message. This commit should be an ancestor of - :param paths: a set of file paths - :return: commit messages. - """ - tmp_dir = "/tmp/repo" - shutil.rmtree(tmp_dir, ignore_errors=True) - os.mkdir(tmp_dir) - messages = [] - repo = Repo.clone_from(repo_url, tmp_dir) - commit = repo.commit(latest_commit) - while str(commit.hexsha) != baseline_commit: - if __is_qualified_commit(paths, commit): - messages.append(f"{commit.hexsha}\n{commit.message}") - commit_parents = commit.parents - if len(commit_parents) == 0: - break - commit = commit_parents[0] - shutil.rmtree(tmp_dir, ignore_errors=True) - return "\n\n".join(messages) From f09fb3b2f284602810c72c0d8517d438898f9e63 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 02:00:42 +0000 Subject: [PATCH 13/32] format pr description --- library_generation/generate_pr_description.py | 71 ++++++++++++++++++- .../integration/pr-description-golden.txt | 11 ++- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 2fc9c48bd3..630cb4e84d 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -12,11 +12,13 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import os +import shutil +from typing import List import click - +from git import Commit, Repo from library_generation.model.generation_config import from_yaml -from library_generation.utilities import get_commit_messages from library_generation.utilities import get_file_paths @@ -74,7 +76,7 @@ def generate_pr_descriptions( ) -> str: config = from_yaml(generation_config_yaml) paths = get_file_paths(config) - return get_commit_messages( + return __get_commit_messages( repo_url=repo_url, latest_commit=config.googleapis_commitish, baseline_commit=baseline_commit, @@ -82,5 +84,68 @@ def generate_pr_descriptions( ) +def __get_commit_messages( + repo_url: str, latest_commit: str, baseline_commit: str, paths: set[str] +) -> str: + """ + Combine commit messages of a repository from latest_commit to + baseline_commit. Only commits which change files in a pre-defined + file paths will be considered. + Note that baseline_commit should be an ancestor of latest_commit. + + :param repo_url: the url of the repository. + :param latest_commit: the newest commit to be considered in + selecting commit message. + :param baseline_commit: the oldest commit to be considered in + selecting commit message. This commit should be an ancestor of + :param paths: a set of file paths + :return: commit messages. + """ + tmp_dir = "/tmp/repo" + shutil.rmtree(tmp_dir, ignore_errors=True) + os.mkdir(tmp_dir) + repo = Repo.clone_from(repo_url, tmp_dir) + commit = repo.commit(latest_commit) + qualified_commits = [] + while str(commit.hexsha) != baseline_commit: + if __is_qualified_commit(paths, commit): + qualified_commits.append(commit) + commit_parents = commit.parents + if len(commit_parents) == 0: + break + commit = commit_parents[0] + shutil.rmtree(tmp_dir, ignore_errors=True) + return __combine_commit_messages( + latest_commit=latest_commit, + baseline_commit=baseline_commit, + commits=qualified_commits, + ) + + +def __is_qualified_commit(paths: set[str], commit: Commit) -> bool: + for file in commit.stats.files.keys(): + idx = file.rfind("/") + if file[:idx] in paths: + return True + return False + + +def __combine_commit_messages( + latest_commit: str, baseline_commit: str, commits: List[Commit] +) -> str: + messages = [ + f"This pull request is generated with proto changes between googleapis commit {baseline_commit} and {latest_commit}.", + "BEGIN_COMMIT_OVERRIDE", + ] + for commit in commits: + short_sha = commit.hexsha[:7] + messages.append( + f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})\n{commit.message}" + ) + messages.append("END_COMMIT_OVERRIDE") + + return "\n\n".join(messages) + + if __name__ == "__main__": main() diff --git a/library_generation/test/resources/integration/pr-description-golden.txt b/library_generation/test/resources/integration/pr-description-golden.txt index 6229629b98..7e3eb489bf 100644 --- a/library_generation/test/resources/integration/pr-description-golden.txt +++ b/library_generation/test/resources/integration/pr-description-golden.txt @@ -1,10 +1,17 @@ -7a9a855287b5042410c93e5a510f40efd4ce6cb1 +This pull request is generated with proto changes between googleapis commit a17d4caf184b050d50cacf2b0d579ce72c31ce74 and 1a45bf7393b52407188c82e63101db7dc9c72026. + +BEGIN_COMMIT_OVERRIDE + +[googleapis/googleapis@7a9a855](https://github.com/googleapis/googleapis/commit/7a9a855287b5042410c93e5a510f40efd4ce6cb1) feat: expose model_type in v1 processor, so that user can see the model_type after get or list processor version PiperOrigin-RevId: 603727585 -c7fd8bd652ac690ca84f485014f70b52eef7cb9e +[googleapis/googleapis@c7fd8bd](https://github.com/googleapis/googleapis/commit/c7fd8bd652ac690ca84f485014f70b52eef7cb9e) feat: add model_type in v1beta3 processor proto PiperOrigin-RevId: 603726122 + + +END_COMMIT_OVERRIDE \ No newline at end of file From 5e1d85ea60b2a4074132fc65408ef3d20d6c6a27 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 02:03:38 +0000 Subject: [PATCH 14/32] remove common protos --- library_generation/test/unit_tests.py | 5 ----- library_generation/utilities.py | 12 ++---------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 9f928bcb7a..676ed50617 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -224,11 +224,6 @@ def test_get_file_paths_from_yaml_success(self): "google/cloud/asset/v1p2beta1", "google/cloud/asset/v1p5beta1", "google/cloud/asset/v1p7beta1", - "google/api", - "google/longrunning", - "google/rpc", - "google/shopping/type", - "google/type", }, paths, ) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 402c0eed1e..eb40ef93bd 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -33,13 +33,6 @@ group_id_tag = "groupId" artifact_tag = "artifactId" version_tag = "version" -common_protos = { - "google/api", - "google/longrunning", - "google/rpc", - "google/shopping/type", - "google/type", -} def __render(template_name: str, output_name: str, **kwargs): @@ -494,8 +487,7 @@ def get_version_from( def get_file_paths(config: GenerationConfig) -> set[str]: """ - Get versioned proto_path from configuration file, plus known paths of - common protos. + Get versioned proto_path from configuration file. :param config: :return: versioned proto_path plus paths of common protos @@ -504,4 +496,4 @@ def get_file_paths(config: GenerationConfig) -> set[str]: for library in config.libraries: for gapic_config in library.gapic_configs: paths.add(gapic_config.proto_path) - return paths.union(common_protos) + return paths From 7e9be85179af4e0a8a20ecdb6c369fdff39353ca Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 02:32:11 +0000 Subject: [PATCH 15/32] add integration test for split repo --- library_generation/test/integration_tests.py | 36 +++++++++---------- .../pr-description-golden.txt | 0 .../java-bigtable/pr-description-golden.txt | 17 +++++++++ 3 files changed, 35 insertions(+), 18 deletions(-) rename library_generation/test/resources/integration/{ => google-cloud-java}/pr-description-golden.txt (100%) create mode 100644 library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index e7f77db17d..ae2a9e99fe 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -53,25 +53,25 @@ class IntegrationTest(unittest.TestCase): def test_get_commit_message_success(self): repo_url = "https://github.com/googleapis/googleapis.git" - baseline_commit = "a17d4caf184b050d50cacf2b0d579ce72c31ce74" - description = generate_pr_descriptions( - f"{config_dir}/google-cloud-java/{config_name}", - repo_url=repo_url, - baseline_commit=baseline_commit, - ) - description_file = f"{config_dir}/pr-description.txt" - if os.path.isfile(f"{description_file}"): + config_files = self.__get_config_files(config_dir) + monorepo_baseline_commit = "a17d4caf184b050d50cacf2b0d579ce72c31ce74" + split_repo_baseline_commit = "679060c64136e85b52838f53cfe612ce51e60d1d" + for repo, config_file in config_files: + baseline_commit = monorepo_baseline_commit if repo == "google-cloud-java" else split_repo_baseline_commit + description = generate_pr_descriptions( + generation_config_yaml=config_file, + repo_url=repo_url, + baseline_commit=baseline_commit, + ) + description_file = f"{config_dir}/{repo}/pr-description.txt" + if os.path.isfile(f"{description_file}"): + os.remove(f"{description_file}") + with open(f"{description_file}", "w+") as f: + f.write(description) + self.assertTrue( + cmp(f"{config_dir}/{repo}/pr-description-golden.txt", f"{description_file}") + ) os.remove(f"{description_file}") - with open(f"{description_file}", "w+") as f: - f.write(description) - # There are five commits from googleapis commitish to baseline commit, - # inclusively. Only two of the commits contain changes in proto_path - # that are in configuration. Therefore, only two commit messages should - # be included in the PR description. - self.assertTrue( - cmp(f"{config_dir}/pr-description-golden.txt", f"{description_file}") - ) - os.remove(f"{description_file}") def test_generate_repo(self): shutil.rmtree(f"{golden_dir}", ignore_errors=True) diff --git a/library_generation/test/resources/integration/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt similarity index 100% rename from library_generation/test/resources/integration/pr-description-golden.txt rename to library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt new file mode 100644 index 0000000000..519ce1d221 --- /dev/null +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -0,0 +1,17 @@ +This pull request is generated with proto changes between googleapis commit 679060c64136e85b52838f53cfe612ce51e60d1d and fc3043ebe12fb6bc1729c175e1526c859ce751d8. + +BEGIN_COMMIT_OVERRIDE + +[googleapis/googleapis@fbcfef0](https://github.com/googleapis/googleapis/commit/fbcfef09510b842774530989889ed1584a8b5acb) +fix: extend timeouts for deleting snapshots, backups and tables + +PiperOrigin-RevId: 605388988 + + +[googleapis/googleapis@63d2a60](https://github.com/googleapis/googleapis/commit/63d2a60056ad5b156c05c7fb13138fc886c3b739) +chore: update retry settings for backup rpcs + +PiperOrigin-RevId: 605367937 + + +END_COMMIT_OVERRIDE \ No newline at end of file From 9f3bbcaaf815d41392b0dd1b69ef4a76eeaf557d Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 02:33:09 +0000 Subject: [PATCH 16/32] format code --- library_generation/test/integration_tests.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library_generation/test/integration_tests.py b/library_generation/test/integration_tests.py index ae2a9e99fe..3ca7d45881 100755 --- a/library_generation/test/integration_tests.py +++ b/library_generation/test/integration_tests.py @@ -57,7 +57,11 @@ def test_get_commit_message_success(self): monorepo_baseline_commit = "a17d4caf184b050d50cacf2b0d579ce72c31ce74" split_repo_baseline_commit = "679060c64136e85b52838f53cfe612ce51e60d1d" for repo, config_file in config_files: - baseline_commit = monorepo_baseline_commit if repo == "google-cloud-java" else split_repo_baseline_commit + baseline_commit = ( + monorepo_baseline_commit + if repo == "google-cloud-java" + else split_repo_baseline_commit + ) description = generate_pr_descriptions( generation_config_yaml=config_file, repo_url=repo_url, @@ -69,7 +73,10 @@ def test_get_commit_message_success(self): with open(f"{description_file}", "w+") as f: f.write(description) self.assertTrue( - cmp(f"{config_dir}/{repo}/pr-description-golden.txt", f"{description_file}") + cmp( + f"{config_dir}/{repo}/pr-description-golden.txt", + f"{description_file}", + ) ) os.remove(f"{description_file}") From 832477664ca490853e8f5ec30789d790c5e3c43d Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 15:20:12 +0000 Subject: [PATCH 17/32] format pr description --- library_generation/generate_pr_description.py | 9 +++++++-- .../google-cloud-java/pr-description-golden.txt | 12 +++++------- .../java-bigtable/pr-description-golden.txt | 12 +++++------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 630cb4e84d..0d8fb0e766 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -135,13 +135,18 @@ def __combine_commit_messages( ) -> str: messages = [ f"This pull request is generated with proto changes between googleapis commit {baseline_commit} and {latest_commit}.", - "BEGIN_COMMIT_OVERRIDE", + "Qualified commits are:", ] for commit in commits: short_sha = commit.hexsha[:7] messages.append( - f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})\n{commit.message}" + f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})" ) + + messages.append("BEGIN_COMMIT_OVERRIDE") + for commit in commits: + first_line = commit.message.partition("\n")[0] + messages.append(f"{first_line}") messages.append("END_COMMIT_OVERRIDE") return "\n\n".join(messages) diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 7e3eb489bf..2a6a218bd2 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -1,17 +1,15 @@ This pull request is generated with proto changes between googleapis commit a17d4caf184b050d50cacf2b0d579ce72c31ce74 and 1a45bf7393b52407188c82e63101db7dc9c72026. -BEGIN_COMMIT_OVERRIDE +Qualified commits are: [googleapis/googleapis@7a9a855](https://github.com/googleapis/googleapis/commit/7a9a855287b5042410c93e5a510f40efd4ce6cb1) -feat: expose model_type in v1 processor, so that user can see the model_type after get or list processor version - -PiperOrigin-RevId: 603727585 - [googleapis/googleapis@c7fd8bd](https://github.com/googleapis/googleapis/commit/c7fd8bd652ac690ca84f485014f70b52eef7cb9e) -feat: add model_type in v1beta3 processor proto -PiperOrigin-RevId: 603726122 +BEGIN_COMMIT_OVERRIDE + +feat: expose model_type in v1 processor, so that user can see the model_type after get or list processor version +feat: add model_type in v1beta3 processor proto END_COMMIT_OVERRIDE \ No newline at end of file diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index 519ce1d221..5d28b4f1fc 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -1,17 +1,15 @@ This pull request is generated with proto changes between googleapis commit 679060c64136e85b52838f53cfe612ce51e60d1d and fc3043ebe12fb6bc1729c175e1526c859ce751d8. -BEGIN_COMMIT_OVERRIDE +Qualified commits are: [googleapis/googleapis@fbcfef0](https://github.com/googleapis/googleapis/commit/fbcfef09510b842774530989889ed1584a8b5acb) -fix: extend timeouts for deleting snapshots, backups and tables - -PiperOrigin-RevId: 605388988 - [googleapis/googleapis@63d2a60](https://github.com/googleapis/googleapis/commit/63d2a60056ad5b156c05c7fb13138fc886c3b739) -chore: update retry settings for backup rpcs -PiperOrigin-RevId: 605367937 +BEGIN_COMMIT_OVERRIDE + +fix: extend timeouts for deleting snapshots, backups and tables +chore: update retry settings for backup rpcs END_COMMIT_OVERRIDE \ No newline at end of file From cc9088e1b25deb31addc0f780584a09ca8c78323 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 15:29:22 +0000 Subject: [PATCH 18/32] change parameter comments --- library_generation/generate_pr_description.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 0d8fb0e766..71484b18d0 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -37,6 +37,8 @@ def main(ctx): help=""" Path to generation_config.yaml that contains the metadata about library generation. + The googleapis commit in the configuration is the latest commit, + inclusively, from which the commit message is considered. """, ) @click.option( @@ -44,7 +46,7 @@ def main(ctx): required=True, type=str, help=""" - The baseline commit from which the commit message is considered. + The baseline, exclusively, commit from which the commit message is considered. This commit should be an ancestor of googleapis commit in configuration. """, ) From 334298b1797abd190fce4062d31003cccd375a01 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 15:31:14 +0000 Subject: [PATCH 19/32] change parameter comments --- library_generation/generate_pr_description.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 71484b18d0..05e2304c35 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -46,7 +46,8 @@ def main(ctx): required=True, type=str, help=""" - The baseline, exclusively, commit from which the commit message is considered. + The baseline (oldest) commit, exclusively, from which the commit message is + considered. This commit should be an ancestor of googleapis commit in configuration. """, ) From aabd9c17309efe9399016da110d503bb5003546a Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 18:09:16 +0000 Subject: [PATCH 20/32] add generator version in pr description --- library_generation/generate_pr_description.py | 17 +++++++++++++++-- .../google-cloud-java/pr-description-golden.txt | 2 ++ .../java-bigtable/pr-description-golden.txt | 2 ++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 05e2304c35..cd0bac69f1 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -84,11 +84,16 @@ def generate_pr_descriptions( latest_commit=config.googleapis_commitish, baseline_commit=baseline_commit, paths=paths, + generator_version=config.gapic_generator_version, ) def __get_commit_messages( - repo_url: str, latest_commit: str, baseline_commit: str, paths: set[str] + repo_url: str, + latest_commit: str, + baseline_commit: str, + paths: set[str], + generator_version: str, ) -> str: """ Combine commit messages of a repository from latest_commit to @@ -102,6 +107,7 @@ def __get_commit_messages( :param baseline_commit: the oldest commit to be considered in selecting commit message. This commit should be an ancestor of :param paths: a set of file paths + :param generator_version: the version of the generator. :return: commit messages. """ tmp_dir = "/tmp/repo" @@ -122,6 +128,7 @@ def __get_commit_messages( latest_commit=latest_commit, baseline_commit=baseline_commit, commits=qualified_commits, + generator_version=generator_version, ) @@ -134,7 +141,10 @@ def __is_qualified_commit(paths: set[str], commit: Commit) -> bool: def __combine_commit_messages( - latest_commit: str, baseline_commit: str, commits: List[Commit] + latest_commit: str, + baseline_commit: str, + commits: List[Commit], + generator_version: str, ) -> str: messages = [ f"This pull request is generated with proto changes between googleapis commit {baseline_commit} and {latest_commit}.", @@ -150,6 +160,9 @@ def __combine_commit_messages( for commit in commits: first_line = commit.message.partition("\n")[0] messages.append(f"{first_line}") + messages.append( + f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" + ) messages.append("END_COMMIT_OVERRIDE") return "\n\n".join(messages) diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 2a6a218bd2..c5152d00e8 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -12,4 +12,6 @@ feat: expose model_type in v1 processor, so that user can see the model_type aft feat: add model_type in v1beta3 processor proto +feat: Regenerate with the Java code generator (gapic-generator-java) v2.34.0 + END_COMMIT_OVERRIDE \ No newline at end of file diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index 5d28b4f1fc..32ccd74db4 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -12,4 +12,6 @@ fix: extend timeouts for deleting snapshots, backups and tables chore: update retry settings for backup rpcs +feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 + END_COMMIT_OVERRIDE \ No newline at end of file From 176871d650a340e0b464c112a15cef23aebc6cb2 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 20:17:19 +0000 Subject: [PATCH 21/32] find the versioned proto_path from a file path --- library_generation/test/unit_tests.py | 28 +++++++++++++++++++---- library_generation/utilities.py | 33 ++++++++++++++++++++++----- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 676ed50617..67429d848b 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -30,6 +30,7 @@ from library_generation.model.gapic_inputs import parse as parse_build_file from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig +from library_generation.utilities import find_versioned_proto_path from library_generation.utilities import get_file_paths script_dir = os.path.dirname(os.path.realpath(__file__)) @@ -219,15 +220,32 @@ def test_get_file_paths_from_yaml_success(self): paths = get_file_paths(from_yaml(f"{test_config_dir}/generation_config.yaml")) self.assertEqual( { - "google/cloud/asset/v1", - "google/cloud/asset/v1p1beta1", - "google/cloud/asset/v1p2beta1", - "google/cloud/asset/v1p5beta1", - "google/cloud/asset/v1p7beta1", + "google/cloud/asset/v1": "asset", + "google/cloud/asset/v1p1beta1": "asset", + "google/cloud/asset/v1p2beta1": "asset", + "google/cloud/asset/v1p5beta1": "asset", + "google/cloud/asset/v1p7beta1": "asset", }, paths, ) + @parameterized.expand( + [ + ( + "google/cloud/aiplatform/v1/schema/predict/params/image_classification.proto", + "google/cloud/aiplatform/v1", + ), + ( + "google/cloud/asset/v1p2beta1/assets.proto", + "google/cloud/asset/v1p2beta1", + ), + ("google/type/color.proto", "google/type/color.proto"), + ] + ) + def test_find_versioned_proto_path(self, file_path, expected): + proto_path = find_versioned_proto_path(file_path) + self.assertEqual(expected, proto_path) + @parameterized.expand( [ ("BUILD_no_additional_protos.bazel", " "), diff --git a/library_generation/utilities.py b/library_generation/utilities.py index eb40ef93bd..72f2cbe577 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -18,6 +18,8 @@ import shutil import re from pathlib import Path +from typing import Dict + from lxml import etree from library_generation.model.bom_config import BomConfig from library_generation.model.generation_config import GenerationConfig @@ -485,15 +487,34 @@ def get_version_from( return line.split(":")[index].strip() -def get_file_paths(config: GenerationConfig) -> set[str]: +def get_file_paths(config: GenerationConfig) -> Dict[str, str]: """ - Get versioned proto_path from configuration file. + Get versioned proto_path to library_name mapping from configuration file. - :param config: - :return: versioned proto_path plus paths of common protos + :param config: a GenerationConfig object. + :return: versioned proto_path to library_name mapping """ - paths = set() + paths = {} for library in config.libraries: for gapic_config in library.gapic_configs: - paths.add(gapic_config.proto_path) + paths[gapic_config.proto_path] = get_library_name(library) return paths + + +def find_versioned_proto_path(file_path: str) -> str: + """ + Returns a versioned proto_path from a given file_path; or file_path itself + if it doesn't contain a versioned proto_path. + + :param file_path: + :return: + """ + version_regex = re.compile(r"^v[1-9].*") + directories = file_path.split("/") + for directory in directories: + result = version_regex.search(directory) + if result: + version = result[0] + idx = file_path.find(version) + return file_path[:idx] + version + return file_path From 93e60de8edd33a03267a06b368c88067ebc1ad67 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 20:17:48 +0000 Subject: [PATCH 22/32] add library name in pr description --- library_generation/generate_pr_description.py | 41 +++++++++++++------ .../pr-description-golden.txt | 4 +- .../java-bigtable/pr-description-golden.txt | 4 +- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index cd0bac69f1..93c72f1547 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -14,11 +14,14 @@ # limitations under the License. import os import shutil +from typing import Dict + from typing import List import click from git import Commit, Repo from library_generation.model.generation_config import from_yaml +from library_generation.utilities import find_versioned_proto_path from library_generation.utilities import get_file_paths @@ -92,7 +95,7 @@ def __get_commit_messages( repo_url: str, latest_commit: str, baseline_commit: str, - paths: set[str], + paths: Dict[str, str], generator_version: str, ) -> str: """ @@ -106,7 +109,7 @@ def __get_commit_messages( selecting commit message. :param baseline_commit: the oldest commit to be considered in selecting commit message. This commit should be an ancestor of - :param paths: a set of file paths + :param paths: a mapping from file paths to library_name. :param generator_version: the version of the generator. :return: commit messages. """ @@ -115,10 +118,11 @@ def __get_commit_messages( os.mkdir(tmp_dir) repo = Repo.clone_from(repo_url, tmp_dir) commit = repo.commit(latest_commit) - qualified_commits = [] + qualified_commits = {} while str(commit.hexsha) != baseline_commit: - if __is_qualified_commit(paths, commit): - qualified_commits.append(commit) + commit_and_name = __filter_qualified_commit(paths=paths, commit=commit) + if commit_and_name != (): + qualified_commits[commit_and_name[0]] = commit_and_name[1] commit_parents = commit.parents if len(commit_parents) == 0: break @@ -132,18 +136,28 @@ def __get_commit_messages( ) -def __is_qualified_commit(paths: set[str], commit: Commit) -> bool: +def __filter_qualified_commit(paths: Dict[str, str], commit: Commit) -> (Commit, str): + """ + Returns a tuple of a commit and libray_name. + A qualified commit means at least one file changes in that commit is + within the versioned proto_path in paths. + + :param paths: a mapping from versioned proto_path to library_name. + :param commit: a commit under consideration. + :return: a tuple of a commit and library_name if the commit is + qualified; otherwise an empty tuple. + """ for file in commit.stats.files.keys(): - idx = file.rfind("/") - if file[:idx] in paths: - return True - return False + versioned_proto_path = find_versioned_proto_path(file) + if versioned_proto_path in paths: + return commit, paths[versioned_proto_path] + return () def __combine_commit_messages( latest_commit: str, baseline_commit: str, - commits: List[Commit], + commits: Dict[Commit, str], generator_version: str, ) -> str: messages = [ @@ -157,9 +171,10 @@ def __combine_commit_messages( ) messages.append("BEGIN_COMMIT_OVERRIDE") - for commit in commits: + for commit, library_name in commits.items(): first_line = commit.message.partition("\n")[0] - messages.append(f"{first_line}") + convention, _, summary = first_line.partition(":") + messages.append(f"{convention}: [{library_name}] {summary.strip()}") messages.append( f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" ) diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index c5152d00e8..20dff6d34f 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -8,9 +8,9 @@ Qualified commits are: BEGIN_COMMIT_OVERRIDE -feat: expose model_type in v1 processor, so that user can see the model_type after get or list processor version +feat: [document-ai] expose model_type in v1 processor, so that user can see the model_type after get or list processor version -feat: add model_type in v1beta3 processor proto +feat: [document-ai] add model_type in v1beta3 processor proto feat: Regenerate with the Java code generator (gapic-generator-java) v2.34.0 diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index 32ccd74db4..d5d1e764e9 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -8,9 +8,9 @@ Qualified commits are: BEGIN_COMMIT_OVERRIDE -fix: extend timeouts for deleting snapshots, backups and tables +fix: [bigtable] extend timeouts for deleting snapshots, backups and tables -chore: update retry settings for backup rpcs +chore: [bigtable] update retry settings for backup rpcs feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 From d4d16f6235c3c10050f1dc6a1ea626c06b13c0b7 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 20:46:58 +0000 Subject: [PATCH 23/32] format pr description --- library_generation/generate_pr_description.py | 2 +- .../integration/google-cloud-java/pr-description-golden.txt | 2 +- .../integration/java-bigtable/pr-description-golden.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 93c72f1547..2b0628c666 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -161,7 +161,7 @@ def __combine_commit_messages( generator_version: str, ) -> str: messages = [ - f"This pull request is generated with proto changes between googleapis commit {baseline_commit} and {latest_commit}.", + f"This pull request is generated with proto changes between googleapis commit {baseline_commit} (exclusive) and {latest_commit} (inclusive).", "Qualified commits are:", ] for commit in commits: diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 20dff6d34f..2c1a50da44 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -1,4 +1,4 @@ -This pull request is generated with proto changes between googleapis commit a17d4caf184b050d50cacf2b0d579ce72c31ce74 and 1a45bf7393b52407188c82e63101db7dc9c72026. +This pull request is generated with proto changes between googleapis commit a17d4caf184b050d50cacf2b0d579ce72c31ce74 (exclusive) and 1a45bf7393b52407188c82e63101db7dc9c72026 (inclusive). Qualified commits are: diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index d5d1e764e9..726b8ec9b0 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -1,4 +1,4 @@ -This pull request is generated with proto changes between googleapis commit 679060c64136e85b52838f53cfe612ce51e60d1d and fc3043ebe12fb6bc1729c175e1526c859ce751d8. +This pull request is generated with proto changes between googleapis commit 679060c64136e85b52838f53cfe612ce51e60d1d (exclusive) and fc3043ebe12fb6bc1729c175e1526c859ce751d8 (inclusive). Qualified commits are: From a279549fe4d3edb85eae88198df9816a91ca03fb Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Tue, 5 Mar 2024 20:54:38 +0000 Subject: [PATCH 24/32] do not include library_name in split repo --- library_generation/generate_pr_description.py | 12 +++++++++++- .../java-bigtable/pr-description-golden.txt | 4 ++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 2b0628c666..f092e10bc7 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -88,6 +88,7 @@ def generate_pr_descriptions( baseline_commit=baseline_commit, paths=paths, generator_version=config.gapic_generator_version, + is_monorepo=config.is_monorepo, ) @@ -97,6 +98,7 @@ def __get_commit_messages( baseline_commit: str, paths: Dict[str, str], generator_version: str, + is_monorepo: bool, ) -> str: """ Combine commit messages of a repository from latest_commit to @@ -111,6 +113,7 @@ def __get_commit_messages( selecting commit message. This commit should be an ancestor of :param paths: a mapping from file paths to library_name. :param generator_version: the version of the generator. + :param is_monorepo: whether to generate commit messages in a monorepo. :return: commit messages. """ tmp_dir = "/tmp/repo" @@ -133,6 +136,7 @@ def __get_commit_messages( baseline_commit=baseline_commit, commits=qualified_commits, generator_version=generator_version, + is_monorepo=is_monorepo, ) @@ -159,6 +163,7 @@ def __combine_commit_messages( baseline_commit: str, commits: Dict[Commit, str], generator_version: str, + is_monorepo: bool, ) -> str: messages = [ f"This pull request is generated with proto changes between googleapis commit {baseline_commit} (exclusive) and {latest_commit} (inclusive).", @@ -174,7 +179,12 @@ def __combine_commit_messages( for commit, library_name in commits.items(): first_line = commit.message.partition("\n")[0] convention, _, summary = first_line.partition(":") - messages.append(f"{convention}: [{library_name}] {summary.strip()}") + formatted_message = ( + f"{convention}: [{library_name}] {summary.strip()}" + if is_monorepo + else f"{convention}: {summary.strip()}" + ) + messages.append(formatted_message) messages.append( f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" ) diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index 726b8ec9b0..0f23cc5acb 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -8,9 +8,9 @@ Qualified commits are: BEGIN_COMMIT_OVERRIDE -fix: [bigtable] extend timeouts for deleting snapshots, backups and tables +fix: extend timeouts for deleting snapshots, backups and tables -chore: [bigtable] update retry settings for backup rpcs +chore: update retry settings for backup rpcs feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 From 1946d64ea79d541c6c796c3c441402027ab41ac7 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 6 Mar 2024 00:16:06 +0000 Subject: [PATCH 25/32] bring back PiperOrigin-RevId --- library_generation/generate_pr_description.py | 7 +++---- .../google-cloud-java/pr-description-golden.txt | 6 ++++++ .../integration/java-bigtable/pr-description-golden.txt | 6 ++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index f092e10bc7..f9dd5d2970 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -177,12 +177,11 @@ def __combine_commit_messages( messages.append("BEGIN_COMMIT_OVERRIDE") for commit, library_name in commits.items(): - first_line = commit.message.partition("\n")[0] - convention, _, summary = first_line.partition(":") + convention, _, summary = commit.message.partition(":") formatted_message = ( - f"{convention}: [{library_name}] {summary.strip()}" + f"{convention}: [{library_name}]{summary}" if is_monorepo - else f"{convention}: {summary.strip()}" + else f"{convention}:{summary}" ) messages.append(formatted_message) messages.append( diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 2c1a50da44..0db16af43e 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -10,8 +10,14 @@ BEGIN_COMMIT_OVERRIDE feat: [document-ai] expose model_type in v1 processor, so that user can see the model_type after get or list processor version +PiperOrigin-RevId: 603727585 + + feat: [document-ai] add model_type in v1beta3 processor proto +PiperOrigin-RevId: 603726122 + + feat: Regenerate with the Java code generator (gapic-generator-java) v2.34.0 END_COMMIT_OVERRIDE \ No newline at end of file diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index 0f23cc5acb..4674dad01a 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -10,8 +10,14 @@ BEGIN_COMMIT_OVERRIDE fix: extend timeouts for deleting snapshots, backups and tables +PiperOrigin-RevId: 605388988 + + chore: update retry settings for backup rpcs +PiperOrigin-RevId: 605367937 + + feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 END_COMMIT_OVERRIDE \ No newline at end of file From be189c55b1fa6be4c1a8e7382f66391b61b8622d Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 6 Mar 2024 00:18:19 +0000 Subject: [PATCH 26/32] use NESTED_COMMIT --- library_generation/generate_pr_description.py | 4 ++-- .../integration/google-cloud-java/pr-description-golden.txt | 4 ++-- .../integration/java-bigtable/pr-description-golden.txt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index f9dd5d2970..2024528e21 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -175,7 +175,7 @@ def __combine_commit_messages( f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})" ) - messages.append("BEGIN_COMMIT_OVERRIDE") + messages.append("BEGIN_NESTED_COMMIT") for commit, library_name in commits.items(): convention, _, summary = commit.message.partition(":") formatted_message = ( @@ -187,7 +187,7 @@ def __combine_commit_messages( messages.append( f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" ) - messages.append("END_COMMIT_OVERRIDE") + messages.append("END_NESTED_COMMIT") return "\n\n".join(messages) diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 0db16af43e..8be3061212 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -6,7 +6,7 @@ Qualified commits are: [googleapis/googleapis@c7fd8bd](https://github.com/googleapis/googleapis/commit/c7fd8bd652ac690ca84f485014f70b52eef7cb9e) -BEGIN_COMMIT_OVERRIDE +BEGIN_NESTED_COMMIT feat: [document-ai] expose model_type in v1 processor, so that user can see the model_type after get or list processor version @@ -20,4 +20,4 @@ PiperOrigin-RevId: 603726122 feat: Regenerate with the Java code generator (gapic-generator-java) v2.34.0 -END_COMMIT_OVERRIDE \ No newline at end of file +END_NESTED_COMMIT \ No newline at end of file diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index 4674dad01a..a890cb1c4d 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -6,7 +6,7 @@ Qualified commits are: [googleapis/googleapis@63d2a60](https://github.com/googleapis/googleapis/commit/63d2a60056ad5b156c05c7fb13138fc886c3b739) -BEGIN_COMMIT_OVERRIDE +BEGIN_NESTED_COMMIT fix: extend timeouts for deleting snapshots, backups and tables @@ -20,4 +20,4 @@ PiperOrigin-RevId: 605367937 feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 -END_COMMIT_OVERRIDE \ No newline at end of file +END_NESTED_COMMIT \ No newline at end of file From 5d7c91537b98a18561c9183daa48aa44396fe2ba Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 6 Mar 2024 00:25:31 +0000 Subject: [PATCH 27/32] add comments --- library_generation/utilities.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 72f2cbe577..2faee66f7a 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -506,8 +506,8 @@ def find_versioned_proto_path(file_path: str) -> str: Returns a versioned proto_path from a given file_path; or file_path itself if it doesn't contain a versioned proto_path. - :param file_path: - :return: + :param file_path: a proto file path + :return: the versioned proto_path """ version_regex = re.compile(r"^v[1-9].*") directories = file_path.split("/") From 240038fca1cb215ca62b00a76d5518a41ad76a37 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 6 Mar 2024 01:46:45 +0000 Subject: [PATCH 28/32] format pr description --- library_generation/generate_pr_description.py | 21 +++++++++++-------- .../pr-description-golden.txt | 14 ++++--------- .../java-bigtable/pr-description-golden.txt | 14 ++++--------- 3 files changed, 20 insertions(+), 29 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 2024528e21..0f999f369a 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -16,8 +16,6 @@ import shutil from typing import Dict -from typing import List - import click from git import Commit, Repo from library_generation.model.generation_config import from_yaml @@ -175,21 +173,26 @@ def __combine_commit_messages( f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})" ) - messages.append("BEGIN_NESTED_COMMIT") for commit, library_name in commits.items(): convention, _, summary = commit.message.partition(":") formatted_message = ( - f"{convention}: [{library_name}]{summary}" + f"{convention}: [{library_name}]{str(summary).rstrip()}" if is_monorepo - else f"{convention}:{summary}" + else f"{convention}:{str(summary).rstrip()}" ) - messages.append(formatted_message) + messages.append(__wrap_nested_commit(formatted_message)) messages.append( - f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" + __wrap_nested_commit( + f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" + ) ) - messages.append("END_NESTED_COMMIT") - return "\n\n".join(messages) + return "\n".join(messages) + + +def __wrap_nested_commit(message: str) -> str: + result = ["BEGIN_NESTED_COMMIT", message, "END_NESTED_COMMIT"] + return "\n".join(result) if __name__ == "__main__": diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 8be3061212..80541f4052 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -1,23 +1,17 @@ This pull request is generated with proto changes between googleapis commit a17d4caf184b050d50cacf2b0d579ce72c31ce74 (exclusive) and 1a45bf7393b52407188c82e63101db7dc9c72026 (inclusive). - Qualified commits are: - [googleapis/googleapis@7a9a855](https://github.com/googleapis/googleapis/commit/7a9a855287b5042410c93e5a510f40efd4ce6cb1) - [googleapis/googleapis@c7fd8bd](https://github.com/googleapis/googleapis/commit/c7fd8bd652ac690ca84f485014f70b52eef7cb9e) - BEGIN_NESTED_COMMIT - feat: [document-ai] expose model_type in v1 processor, so that user can see the model_type after get or list processor version PiperOrigin-RevId: 603727585 - - +END_NESTED_COMMIT +BEGIN_NESTED_COMMIT feat: [document-ai] add model_type in v1beta3 processor proto PiperOrigin-RevId: 603726122 - - +END_NESTED_COMMIT +BEGIN_NESTED_COMMIT feat: Regenerate with the Java code generator (gapic-generator-java) v2.34.0 - END_NESTED_COMMIT \ No newline at end of file diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index a890cb1c4d..820b6563f4 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -1,23 +1,17 @@ This pull request is generated with proto changes between googleapis commit 679060c64136e85b52838f53cfe612ce51e60d1d (exclusive) and fc3043ebe12fb6bc1729c175e1526c859ce751d8 (inclusive). - Qualified commits are: - [googleapis/googleapis@fbcfef0](https://github.com/googleapis/googleapis/commit/fbcfef09510b842774530989889ed1584a8b5acb) - [googleapis/googleapis@63d2a60](https://github.com/googleapis/googleapis/commit/63d2a60056ad5b156c05c7fb13138fc886c3b739) - BEGIN_NESTED_COMMIT - fix: extend timeouts for deleting snapshots, backups and tables PiperOrigin-RevId: 605388988 - - +END_NESTED_COMMIT +BEGIN_NESTED_COMMIT chore: update retry settings for backup rpcs PiperOrigin-RevId: 605367937 - - +END_NESTED_COMMIT +BEGIN_NESTED_COMMIT feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 - END_NESTED_COMMIT \ No newline at end of file From cd7f639ebfd8a35282db37ff46f2c7f294e3ac06 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 6 Mar 2024 23:46:30 +0000 Subject: [PATCH 29/32] format multi-line commit messages --- library_generation/generate_pr_description.py | 24 ++++------ .../pr-description-golden.txt | 2 + .../java-bigtable/pr-description-golden.txt | 2 + library_generation/utilities.py | 48 +++++++++++++++++++ 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index 0f999f369a..e272e8a4d3 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -20,7 +20,9 @@ from git import Commit, Repo from library_generation.model.generation_config import from_yaml from library_generation.utilities import find_versioned_proto_path +from library_generation.utilities import format_commit_message from library_generation.utilities import get_file_paths +from library_generation.utilities import wrap_nested_commit @click.group(invoke_without_command=False) @@ -173,27 +175,17 @@ def __combine_commit_messages( f"[googleapis/googleapis@{short_sha}](https://github.com/googleapis/googleapis/commit/{commit.hexsha})" ) - for commit, library_name in commits.items(): - convention, _, summary = commit.message.partition(":") - formatted_message = ( - f"{convention}: [{library_name}]{str(summary).rstrip()}" - if is_monorepo - else f"{convention}:{str(summary).rstrip()}" - ) - messages.append(__wrap_nested_commit(formatted_message)) - messages.append( - __wrap_nested_commit( - f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" + messages.extend(format_commit_message(commits=commits, is_monorepo=is_monorepo)) + messages.extend( + wrap_nested_commit( + [ + f"feat: Regenerate with the Java code generator (gapic-generator-java) v{generator_version}" + ] ) ) return "\n".join(messages) -def __wrap_nested_commit(message: str) -> str: - result = ["BEGIN_NESTED_COMMIT", message, "END_NESTED_COMMIT"] - return "\n".join(result) - - if __name__ == "__main__": main() diff --git a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt index 80541f4052..c095ab12a1 100644 --- a/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt +++ b/library_generation/test/resources/integration/google-cloud-java/pr-description-golden.txt @@ -6,11 +6,13 @@ BEGIN_NESTED_COMMIT feat: [document-ai] expose model_type in v1 processor, so that user can see the model_type after get or list processor version PiperOrigin-RevId: 603727585 + END_NESTED_COMMIT BEGIN_NESTED_COMMIT feat: [document-ai] add model_type in v1beta3 processor proto PiperOrigin-RevId: 603726122 + END_NESTED_COMMIT BEGIN_NESTED_COMMIT feat: Regenerate with the Java code generator (gapic-generator-java) v2.34.0 diff --git a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt index 820b6563f4..08cd7d15d1 100644 --- a/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt +++ b/library_generation/test/resources/integration/java-bigtable/pr-description-golden.txt @@ -6,11 +6,13 @@ BEGIN_NESTED_COMMIT fix: extend timeouts for deleting snapshots, backups and tables PiperOrigin-RevId: 605388988 + END_NESTED_COMMIT BEGIN_NESTED_COMMIT chore: update retry settings for backup rpcs PiperOrigin-RevId: 605367937 + END_NESTED_COMMIT BEGIN_NESTED_COMMIT feat: Regenerate with the Java code generator (gapic-generator-java) v2.35.0 diff --git a/library_generation/utilities.py b/library_generation/utilities.py index 2faee66f7a..a4ac74c4b4 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -21,6 +21,7 @@ from typing import Dict from lxml import etree +from git import Commit from library_generation.model.bom_config import BomConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig @@ -518,3 +519,50 @@ def find_versioned_proto_path(file_path: str) -> str: idx = file_path.find(version) return file_path[:idx] + version return file_path + + +def format_commit_message(commits: Dict[Commit, str], is_monorepo: bool) -> List[str]: + """ + Format commit messages. Add library_name to conventional commit messages if + is_monorepo is True; otherwise no op. + + :param commits: a mapping from commit to library_name. + :param is_monorepo: whether it's monorepo or not. + :return: formatted commit messages. + """ + all_commits = [] + # please see go/java-client-releasing#conventional-commit-messages + # for conventional commit. + type_regex = re.compile(r"(feat|fix|docs|deps|test|samples|chore)!?:.*") + for commit, library_name in commits.items(): + # a commit message may contain multiple lines, we need to + # add library_name for each line. + messages = [] + for message_line in commit.message.split("\n"): + # add library name to a conventional commit message; + # otherwise no op. + if type_regex.search(message_line): + commit_type, _, summary = message_line.partition(":") + formatted_message = ( + f"{commit_type}: [{library_name}]{str(summary).rstrip()}" + if is_monorepo + else f"{commit_type}:{str(summary).rstrip()}" + ) + messages.append(formatted_message) + else: + messages.append(message_line) + all_commits.extend(wrap_nested_commit(messages)) + return all_commits + + +def wrap_nested_commit(messages: List[str]) -> List[str]: + """ + Wrap message between `BEGIN_NESTED_COMMIT` and `BEGIN_NESTED_COMMIT`. + + :param messages: a (multi-line) commit message, one line per item. + :return: wrapped messages. + """ + result = ["BEGIN_NESTED_COMMIT"] + result.extend(messages) + result.append("END_NESTED_COMMIT") + return result From 171571410ec393f89762ea2d5b0969b4ebf67d81 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Wed, 6 Mar 2024 23:54:56 +0000 Subject: [PATCH 30/32] add unit test --- library_generation/test/unit_tests.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 67429d848b..0e00ee26dc 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -32,6 +32,7 @@ from library_generation.model.library_config import LibraryConfig from library_generation.utilities import find_versioned_proto_path from library_generation.utilities import get_file_paths +from library_generation.utilities import wrap_nested_commit script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -507,6 +508,20 @@ def test_monorepo_postprocessing_valid_repository_success(self): actual=f"{repository_path}/gapic-libraries-bom/pom.xml", ) + def test_format_commit_message + + def test_wrap_nested_commit_success(self): + messages = ["a commit message", "another message"] + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "a commit message", + "another message", + "END_NESTED_COMMIT", + ], + wrap_nested_commit(messages), + ) + def __compare_files(self, expect: str, actual: str): with open(expect, "r") as f: expected_lines = f.readlines() From 6cf00453a919abafb034bdec608d60fa39893481 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 7 Mar 2024 00:28:49 +0000 Subject: [PATCH 31/32] add unit tests --- library_generation/test/unit_tests.py | 56 ++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 0e00ee26dc..276a576f0f 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -22,6 +22,8 @@ import contextlib from pathlib import Path from difflib import unified_diff +from unittest.mock import patch + from typing import List from parameterized import parameterized from library_generation import utilities as util @@ -31,6 +33,7 @@ from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig from library_generation.utilities import find_versioned_proto_path +from library_generation.utilities import format_commit_message from library_generation.utilities import get_file_paths from library_generation.utilities import wrap_nested_commit @@ -508,7 +511,58 @@ def test_monorepo_postprocessing_valid_repository_success(self): actual=f"{repository_path}/gapic-libraries-bom/pom.xml", ) - def test_format_commit_message + @parameterized.expand( + [ + ( + "feat: a commit message\nPiperOrigin-RevId: 123456", + [ + "BEGIN_NESTED_COMMIT", + "feat: [example_library] a commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + True, + ), + ( + "feat: a commit message\nPiperOrigin-RevId: 123456", + [ + "BEGIN_NESTED_COMMIT", + "feat: a commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + False, + ), + ( + "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456", + [ + "BEGIN_NESTED_COMMIT", + "feat: [example_library] a commit message", + "fix: [example_library] an another commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + True, + ), + ( + "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456", + [ + "BEGIN_NESTED_COMMIT", + "feat: a commit message", + "fix: an another commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + False, + ), + ] + ) + def test_format_commit_message_success(self, message, expected, is_monorepo): + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.message = message + commits = {commit: "example_library"} + self.assertEqual(expected, format_commit_message(commits, is_monorepo)) def test_wrap_nested_commit_success(self): messages = ["a commit message", "another message"] From 2b507aa8a5d41d9c712939ddb71b42c913fd4f1c Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Thu, 7 Mar 2024 19:08:15 +0000 Subject: [PATCH 32/32] refactor class and unit tests --- .../workflows/verify_library_generation.yaml | 2 +- library_generation/generate_pr_description.py | 4 +- library_generation/test/unit_tests.py | 68 ---------- library_generation/test/utils/__init__.py | 0 .../commit_message_formatter_unit_tests.py | 116 ++++++++++++++++++ library_generation/utilities.py | 48 -------- .../utils/commit_message_formatter.py | 64 ++++++++++ 7 files changed, 183 insertions(+), 119 deletions(-) create mode 100644 library_generation/test/utils/__init__.py create mode 100644 library_generation/test/utils/commit_message_formatter_unit_tests.py create mode 100644 library_generation/utils/commit_message_formatter.py diff --git a/.github/workflows/verify_library_generation.yaml b/.github/workflows/verify_library_generation.yaml index 89cec5ddc9..6299c66612 100644 --- a/.github/workflows/verify_library_generation.yaml +++ b/.github/workflows/verify_library_generation.yaml @@ -97,7 +97,7 @@ jobs: - name: Run python unit tests run: | set -x - python -m unittest library_generation/test/unit_tests.py + python -m unittest discover -s library_generation/test/ -p "*unit_tests.py" lint-shell: runs-on: ubuntu-22.04 steps: diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py index e272e8a4d3..5c98912331 100644 --- a/library_generation/generate_pr_description.py +++ b/library_generation/generate_pr_description.py @@ -20,9 +20,9 @@ from git import Commit, Repo from library_generation.model.generation_config import from_yaml from library_generation.utilities import find_versioned_proto_path -from library_generation.utilities import format_commit_message +from library_generation.utils.commit_message_formatter import format_commit_message from library_generation.utilities import get_file_paths -from library_generation.utilities import wrap_nested_commit +from library_generation.utils.commit_message_formatter import wrap_nested_commit @click.group(invoke_without_command=False) diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 276a576f0f..909414e20e 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -22,7 +22,6 @@ import contextlib from pathlib import Path from difflib import unified_diff -from unittest.mock import patch from typing import List from parameterized import parameterized @@ -33,9 +32,7 @@ from library_generation.model.generation_config import from_yaml from library_generation.model.library_config import LibraryConfig from library_generation.utilities import find_versioned_proto_path -from library_generation.utilities import format_commit_message from library_generation.utilities import get_file_paths -from library_generation.utilities import wrap_nested_commit script_dir = os.path.dirname(os.path.realpath(__file__)) resources_dir = os.path.join(script_dir, "resources") @@ -511,71 +508,6 @@ def test_monorepo_postprocessing_valid_repository_success(self): actual=f"{repository_path}/gapic-libraries-bom/pom.xml", ) - @parameterized.expand( - [ - ( - "feat: a commit message\nPiperOrigin-RevId: 123456", - [ - "BEGIN_NESTED_COMMIT", - "feat: [example_library] a commit message", - "PiperOrigin-RevId: 123456", - "END_NESTED_COMMIT", - ], - True, - ), - ( - "feat: a commit message\nPiperOrigin-RevId: 123456", - [ - "BEGIN_NESTED_COMMIT", - "feat: a commit message", - "PiperOrigin-RevId: 123456", - "END_NESTED_COMMIT", - ], - False, - ), - ( - "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456", - [ - "BEGIN_NESTED_COMMIT", - "feat: [example_library] a commit message", - "fix: [example_library] an another commit message", - "PiperOrigin-RevId: 123456", - "END_NESTED_COMMIT", - ], - True, - ), - ( - "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456", - [ - "BEGIN_NESTED_COMMIT", - "feat: a commit message", - "fix: an another commit message", - "PiperOrigin-RevId: 123456", - "END_NESTED_COMMIT", - ], - False, - ), - ] - ) - def test_format_commit_message_success(self, message, expected, is_monorepo): - with patch("git.Commit") as mock_commit: - commit = mock_commit.return_value - commit.message = message - commits = {commit: "example_library"} - self.assertEqual(expected, format_commit_message(commits, is_monorepo)) - - def test_wrap_nested_commit_success(self): - messages = ["a commit message", "another message"] - self.assertEqual( - [ - "BEGIN_NESTED_COMMIT", - "a commit message", - "another message", - "END_NESTED_COMMIT", - ], - wrap_nested_commit(messages), - ) - def __compare_files(self, expect: str, actual: str): with open(expect, "r") as f: expected_lines = f.readlines() diff --git a/library_generation/test/utils/__init__.py b/library_generation/test/utils/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/library_generation/test/utils/commit_message_formatter_unit_tests.py b/library_generation/test/utils/commit_message_formatter_unit_tests.py new file mode 100644 index 0000000000..5fd3599963 --- /dev/null +++ b/library_generation/test/utils/commit_message_formatter_unit_tests.py @@ -0,0 +1,116 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import unittest +from unittest.mock import patch + +from library_generation.utils.commit_message_formatter import format_commit_message +from library_generation.utils.commit_message_formatter import wrap_nested_commit + + +class CommitMessageFormatterTest(unittest.TestCase): + def test_format_commit_message_should_add_library_name_for_conventional_commit( + self, + ): + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.message = "feat: a commit message\nPiperOrigin-RevId: 123456" + commits = {commit: "example_library"} + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "feat: [example_library] a commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + format_commit_message(commits, True), + ) + + def test_format_commit_message_should_add_library_name_for_mutliline_conventional_commit( + self, + ): + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.message = "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456" + commits = {commit: "example_library"} + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "feat: [example_library] a commit message", + "fix: [example_library] an another commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + format_commit_message(commits, True), + ) + + def test_format_commit_message_should_not_add_library_name_for_nonconvnentional_commit( + self, + ): + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.message = "PiperOrigin-RevId: 123456" + commits = {commit: "example_library"} + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + format_commit_message(commits, True), + ) + + def test_format_commit_message_should_not_add_library_name_if_not_monorepo(self): + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.message = "feat: a commit message\nPiperOrigin-RevId: 123456" + commits = {commit: "example_library"} + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "feat: a commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + format_commit_message(commits, False), + ) + + def test_format_commit_message_should_not_add_library_name_for_multiline_commit_if_not_monorepo( + self, + ): + with patch("git.Commit") as mock_commit: + commit = mock_commit.return_value + commit.message = "feat: a commit message\nfix: an another commit message\nPiperOrigin-RevId: 123456" + commits = {commit: "example_library"} + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "feat: a commit message", + "fix: an another commit message", + "PiperOrigin-RevId: 123456", + "END_NESTED_COMMIT", + ], + format_commit_message(commits, False), + ) + + def test_wrap_nested_commit_success(self): + messages = ["a commit message", "another message"] + self.assertEqual( + [ + "BEGIN_NESTED_COMMIT", + "a commit message", + "another message", + "END_NESTED_COMMIT", + ], + wrap_nested_commit(messages), + ) diff --git a/library_generation/utilities.py b/library_generation/utilities.py index a4ac74c4b4..2faee66f7a 100755 --- a/library_generation/utilities.py +++ b/library_generation/utilities.py @@ -21,7 +21,6 @@ from typing import Dict from lxml import etree -from git import Commit from library_generation.model.bom_config import BomConfig from library_generation.model.generation_config import GenerationConfig from library_generation.model.library_config import LibraryConfig @@ -519,50 +518,3 @@ def find_versioned_proto_path(file_path: str) -> str: idx = file_path.find(version) return file_path[:idx] + version return file_path - - -def format_commit_message(commits: Dict[Commit, str], is_monorepo: bool) -> List[str]: - """ - Format commit messages. Add library_name to conventional commit messages if - is_monorepo is True; otherwise no op. - - :param commits: a mapping from commit to library_name. - :param is_monorepo: whether it's monorepo or not. - :return: formatted commit messages. - """ - all_commits = [] - # please see go/java-client-releasing#conventional-commit-messages - # for conventional commit. - type_regex = re.compile(r"(feat|fix|docs|deps|test|samples|chore)!?:.*") - for commit, library_name in commits.items(): - # a commit message may contain multiple lines, we need to - # add library_name for each line. - messages = [] - for message_line in commit.message.split("\n"): - # add library name to a conventional commit message; - # otherwise no op. - if type_regex.search(message_line): - commit_type, _, summary = message_line.partition(":") - formatted_message = ( - f"{commit_type}: [{library_name}]{str(summary).rstrip()}" - if is_monorepo - else f"{commit_type}:{str(summary).rstrip()}" - ) - messages.append(formatted_message) - else: - messages.append(message_line) - all_commits.extend(wrap_nested_commit(messages)) - return all_commits - - -def wrap_nested_commit(messages: List[str]) -> List[str]: - """ - Wrap message between `BEGIN_NESTED_COMMIT` and `BEGIN_NESTED_COMMIT`. - - :param messages: a (multi-line) commit message, one line per item. - :return: wrapped messages. - """ - result = ["BEGIN_NESTED_COMMIT"] - result.extend(messages) - result.append("END_NESTED_COMMIT") - return result diff --git a/library_generation/utils/commit_message_formatter.py b/library_generation/utils/commit_message_formatter.py new file mode 100644 index 0000000000..afa51a4db6 --- /dev/null +++ b/library_generation/utils/commit_message_formatter.py @@ -0,0 +1,64 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import re +from typing import List +from typing import Dict +from git import Commit + + +def format_commit_message(commits: Dict[Commit, str], is_monorepo: bool) -> List[str]: + """ + Format commit messages. Add library_name to conventional commit messages if + is_monorepo is True; otherwise no op. + + :param commits: a mapping from commit to library_name. + :param is_monorepo: whether it's monorepo or not. + :return: formatted commit messages. + """ + all_commits = [] + # please see go/java-client-releasing#conventional-commit-messages + # for conventional commit. + type_regex = re.compile(r"(feat|fix|docs|deps|test|samples|chore)!?:.*") + for commit, library_name in commits.items(): + # a commit message may contain multiple lines, we need to + # add library_name for each line. + messages = [] + for message_line in commit.message.split("\n"): + # add library name to a conventional commit message; + # otherwise no op. + if type_regex.search(message_line): + commit_type, _, summary = message_line.partition(":") + formatted_message = ( + f"{commit_type}: [{library_name}]{str(summary).rstrip()}" + if is_monorepo + else f"{commit_type}:{str(summary).rstrip()}" + ) + messages.append(formatted_message) + else: + messages.append(message_line) + all_commits.extend(wrap_nested_commit(messages)) + return all_commits + + +def wrap_nested_commit(messages: List[str]) -> List[str]: + """ + Wrap message between `BEGIN_NESTED_COMMIT` and `BEGIN_NESTED_COMMIT`. + + :param messages: a (multi-line) commit message, one line per item. + :return: wrapped messages. + """ + result = ["BEGIN_NESTED_COMMIT"] + result.extend(messages) + result.append("END_NESTED_COMMIT") + return result