From 4cc72ad583a5e40fad725cbaa2f02d9953b29505 Mon Sep 17 00:00:00 2001 From: Diego Marquez Date: Fri, 30 Aug 2024 22:20:19 -0400 Subject: [PATCH] chore: always generate a repository if not a monorepo (#3131) Context in [this doc](https://docs.google.com/document/d/1wrpyBtphdenM3BNelcnpBKGADYrGJUo686HXvSA0h-0/edit?pli=1&tab=t.luhbzlh25mfs#bookmark=id.ruacqw13dxd2) This PR makes the generation workflow to always get triggered when the target repository is not a monorepo (i.e. handwritten libraries). This will still respect other changes between baseline and current configurations when it comes to PR description generation, but if no config changes are detected then no PR description will be created, assuming that the PR description of the original PR that will trigger the generation will be informative enough. --- library_generation/cli/entry_point.py | 25 +++++++- library_generation/generate_pr_description.py | 0 .../test/cli/entry_point_unit_tests.py | 62 ++++++++++++++++++- .../generation_config_library_modified.yaml | 14 +++++ 4 files changed, 99 insertions(+), 2 deletions(-) mode change 100644 => 100755 library_generation/generate_pr_description.py create mode 100644 library_generation/test/resources/test-config/generation_config_library_modified.yaml diff --git a/library_generation/cli/entry_point.py b/library_generation/cli/entry_point.py index dbcc18b267..7eefeeca05 100644 --- a/library_generation/cli/entry_point.py +++ b/library_generation/cli/entry_point.py @@ -88,6 +88,22 @@ def generate( The commit history, if generated, will be available in repository_path/pr_description.txt. """ + __generate_repo_and_pr_description_impl( + baseline_generation_config_path, current_generation_config_path, repository_path + ) + + +def __generate_repo_and_pr_description_impl( + baseline_generation_config_path: str, + current_generation_config_path: str, + repository_path: str, +): + """ + Implementation method for generate(). + The decoupling of generate and __generate_repo_and_pr_description_impl is + meant to allow testing of this implementation function. + """ + default_generation_config_path = f"{os.getcwd()}/generation_config.yaml" if ( @@ -129,10 +145,17 @@ def generate( baseline_config=from_yaml(baseline_generation_config_path), current_config=from_yaml(current_generation_config_path), ) + # pass None if this is not a monorepo in order to trigger the full + # generation + target_library_names = ( + config_change.get_changed_libraries() + if config_change.current_config.is_monorepo() + else None + ) generate_from_yaml( config=config_change.current_config, repository_path=repository_path, - target_library_names=config_change.get_changed_libraries(), + target_library_names=target_library_names, ) generate_pr_descriptions( config_change=config_change, diff --git a/library_generation/generate_pr_description.py b/library_generation/generate_pr_description.py old mode 100644 new mode 100755 diff --git a/library_generation/test/cli/entry_point_unit_tests.py b/library_generation/test/cli/entry_point_unit_tests.py index 55fb583651..66756be500 100644 --- a/library_generation/test/cli/entry_point_unit_tests.py +++ b/library_generation/test/cli/entry_point_unit_tests.py @@ -13,8 +13,14 @@ # limitations under the License. import os import unittest +from unittest.mock import patch, ANY from click.testing import CliRunner -from library_generation.cli.entry_point import generate, validate_generation_config +from library_generation.cli.entry_point import ( + generate, + validate_generation_config, + __generate_repo_and_pr_description_impl as generate_impl, +) +from library_generation.model.generation_config import from_yaml script_dir = os.path.dirname(os.path.realpath(__file__)) test_resource_dir = os.path.join(script_dir, "..", "resources", "test-config") @@ -78,3 +84,57 @@ def test_validate_generation_config_with_duplicate_library_name_raise_file_excep result.output, "have the same library name", ) + + @patch("library_generation.cli.entry_point.generate_from_yaml") + @patch("library_generation.cli.entry_point.generate_pr_descriptions") + def test_generate_non_monorepo_without_changes_triggers_full_generation( + self, + generate_pr_descriptions, + generate_from_yaml, + ): + """ + this tests confirms the behavior of generation of non monorepos + (HW libraries). generate() should call generate_from_yaml() + with target_library_names=None in order to trigger the full generation + """ + config_path = f"{test_resource_dir}/generation_config.yaml" + self.assertFalse(from_yaml(config_path).is_monorepo()) + # we call the implementation method directly since click + # does special handling when a method is annotated with @main.command() + generate_impl( + baseline_generation_config_path=config_path, + current_generation_config_path=config_path, + repository_path=".", + ) + generate_from_yaml.assert_called_with( + config=ANY, repository_path=ANY, target_library_names=None + ) + + @patch("library_generation.cli.entry_point.generate_from_yaml") + @patch("library_generation.cli.entry_point.generate_pr_descriptions") + def test_generate_non_monorepo_with_changes_triggers_full_generation( + self, + generate_pr_descriptions, + generate_from_yaml, + ): + """ + this tests confirms the behavior of generation of non monorepos + (HW libraries). generate() should call generate_from_yaml() + with target_library_names=None in order to trigger the full generation + """ + baseline_config_path = f"{test_resource_dir}/generation_config.yaml" + current_config_path = ( + f"{test_resource_dir}/generation_config_library_modified.yaml" + ) + self.assertFalse(from_yaml(current_config_path).is_monorepo()) + self.assertFalse(from_yaml(baseline_config_path).is_monorepo()) + # we call the implementation method directly since click + # does special handling when a method is annotated with @main.command() + generate_impl( + baseline_generation_config_path=baseline_config_path, + current_generation_config_path=current_config_path, + repository_path=".", + ) + generate_from_yaml.assert_called_with( + config=ANY, repository_path=ANY, target_library_names=None + ) diff --git a/library_generation/test/resources/test-config/generation_config_library_modified.yaml b/library_generation/test/resources/test-config/generation_config_library_modified.yaml new file mode 100644 index 0000000000..f9ae96693b --- /dev/null +++ b/library_generation/test/resources/test-config/generation_config_library_modified.yaml @@ -0,0 +1,14 @@ +gapic_generator_version: 2.34.0 +googleapis_commitish: 1a45bf7393b52407188c82e63101db7dc9c72026 +libraries_bom_version: 26.37.0 +libraries: + - api_shortname: cloudasset + name_pretty: Cloud Asset Inventory + product_documentation: "https://cloud.google.com/resource-manager/docs/cloud-asset-inventory/overview" + api_description: "provides inventory services based on a time series database." + GAPICs: + - proto_path: google/cloud/asset/v1 + - proto_path: google/cloud/asset/v1p1beta1 + - proto_path: google/cloud/asset/v1p2beta1 + - proto_path: google/cloud/asset/v1p5beta1 + - proto_path: google/cloud/asset/v1p7beta1