From 80b4384922e1d3dd00daffb211b34449a3663d4a Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Mon, 6 May 2024 11:22:09 -0400 Subject: [PATCH] fix: do not populate repo level change while removing library --- ...generation_config_comparator_unit_tests.py | 15 ++++++++++++ .../utils/generation_config_comparator.py | 23 +++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/library_generation/test/utils/generation_config_comparator_unit_tests.py b/library_generation/test/utils/generation_config_comparator_unit_tests.py index fa466ac34d..5792ead347 100644 --- a/library_generation/test/utils/generation_config_comparator_unit_tests.py +++ b/library_generation/test/utils/generation_config_comparator_unit_tests.py @@ -178,6 +178,21 @@ def test_compare_config_library_addition(self): config_change = result.change_to_libraries[ChangeType.LIBRARIES_ADDITION][0] self.assertEqual("new_library", config_change.library_name) + def test_compare_config_library_removal_does_not_have_repo_or_library_level_change( + self, + ): + self.current_config.libraries = [] + result = compare_config( + baseline_config=self.baseline_config, + current_config=self.current_config, + ) + self.assertTrue( + len(result.change_to_libraries[ChangeType.REPO_LEVEL_CHANGE]) == 0 + ) + self.assertTrue( + len(result.change_to_libraries[ChangeType.LIBRARY_LEVEL_CHANGE]) == 0 + ) + def test_compare_config_api_shortname_update_without_library_name(self): self.current_config.libraries[0].api_shortname = "new_api_shortname" result = compare_config( diff --git a/library_generation/utils/generation_config_comparator.py b/library_generation/utils/generation_config_comparator.py index 09b8a9c514..d96a09f378 100644 --- a/library_generation/utils/generation_config_comparator.py +++ b/library_generation/utils/generation_config_comparator.py @@ -38,8 +38,17 @@ def compare_config( :return: a ConfigChange objects. """ diff = defaultdict(list[LibraryChange]) - baseline_params = __convert_params_to_sorted_list(baseline_config) - current_params = __convert_params_to_sorted_list(current_config) + # Exclude `libraries` because an empty library list (e.g., by library + # removal) will cause this parameter appears in the sorted param list, + # which leads to unequal list of parameters. + excluded_params = {"libraries"} + baseline_params = __convert_params_to_sorted_list( + obj=baseline_config, excluded_params=excluded_params + ) + current_params = __convert_params_to_sorted_list( + obj=current_config, excluded_params=excluded_params + ) + for baseline_param, current_param in zip(baseline_params, current_params): if baseline_param == current_param: continue @@ -216,7 +225,7 @@ def __compare_gapic_configs( diff[ChangeType.GAPIC_ADDITION].append(config_change) -def __convert_params_to_sorted_list(obj: Any) -> List[tuple]: +def __convert_params_to_sorted_list(obj: Any, excluded_params=None) -> List[tuple]: """ Convert the parameter and its value of a given object to a sorted list of tuples. @@ -230,13 +239,17 @@ def __convert_params_to_sorted_list(obj: Any) -> List[tuple]: Note that built-in params, e.g., __str__, and methods will be skipped. - :param obj: an object + :param obj: an object. + :param excluded_params: excluded params. :return: a sorted list of tuples. """ + if excluded_params is None: + excluded_params = set() param_and_values = [] for param, value in vars(obj).items(): if ( - param.startswith("__") # skip built-in params + param in excluded_params + or param.startswith("__") # skip built-in params or callable(getattr(obj, param)) # skip methods # skip if the type of param is not one of the following types # 1. str