Skip to content

Commit 1d28775

Browse files
dizcologydizcology
authored
fix: mock return_value should not populate oneof message fields (#1668)
* fix: mock return_value should not populate oneof message fields
1 parent b887c73 commit 1d28775

4 files changed

Lines changed: 76 additions & 25 deletions

File tree

‎packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -545,11 +545,15 @@ def test_{{ method_name }}(request_type, transport: str = 'grpc'):
545545
{% if not field.oneof or field.proto3_optional %}
546546
{{ field.name }}={{ field.mock_value }},
547547
{% endif %}{% endfor %}
548-
{# This is a hack to only pick one field #}
548+
{# This is a hack to only pick one field #}
549549
{% for oneof_fields in method.output.oneof_fields().values() %}
550-
{% with field = oneof_fields[0] %}
550+
{# 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 #}
551+
{# Use an outer if-statement here because `first` raises an error if called on an empty sequence #}
552+
{% if (oneof_fields | rejectattr('message') | list) %}
553+
{% with field = (oneof_fields | rejectattr('message') | first) %}
551554
{{ field.name }}={{ field.mock_value }},
552555
{% endwith %}
556+
{% endif %}
553557
{% endfor %}
554558
)
555559
{% endif %}
@@ -959,11 +963,13 @@ def test_{{ method_name }}_rest(request_type, transport: str = 'rest'):
959963
{% if not field.oneof or field.proto3_optional %}
960964
{{ field.name }}={{ field.mock_value }},
961965
{% endif %}{% endfor %}
962-
{# This is a hack to only pick one field #}
966+
{# This is a hack to only pick one field #}
963967
{% for oneof_fields in method.output.oneof_fields().values() %}
964-
{% with field = oneof_fields[0] %}
968+
{% if (oneof_fields | rejectattr('message') | list) %}
969+
{% with field = (oneof_fields | rejectattr('message') | first) %}
965970
{{ field.name }}={{ field.mock_value }},
966971
{% endwith %}
972+
{% endif %}
967973
{% endfor %}
968974
)
969975
{% endif %}
@@ -1029,11 +1035,13 @@ def test_{{ method.name|snake_case }}_rest(request_type):
10291035
{% if not field.oneof or field.proto3_optional %}
10301036
{{ field.name }}={{ field.mock_value }},
10311037
{% endif %}{% endfor %}
1032-
{# This is a hack to only pick one field #}
1038+
{# This is a hack to only pick one field #}
10331039
{% for oneof_fields in method.output.oneof_fields().values() %}
1034-
{% with field = oneof_fields[0] %}
1040+
{% if (oneof_fields | rejectattr('message') | list) %}
1041+
{% with field = (oneof_fields | rejectattr('message') | first) %}
10351042
{{ field.name }}={{ field.mock_value }},
10361043
{% endwith %}
1044+
{% endif %}
10371045
{% endfor %}
10381046
)
10391047
{% endif %}

‎packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,15 @@ def test_{{ method_name }}(request_type, transport: str = 'grpc'):
3434
{% if not field.oneof or field.proto3_optional %}
3535
{{ field.name }}={{ field.mock_value }},
3636
{% endif %}{% endfor %}
37-
{# This is a hack to only pick one field #}
37+
{# This is a hack to only pick one field #}
3838
{% for oneof_fields in method.output.oneof_fields().values() %}
39-
{% with field = oneof_fields[0] %}
39+
{# 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 #}
40+
{# Use an outer if-statement here because `first` raises an error if called on an empty sequence #}
41+
{% if (oneof_fields | rejectattr('message') | list) %}
42+
{% with field = (oneof_fields | rejectattr('message') | first) %}
4043
{{ field.name }}={{ field.mock_value }},
4144
{% endwith %}
45+
{% endif %}
4246
{% endfor %}
4347
)
4448
{% endif %}
@@ -903,11 +907,13 @@ def test_{{ method_name }}_rest(request_type):
903907
{% if not field.oneof or field.proto3_optional %}
904908
{{ field.name }}={{ field.mock_value }},
905909
{% endif %}{% endfor %}
906-
{# This is a hack to only pick one field #}
907-
{% for oneof_fields in method.extended_lro.operation_type.oneof_fields().values() %}
908-
{% with field = oneof_fields[0] %}
909-
{{ field.name }}={{ field.mock_value }},
910-
{% endwith %}
910+
{# This is a hack to only pick one field #}
911+
{% for oneof_fields in method.output.oneof_fields().values() %}
912+
{% if (oneof_fields | rejectattr('message') | list) %}
913+
{% with field = (oneof_fields | rejectattr('message') | first) %}
914+
{{ field.name }}={{ field.mock_value }},
915+
{% endwith %}
916+
{% endif %}
911917
{% endfor %}
912918
)
913919
{% else %}
@@ -916,12 +922,14 @@ def test_{{ method_name }}_rest(request_type):
916922
{% if not field.oneof or field.proto3_optional %}
917923
{{ field.name }}={{ field.mock_value }},
918924
{% endif %}{% endfor %}
919-
{# This is a hack to only pick one field #}
920-
{% for oneof_fields in method.output.oneof_fields().values() %}
921-
{% with field = oneof_fields[0] %}
922-
{{ field.name }}={{ field.mock_value }},
923-
{% endwith %}
924-
{% endfor %}
925+
{# This is a hack to only pick one field #}
926+
{% for oneof_fields in method.output.oneof_fields().values() %}
927+
{% if (oneof_fields | rejectattr('message') | list) %}
928+
{% with field = (oneof_fields | rejectattr('message') | first) %}
929+
{{ field.name }}={{ field.mock_value }},
930+
{% endwith %}
931+
{% endif %}
932+
{% endfor %}
925933
)
926934
{% endif %}
927935

‎packages/gapic-generator/tests/fragments/test_oneof_imported_response.proto‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright (C) 2021 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
syntax = "proto3";
16+
17+
package google.fragment;
18+
19+
import "google/api/client.proto";
20+
import "import.proto";
21+
22+
service MyService {
23+
option (google.api.default_host) = "my.example.com";
24+
rpc MyMethod(MethodRequest) returns (MethodResponse) {}
25+
}
26+
27+
message MethodRequest {
28+
string input = 1;
29+
}
30+
31+
message Container {
32+
Import import = 1;
33+
}
34+
35+
message MethodResponse {
36+
string parent = 1;
37+
oneof format {
38+
Container container = 2;
39+
string name = 3;
40+
}
41+
}

‎packages/gapic-generator/tests/integration/goldens/logging/tests/unit/gapic/logging_v2/test_config_service_v2.py‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3248,7 +3248,6 @@ def test_get_sink(request_type, transport: str = 'grpc'):
32483248
output_version_format=logging_config.LogSink.VersionFormat.V2,
32493249
writer_identity='writer_identity_value',
32503250
include_children=True,
3251-
bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True),
32523251
)
32533252
response = client.get_sink(request)
32543253

@@ -3509,7 +3508,6 @@ def test_create_sink(request_type, transport: str = 'grpc'):
35093508
output_version_format=logging_config.LogSink.VersionFormat.V2,
35103509
writer_identity='writer_identity_value',
35113510
include_children=True,
3512-
bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True),
35133511
)
35143512
response = client.create_sink(request)
35153513

@@ -3780,7 +3778,6 @@ def test_update_sink(request_type, transport: str = 'grpc'):
37803778
output_version_format=logging_config.LogSink.VersionFormat.V2,
37813779
writer_identity='writer_identity_value',
37823780
include_children=True,
3783-
bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True),
37843781
)
37853782
response = client.update_sink(request)
37863783

@@ -8546,7 +8543,6 @@ def test_get_sink_rest(request_type):
85468543
output_version_format=logging_config.LogSink.VersionFormat.V2,
85478544
writer_identity='writer_identity_value',
85488545
include_children=True,
8549-
bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True),
85508546
)
85518547

85528548
# Wrap the value into a proper Response obj
@@ -8794,7 +8790,6 @@ def test_create_sink_rest(request_type):
87948790
output_version_format=logging_config.LogSink.VersionFormat.V2,
87958791
writer_identity='writer_identity_value',
87968792
include_children=True,
8797-
bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True),
87988793
)
87998794

88008795
# Wrap the value into a proper Response obj
@@ -9048,7 +9043,6 @@ def test_update_sink_rest(request_type):
90489043
output_version_format=logging_config.LogSink.VersionFormat.V2,
90499044
writer_identity='writer_identity_value',
90509045
include_children=True,
9051-
bigquery_options=logging_config.BigQueryOptions(use_partitioned_tables=True),
90529046
)
90539047

90549048
# Wrap the value into a proper Response obj

0 commit comments

Comments
 (0)