From 3789002586b56e1cc229dc59fd510c3fa39f1b79 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 16 Feb 2024 19:51:16 +0000 Subject: [PATCH 1/4] fix: ignore comment in BUILD --- library_generation/model/gapic_inputs.py | 23 +++++++---- .../misc/BUILD_comment_common_resources.bazel | 5 +++ .../misc/BUILD_comment_iam_policy.bazel | 5 +++ .../misc/BUILD_comment_locations.bazel | 5 +++ .../misc/BUILD_common_resources.bazel | 5 +++ .../BUILD_iam_locations.bazel | 2 - .../resources/misc/BUILD_iam_policy.bazel | 5 +++ .../test/resources/misc/BUILD_locations.bazel | 5 +++ .../misc/BUILD_no_additional_protos.bazel | 4 ++ .../BUILD_common_resources.bazel | 6 --- .../BUILD_iam_policy.bazel | 7 ---- .../BUILD_locations.bazel | 7 ---- library_generation/test/unit_tests.py | 41 +++++++++++++++++++ 13 files changed, 91 insertions(+), 29 deletions(-) create mode 100644 library_generation/test/resources/misc/BUILD_comment_common_resources.bazel create mode 100644 library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel create mode 100644 library_generation/test/resources/misc/BUILD_comment_locations.bazel create mode 100644 library_generation/test/resources/misc/BUILD_common_resources.bazel rename library_generation/test/resources/{search_additional_protos => misc}/BUILD_iam_locations.bazel (57%) create mode 100644 library_generation/test/resources/misc/BUILD_iam_policy.bazel create mode 100644 library_generation/test/resources/misc/BUILD_locations.bazel create mode 100644 library_generation/test/resources/misc/BUILD_no_additional_protos.bazel delete mode 100644 library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel delete mode 100644 library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel delete mode 100644 library_generation/test/resources/search_additional_protos/BUILD_locations.bazel diff --git a/library_generation/model/gapic_inputs.py b/library_generation/model/gapic_inputs.py index b6500a6f3d..7f70e25277 100644 --- a/library_generation/model/gapic_inputs.py +++ b/library_generation/model/gapic_inputs.py @@ -31,9 +31,11 @@ (.*?) \) """ -resource_pattern = r"//google/cloud:common_resources_proto" -location_pattern = r"//google/cloud/location:location_proto" -iam_pattern = r"//google/iam/v1:iam_policy_proto" +# match leading "#" and any number of whitespace before +# "//google/cloud:common_resources_proto. +resource_pattern = r"\#*\s*\"//google/cloud:common_resources_proto" +location_pattern = r"\#*\s*\"//google/cloud/location:location_proto" +iam_pattern = r"\#*\s*\"//google/iam/v1:iam_policy_proto" transport_pattern = r"transport = \"(.*?)\"" rest_pattern = r"rest_numeric_enums = True" gapic_yaml_pattern = r"gapic_yaml = \"(.*?)\"" @@ -97,7 +99,9 @@ def parse( if len(assembly_target) > 0: include_samples = __parse_include_samples(assembly_target[0]) if len(gapic_target) == 0: - return GapicInputs(include_samples=include_samples) + return GapicInputs( + additional_protos=additional_protos, include_samples=include_samples + ) transport = __parse_transport(gapic_target[0]) rest_numeric_enum = __parse_rest_numeric_enums(gapic_target[0]) @@ -119,11 +123,16 @@ def parse( def __parse_additional_protos(proto_library_target: str) -> str: res = [" "] - if len(re.findall(resource_pattern, proto_library_target)) != 0: + resource = re.findall(resource_pattern, proto_library_target) + if len(resource) != 0 and (not str(resource[0]).strip().startswith("#")): + # only add common_resources.proto if this is not a comment (with + # leading "#"). res.append("google/cloud/common_resources.proto") - if len(re.findall(location_pattern, proto_library_target)) != 0: + location = re.findall(location_pattern, proto_library_target) + if len(location) != 0 and (not str(location[0]).strip().startswith("#")): res.append("google/cloud/location/locations.proto") - if len(re.findall(iam_pattern, proto_library_target)) != 0: + iam = re.findall(iam_pattern, proto_library_target) + if len(iam) != 0 and (not str(iam[0]).strip().startswith("#")): res.append("google/iam/v1/iam_policy.proto") return " ".join(res) diff --git a/library_generation/test/resources/misc/BUILD_comment_common_resources.bazel b/library_generation/test/resources/misc/BUILD_comment_common_resources.bazel new file mode 100644 index 0000000000..126ffdb7ca --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_comment_common_resources.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + #"//google/cloud:common_resources_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel b/library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel new file mode 100644 index 0000000000..a9a2c1ca75 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_comment_iam_policy.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + # "//google/iam/v1:iam_policy_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_comment_locations.bazel b/library_generation/test/resources/misc/BUILD_comment_locations.bazel new file mode 100644 index 0000000000..8b96e3ab81 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_comment_locations.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + # "//google/cloud/location:location_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_common_resources.bazel b/library_generation/test/resources/misc/BUILD_common_resources.bazel new file mode 100644 index 0000000000..9b749e6ad5 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_common_resources.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + "//google/cloud:common_resources_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_iam_locations.bazel b/library_generation/test/resources/misc/BUILD_iam_locations.bazel similarity index 57% rename from library_generation/test/resources/search_additional_protos/BUILD_iam_locations.bazel rename to library_generation/test/resources/misc/BUILD_iam_locations.bazel index e8241995e2..d26da6587a 100644 --- a/library_generation/test/resources/search_additional_protos/BUILD_iam_locations.bazel +++ b/library_generation/test/resources/misc/BUILD_iam_locations.bazel @@ -1,5 +1,3 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - proto_library_with_info( deps = [ "//google/iam/v1:iam_policy_proto", diff --git a/library_generation/test/resources/misc/BUILD_iam_policy.bazel b/library_generation/test/resources/misc/BUILD_iam_policy.bazel new file mode 100644 index 0000000000..af5d4a32f8 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_iam_policy.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + "//google/iam/v1:iam_policy_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_locations.bazel b/library_generation/test/resources/misc/BUILD_locations.bazel new file mode 100644 index 0000000000..29ee14fdba --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_locations.bazel @@ -0,0 +1,5 @@ +proto_library_with_info( + deps = [ + "//google/cloud/location:location_proto", + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/misc/BUILD_no_additional_protos.bazel b/library_generation/test/resources/misc/BUILD_no_additional_protos.bazel new file mode 100644 index 0000000000..a22257cad4 --- /dev/null +++ b/library_generation/test/resources/misc/BUILD_no_additional_protos.bazel @@ -0,0 +1,4 @@ +proto_library_with_info( + deps = [ + ] +) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel b/library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel deleted file mode 100644 index 45e3987adb..0000000000 --- a/library_generation/test/resources/search_additional_protos/BUILD_common_resources.bazel +++ /dev/null @@ -1,6 +0,0 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - -proto_library_with_info( - deps = [ - ] -) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel b/library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel deleted file mode 100644 index 81064a7eb4..0000000000 --- a/library_generation/test/resources/search_additional_protos/BUILD_iam_policy.bazel +++ /dev/null @@ -1,7 +0,0 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - -proto_library_with_info( - deps = [ - "//google/iam/v1:iam_policy_proto", - ] -) \ No newline at end of file diff --git a/library_generation/test/resources/search_additional_protos/BUILD_locations.bazel b/library_generation/test/resources/search_additional_protos/BUILD_locations.bazel deleted file mode 100644 index 07faa4ac95..0000000000 --- a/library_generation/test/resources/search_additional_protos/BUILD_locations.bazel +++ /dev/null @@ -1,7 +0,0 @@ -# this file is only used in testing `get_gapic_additional_protos_from_BUILD` in utilities.sh - -proto_library_with_info( - deps = [ - "//google/cloud/location:location_proto", - ] -) \ No newline at end of file diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index fc251e2472..6d88db44d9 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -214,6 +214,47 @@ 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_gapic_inputs_parse_add_protos_no_additions(self): + parsed = parse_build_file(build_file, "", "BUILD_no_additional_protos.bazel") + self.assertEqual(" ", parsed.additional_protos) + + def test_gapic_inputs_parse_add_protos_common_resources(self): + parsed = parse_build_file(build_file, "", "BUILD_common_resources.bazel") + self.assertEqual( + " google/cloud/common_resources.proto", parsed.additional_protos + ) + + def test_gapic_inputs_parse_add_protos_common_resources_comment_out(self): + parsed = parse_build_file( + build_file, "", "BUILD_comment_common_resources.bazel" + ) + self.assertEqual(" ", parsed.additional_protos) + + def test_gapic_inputs_parse_add_protos_location(self): + parsed = parse_build_file(build_file, "", "BUILD_locations.bazel") + self.assertEqual( + " google/cloud/location/locations.proto", parsed.additional_protos + ) + + def test_gapic_inputs_parse_add_protos_location_comment_out(self): + parsed = parse_build_file(build_file, "", "BUILD_comment_locations.bazel") + self.assertEqual(" ", parsed.additional_protos) + + def test_gapic_inputs_parse_add_protos_iam(self): + parsed = parse_build_file(build_file, "", "BUILD_iam_policy.bazel") + self.assertEqual(" google/iam/v1/iam_policy.proto", parsed.additional_protos) + + def test_gapic_inputs_parse_add_protos_iam_comment_out(self): + parsed = parse_build_file(build_file, "", "BUILD_comment_iam_policy.bazel") + self.assertEqual(" ", parsed.additional_protos) + + def test_gapic_inputs_parse_add_protos_iam_locations(self): + parsed = parse_build_file(build_file, "", "BUILD_iam_locations.bazel") + self.assertEqual( + " google/cloud/location/locations.proto google/iam/v1/iam_policy.proto", + parsed.additional_protos, + ) + def test_gapic_inputs_parse_grpc_only_succeeds(self): parsed = parse_build_file(build_file, "", "BUILD_grpc.bazel") self.assertEqual("grpc", parsed.transport) From 23b2a7b33dd8d6350f9e1a073960626c32ebe833 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 16 Feb 2024 22:18:46 +0000 Subject: [PATCH 2/4] use comment pattern --- library_generation/model/gapic_inputs.py | 33 ++++++++++++------------ 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/library_generation/model/gapic_inputs.py b/library_generation/model/gapic_inputs.py index 7f70e25277..6210a66597 100644 --- a/library_generation/model/gapic_inputs.py +++ b/library_generation/model/gapic_inputs.py @@ -31,11 +31,13 @@ (.*?) \) """ -# match leading "#" and any number of whitespace before -# "//google/cloud:common_resources_proto. -resource_pattern = r"\#*\s*\"//google/cloud:common_resources_proto" -location_pattern = r"\#*\s*\"//google/cloud/location:location_proto" -iam_pattern = r"\#*\s*\"//google/iam/v1:iam_policy_proto" +# match a line which the first character is "#". +comment_pattern = r"^\s*\#+" +pattern_to_proto = { + r"//google/cloud:common_resources_proto": "google/cloud/common_resources.proto", + r"//google/cloud/location:location_proto": "google/cloud/location/locations.proto", + r"//google/iam/v1:iam_policy_proto": "google/iam/v1/iam_policy.proto" +} transport_pattern = r"transport = \"(.*?)\"" rest_pattern = r"rest_numeric_enums = True" gapic_yaml_pattern = r"gapic_yaml = \"(.*?)\"" @@ -123,17 +125,16 @@ def parse( def __parse_additional_protos(proto_library_target: str) -> str: res = [" "] - resource = re.findall(resource_pattern, proto_library_target) - if len(resource) != 0 and (not str(resource[0]).strip().startswith("#")): - # only add common_resources.proto if this is not a comment (with - # leading "#"). - res.append("google/cloud/common_resources.proto") - location = re.findall(location_pattern, proto_library_target) - if len(location) != 0 and (not str(location[0]).strip().startswith("#")): - res.append("google/cloud/location/locations.proto") - iam = re.findall(iam_pattern, proto_library_target) - if len(iam) != 0 and (not str(iam[0]).strip().startswith("#")): - res.append("google/iam/v1/iam_policy.proto") + lines = proto_library_target.split("\n") + for line in lines: + if len(re.findall(comment_pattern, line)) != 0: + # skip a line which the first charactor is "#" since it's + # a comment. + continue + for pattern in pattern_to_proto: + if len(re.findall(pattern, line)) == 0: + continue + res.append(pattern_to_proto[pattern]) return " ".join(res) From aee92f7afd17f8b8565aaa193be6ec435ec32790 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 16 Feb 2024 22:19:07 +0000 Subject: [PATCH 3/4] add parameterized test --- .../resources/misc/BUILD_iam_locations.bazel | 2 +- library_generation/test/unit_tests.py | 55 ++++++------------- 2 files changed, 19 insertions(+), 38 deletions(-) diff --git a/library_generation/test/resources/misc/BUILD_iam_locations.bazel b/library_generation/test/resources/misc/BUILD_iam_locations.bazel index d26da6587a..d0c971da7c 100644 --- a/library_generation/test/resources/misc/BUILD_iam_locations.bazel +++ b/library_generation/test/resources/misc/BUILD_iam_locations.bazel @@ -1,6 +1,6 @@ proto_library_with_info( deps = [ - "//google/iam/v1:iam_policy_proto", "//google/cloud/location:location_proto", + "//google/iam/v1:iam_policy_proto", ] ) \ No newline at end of file diff --git a/library_generation/test/unit_tests.py b/library_generation/test/unit_tests.py index 6d88db44d9..625aaf8ffd 100644 --- a/library_generation/test/unit_tests.py +++ b/library_generation/test/unit_tests.py @@ -214,44 +214,25 @@ 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_gapic_inputs_parse_add_protos_no_additions(self): - parsed = parse_build_file(build_file, "", "BUILD_no_additional_protos.bazel") - self.assertEqual(" ", parsed.additional_protos) - - def test_gapic_inputs_parse_add_protos_common_resources(self): - parsed = parse_build_file(build_file, "", "BUILD_common_resources.bazel") - self.assertEqual( - " google/cloud/common_resources.proto", parsed.additional_protos - ) - - def test_gapic_inputs_parse_add_protos_common_resources_comment_out(self): - parsed = parse_build_file( - build_file, "", "BUILD_comment_common_resources.bazel" - ) - self.assertEqual(" ", parsed.additional_protos) - - def test_gapic_inputs_parse_add_protos_location(self): - parsed = parse_build_file(build_file, "", "BUILD_locations.bazel") - self.assertEqual( - " google/cloud/location/locations.proto", parsed.additional_protos - ) - - def test_gapic_inputs_parse_add_protos_location_comment_out(self): - parsed = parse_build_file(build_file, "", "BUILD_comment_locations.bazel") - self.assertEqual(" ", parsed.additional_protos) - - def test_gapic_inputs_parse_add_protos_iam(self): - parsed = parse_build_file(build_file, "", "BUILD_iam_policy.bazel") - self.assertEqual(" google/iam/v1/iam_policy.proto", parsed.additional_protos) - - def test_gapic_inputs_parse_add_protos_iam_comment_out(self): - parsed = parse_build_file(build_file, "", "BUILD_comment_iam_policy.bazel") - self.assertEqual(" ", parsed.additional_protos) - - def test_gapic_inputs_parse_add_protos_iam_locations(self): - parsed = parse_build_file(build_file, "", "BUILD_iam_locations.bazel") + @parameterized.expand( + [ + ("BUILD_no_additional_protos.bazel", " "), + ("BUILD_common_resources.bazel", " google/cloud/common_resources.proto"), + ("BUILD_comment_common_resources.bazel", " "), + ("BUILD_locations.bazel", " google/cloud/location/locations.proto"), + ("BUILD_comment_locations.bazel", " "), + ("BUILD_iam_policy.bazel", " google/iam/v1/iam_policy.proto"), + ("BUILD_comment_iam_policy.bazel", " "), + ( + "BUILD_iam_locations.bazel", + " google/cloud/location/locations.proto google/iam/v1/iam_policy.proto", + ), + ] + ) + def test_gapic_inputs_parse_additional_protos(self, build_name, expected): + parsed = parse_build_file(build_file, "", build_name) self.assertEqual( - " google/cloud/location/locations.proto google/iam/v1/iam_policy.proto", + expected, parsed.additional_protos, ) From 7283e96bd4ffc3553dca7a328ab198e031271537 Mon Sep 17 00:00:00 2001 From: JoeWang1127 Date: Fri, 16 Feb 2024 22:23:06 +0000 Subject: [PATCH 4/4] format code --- library_generation/model/gapic_inputs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library_generation/model/gapic_inputs.py b/library_generation/model/gapic_inputs.py index 6210a66597..4bb9ce64f4 100644 --- a/library_generation/model/gapic_inputs.py +++ b/library_generation/model/gapic_inputs.py @@ -36,7 +36,7 @@ pattern_to_proto = { r"//google/cloud:common_resources_proto": "google/cloud/common_resources.proto", r"//google/cloud/location:location_proto": "google/cloud/location/locations.proto", - r"//google/iam/v1:iam_policy_proto": "google/iam/v1/iam_policy.proto" + r"//google/iam/v1:iam_policy_proto": "google/iam/v1/iam_policy.proto", } transport_pattern = r"transport = \"(.*?)\"" rest_pattern = r"rest_numeric_enums = True"