fix: generate pr description with repo level change (#3182)

JoeWang1127 · web-flow · commit edd2168fdc7b · 2024-09-10T12:32:07.000-04:00
In this PR: - Generate PR description if config has repo level change without qualified commit. Context: googleapis/java-storage#2672 doesn't generate PR description even though the generator version and librararies-bom version are changed.
diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py
@@ -81,13 +81,14 @@ def get_repo_level_commit_messages(
     baseline_commit_sha: str,
     paths: Dict[str, str],
     is_monorepo: bool,
-    repo_level_message: list[str] = None,
+    repo_level_message: list[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.
+    Note that baseline_commit should be an ancestor of or the same as
+    latest_commit.
 
     :param repo_url: the url of the repository.
     :param current_commit_sha: the newest commit to be considered in
@@ -101,8 +102,6 @@ def get_repo_level_commit_messages(
     :raise ValueError: if current_commit is older than or equal to
     baseline_commit.
     """
-    if current_commit_sha == baseline_commit_sha:
-        return EMPTY_MESSAGE
     tmp_dir = "/tmp/repo"
     shutil.rmtree(tmp_dir, ignore_errors=True)
     os.mkdir(tmp_dir)
@@ -111,11 +110,12 @@ def get_repo_level_commit_messages(
     baseline_commit = repo.commit(baseline_commit_sha)
     current_commit_time = __get_commit_timestamp(current_commit)
     baseline_commit_time = __get_commit_timestamp(baseline_commit)
-    if current_commit_time <= baseline_commit_time:
+    if current_commit_time < baseline_commit_time:
         raise ValueError(
             f"current_commit ({current_commit_sha[:7]}, committed on "
-            f"{current_commit_time}) should be newer than baseline_commit "
-            f"({baseline_commit_sha[:7]}, committed on {baseline_commit_time})."
+            f"{current_commit_time}) should be newer than or equal to "
+            f"baseline_commit ({baseline_commit_sha[:7]}, committed on "
+            f"{baseline_commit_time})."
         )
     qualified_commits = {}
     commit = current_commit
@@ -128,8 +128,6 @@ def get_repo_level_commit_messages(
             break
         commit = commit_parents[0]
     shutil.rmtree(tmp_dir, ignore_errors=True)
-    if len(qualified_commits) == 0:
-        return EMPTY_MESSAGE
 
     return __combine_commit_messages(
         current_commit=current_commit,
@@ -165,15 +163,19 @@ def __combine_commit_messages(
     is_monorepo: bool,
     repo_level_message: list[str],
 ) -> str:
-    description = [
-        f"This pull request is generated with proto changes between "
-        f"{commit_link(baseline_commit)} (exclusive) "
-        f"and {commit_link(current_commit)} (inclusive).\n",
-    ]
+    description = []
+    if current_commit != baseline_commit:
+        description.append(
+            f"This pull request is generated with proto changes between "
+            f"{commit_link(baseline_commit)} (exclusive) "
+            f"and {commit_link(current_commit)} (inclusive).\n",
+        )
     commit_message = repo_level_message
     commit_message.extend(
         format_commit_message(commits=commits, is_monorepo=is_monorepo)
     )
+    if len(commit_message) == 0:
+        return EMPTY_MESSAGE
     description.extend(wrap_override_commit(commit_message))
     return "\n".join(description)
 
diff --git a/library_generation/test/generate_pr_description_unit_tests.py b/library_generation/test/generate_pr_description_unit_tests.py
@@ -47,6 +47,7 @@ def test_get_commit_messages_current_is_older_raise_exception(self):
             baseline_commit,
             {},
             True,
+            [],
         )
 
     def test_get_commit_messages_with_same_current_and_baseline_returns_empty_message(
@@ -63,6 +64,7 @@ def test_get_commit_messages_with_same_current_and_baseline_returns_empty_messag
                 baseline_commit,
                 {},
                 True,
+                [],
             ),
         )
 
@@ -168,3 +170,101 @@ def test_generate_pr_description_with_combined_message(
             "The generated PR description does not match the expected golden file",
         )
         os.remove(f"{cwd}/pr_description.txt")
+
+    def test_generate_pr_description_with_repo_level_change_without_qualified_commit(
+        self,
+    ):
+        # no other commits between these two commits.
+        baseline_commit_sha = "3b6f144d47b0a1d2115ab2445ec06e80cc324a44"
+        current_commit_sha = "0cea7170404bec3d994f43db4fa292f5034cbe9a"
+        cwd = os.getcwd()
+        library = LibraryConfig(
+            api_shortname="example_library",
+            api_description="",
+            name_pretty="",
+            product_documentation="",
+            gapic_configs=[GapicConfig(proto_path="google/example/v1")],
+        )
+        generate_pr_descriptions(
+            config_change=ConfigChange(
+                change_to_libraries={
+                    ChangeType.REPO_LEVEL_CHANGE: [
+                        LibraryChange(
+                            changed_param="gapic_generator_version",
+                            current_value="1.2.3",
+                        ),
+                        LibraryChange(
+                            changed_param="libraries_bom_version", current_value="2.3.4"
+                        ),
+                    ],
+                    ChangeType.GOOGLEAPIS_COMMIT: [],
+                },
+                baseline_config=GenerationConfig(
+                    gapic_generator_version="",
+                    googleapis_commitish=baseline_commit_sha,
+                    libraries=[library],
+                ),
+                current_config=GenerationConfig(
+                    gapic_generator_version="1.2.3",
+                    googleapis_commitish=current_commit_sha,
+                    libraries_bom_version="2.3.4",
+                    libraries=[library],
+                ),
+            ),
+            description_path=cwd,
+        )
+        self.assertTrue(os.path.isfile(f"{cwd}/pr_description.txt"))
+        self.assertTrue(
+            cmp(
+                f"{resources_dir}/repo_level_and_no_qualified_commit_pr_description-golden.txt",
+                f"{cwd}/pr_description.txt",
+            ),
+            "The generated PR description does not match the expected golden file",
+        )
+        os.remove(f"{cwd}/pr_description.txt")
+
+    def test_generate_pr_description_create_description_with_only_repo_level_change(
+        self,
+    ):
+        commit_sha = "3b6f144d47b0a1d2115ab2445ec06e80cc324a44"
+        cwd = os.getcwd()
+        library = LibraryConfig(
+            api_shortname="documentai",
+            api_description="",
+            name_pretty="",
+            product_documentation="",
+            gapic_configs=[GapicConfig(proto_path="google/cloud/documentai/v1")],
+        )
+        generate_pr_descriptions(
+            config_change=ConfigChange(
+                change_to_libraries={
+                    ChangeType.REPO_LEVEL_CHANGE: [
+                        LibraryChange(
+                            changed_param="gapic_generator_version",
+                            current_value="1.2.3",
+                        )
+                    ],
+                    ChangeType.GOOGLEAPIS_COMMIT: [],
+                },
+                baseline_config=GenerationConfig(
+                    gapic_generator_version="1.2.2",
+                    googleapis_commitish=commit_sha,
+                    libraries=[library],
+                ),
+                current_config=GenerationConfig(
+                    gapic_generator_version="1.2.3",
+                    googleapis_commitish=commit_sha,
+                    libraries=[library],
+                ),
+            ),
+            description_path=cwd,
+        )
+        self.assertTrue(os.path.isfile(f"{cwd}/pr_description.txt"))
+        self.assertTrue(
+            cmp(
+                f"{resources_dir}/repo_level_only_pr_description-golden.txt",
+                f"{cwd}/pr_description.txt",
+            ),
+            "The generated PR description does not match the expected golden file",
+        )
+        os.remove(f"{cwd}/pr_description.txt")
diff --git a/library_generation/test/resources/goldens/repo_level_and_no_qualified_commit_pr_description-golden.txt b/library_generation/test/resources/goldens/repo_level_and_no_qualified_commit_pr_description-golden.txt
@@ -0,0 +1,10 @@
+This pull request is generated with proto changes between [googleapis/googleapis@3b6f144](https://github.com/googleapis/googleapis/commit/3b6f144d47b0a1d2115ab2445ec06e80cc324a44) (exclusive) and [googleapis/googleapis@0cea717](https://github.com/googleapis/googleapis/commit/0cea7170404bec3d994f43db4fa292f5034cbe9a) (inclusive).
+
+BEGIN_COMMIT_OVERRIDE
+BEGIN_NESTED_COMMIT
+fix(deps): update the Java code generator (gapic-generator-java) to 1.2.3
+END_NESTED_COMMIT
+BEGIN_NESTED_COMMIT
+chore: update the libraries_bom version to 2.3.4
+END_NESTED_COMMIT
+END_COMMIT_OVERRIDE
diff --git a/library_generation/test/resources/goldens/repo_level_only_pr_description-golden.txt b/library_generation/test/resources/goldens/repo_level_only_pr_description-golden.txt
@@ -0,0 +1,5 @@
+BEGIN_COMMIT_OVERRIDE
+BEGIN_NESTED_COMMIT
+fix(deps): update the Java code generator (gapic-generator-java) to 1.2.3
+END_NESTED_COMMIT
+END_COMMIT_OVERRIDE
