diff --git a/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 index c174d37e88..3ad8ef7689 100644 --- a/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 +++ b/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 @@ -545,11 +545,15 @@ def test_{{ method_name }}(request_type, transport: str = 'grpc'): {% if not field.oneof or field.proto3_optional %} {{ field.name }}={{ field.mock_value }}, {% endif %}{% endfor %} - {# This is a hack to only pick one field #} + {# This is a hack to only pick one field #} {% for oneof_fields in method.output.oneof_fields().values() %} - {% with field = oneof_fields[0] %} + {# Take the first non-message oneof field. In the corner case when all the fields of a oneof are messages, the oneof will not be populated #} + {# Use an outer if-statement here because `first` raises an error if called on an empty sequence #} + {% if (oneof_fields | rejectattr('message') | list) %} + {% with field = (oneof_fields | rejectattr('message') | first) %} {{ field.name }}={{ field.mock_value }}, {% endwith %} + {% endif %} {% endfor %} ) {% endif %} @@ -959,11 +963,13 @@ def test_{{ method_name }}_rest(request_type, transport: str = 'rest'): {% if not field.oneof or field.proto3_optional %} {{ field.name }}={{ field.mock_value }}, {% endif %}{% endfor %} - {# This is a hack to only pick one field #} + {# This is a hack to only pick one field #} {% for oneof_fields in method.output.oneof_fields().values() %} - {% with field = oneof_fields[0] %} + {% if (oneof_fields | rejectattr('message') | list) %} + {% with field = (oneof_fields | rejectattr('message') | first) %} {{ field.name }}={{ field.mock_value }}, {% endwith %} + {% endif %} {% endfor %} ) {% endif %} @@ -1029,11 +1035,13 @@ def test_{{ method.name|snake_case }}_rest(request_type): {% if not field.oneof or field.proto3_optional %} {{ field.name }}={{ field.mock_value }}, {% endif %}{% endfor %} - {# This is a hack to only pick one field #} + {# This is a hack to only pick one field #} {% for oneof_fields in method.output.oneof_fields().values() %} - {% with field = oneof_fields[0] %} + {% if (oneof_fields | rejectattr('message') | list) %} + {% with field = (oneof_fields | rejectattr('message') | first) %} {{ field.name }}={{ field.mock_value }}, {% endwith %} + {% endif %} {% endfor %} ) {% endif %} diff --git a/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2 b/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2 index 952751323b..2d895cab03 100644 --- a/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2 +++ b/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2 @@ -34,11 +34,15 @@ def test_{{ method_name }}(request_type, transport: str = 'grpc'): {% if not field.oneof or field.proto3_optional %} {{ field.name }}={{ field.mock_value }}, {% endif %}{% endfor %} - {# This is a hack to only pick one field #} + {# This is a hack to only pick one field #} {% for oneof_fields in method.output.oneof_fields().values() %} - {% with field = oneof_fields[0] %} + {# Take the first non-message oneof field. In the corner case when all the fields of a oneof are messages, the oneof will not be populated #} + {# Use an outer if-statement here because `first` raises an error if called on an empty sequence #} + {% if (oneof_fields | rejectattr('message') | list) %} + {% with field = (oneof_fields | rejectattr('message') | first) %} {{ field.name }}={{ field.mock_value }}, {% endwith %} + {% endif %} {% endfor %} ) {% endif %} @@ -903,11 +907,13 @@ def test_{{ method_name }}_rest(request_type): {% if not field.oneof or field.proto3_optional %} {{ field.name }}={{ field.mock_value }}, {% endif %}{% endfor %} - {# This is a hack to only pick one field #} - {% for oneof_fields in method.extended_lro.operation_type.oneof_fields().values() %} - {% with field = oneof_fields[0] %} - {{ field.name }}={{ field.mock_value }}, - {% endwith %} + {# This is a hack to only pick one field #} + {% for oneof_fields in method.output.oneof_fields().values() %} + {% if (oneof_fields | rejectattr('message') | list) %} + {% with field = (oneof_fields | rejectattr('message') | first) %} + {{ field.name }}={{ field.mock_value }}, + {% endwith %} + {% endif %} {% endfor %} ) {% else %} @@ -916,12 +922,14 @@ def test_{{ method_name }}_rest(request_type): {% if not field.oneof or field.proto3_optional %} {{ field.name }}={{ field.mock_value }}, {% endif %}{% endfor %} - {# This is a hack to only pick one field #} - {% for oneof_fields in method.output.oneof_fields().values() %} - {% with field = oneof_fields[0] %} - {{ field.name }}={{ field.mock_value }}, - {% endwith %} - {% endfor %} + {# This is a hack to only pick one field #} + {% for oneof_fields in method.output.oneof_fields().values() %} + {% if (oneof_fields | rejectattr('message') | list) %} + {% with field = (oneof_fields | rejectattr('message') | first) %} + {{ field.name }}={{ field.mock_value }}, + {% endwith %} + {% endif %} + {% endfor %} ) {% endif %} diff --git a/tests/fragments/test_oneof_imported_response.proto b/tests/fragments/test_oneof_imported_response.proto new file mode 100644 index 0000000000..53f2a2cbd2 --- /dev/null +++ b/tests/fragments/test_oneof_imported_response.proto @@ -0,0 +1,41 @@ +// Copyright (C) 2021 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +package google.fragment; + +import "google/api/client.proto"; +import "import.proto"; + +service MyService { + option (google.api.default_host) = "my.example.com"; + rpc MyMethod(MethodRequest) returns (MethodResponse) {} +} + +message MethodRequest { + string input = 1; +} + +message Container { + Import import = 1; +} + +message MethodResponse { + string parent = 1; + oneof format { + Container container = 2; + string name = 3; + } +} diff --git a/tests/integration/goldens/logging/tests/unit/gapic/logging_v2/test_config_service_v2.py b/tests/integration/goldens/logging/tests/unit/gapic/logging_v2/test_config_service_v2.py index a11636125c..079e9cd542 100755 --- a/tests/integration/goldens/logging/tests/unit/gapic/logging_v2/test_config_service_v2.py +++ b/tests/integration/goldens/logging/tests/unit/gapic/logging_v2/test_config_service_v2.py @@ -3248,7 +3248,6 @@ def test_get_sink(request_type, transport: str = 'grpc'): output_version_format=logging_config.LogSink.VersionFormat.V2, writer_identity='writer_identity_value', include_children=True, - bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True), ) response = client.get_sink(request) @@ -3509,7 +3508,6 @@ def test_create_sink(request_type, transport: str = 'grpc'): output_version_format=logging_config.LogSink.VersionFormat.V2, writer_identity='writer_identity_value', include_children=True, - bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True), ) response = client.create_sink(request) @@ -3780,7 +3778,6 @@ def test_update_sink(request_type, transport: str = 'grpc'): output_version_format=logging_config.LogSink.VersionFormat.V2, writer_identity='writer_identity_value', include_children=True, - bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True), ) response = client.update_sink(request) @@ -8546,7 +8543,6 @@ def test_get_sink_rest(request_type): output_version_format=logging_config.LogSink.VersionFormat.V2, writer_identity='writer_identity_value', include_children=True, - bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True), ) # Wrap the value into a proper Response obj @@ -8794,7 +8790,6 @@ def test_create_sink_rest(request_type): output_version_format=logging_config.LogSink.VersionFormat.V2, writer_identity='writer_identity_value', include_children=True, - bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True), ) # Wrap the value into a proper Response obj @@ -9048,7 +9043,6 @@ def test_update_sink_rest(request_type): output_version_format=logging_config.LogSink.VersionFormat.V2, writer_identity='writer_identity_value', include_children=True, - bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True), ) # Wrap the value into a proper Response obj