Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: generate pr description with repo level change #3182

Merged
merged 2 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions library_generation/generate_pr_description.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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)

Expand Down
100 changes: 100 additions & 0 deletions library_generation/test/generate_pr_description_unit_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -63,6 +64,7 @@ def test_get_commit_messages_with_same_current_and_baseline_returns_empty_messag
baseline_commit,
{},
True,
[],
),
)

Expand Down Expand Up @@ -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")
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Loading