Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve unit test failure caused by differences in protobuf runtimes #1749

Merged
merged 35 commits into from
Oct 7, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
5e81e33
fix: resolve unit test failure
parthea Aug 29, 2023
053a39d
update comment
parthea Aug 30, 2023
639fe5e
add comment
parthea Aug 30, 2023
e067764
Only use the constraints file for standard templates
parthea Aug 30, 2023
8a374e6
fix generated ads tests similar to https://github.com/googleapis/gapi…
parthea Aug 30, 2023
77da39a
workaround issue in test where protobuf runtime version may be older
parthea Aug 30, 2023
f49a32c
update goldens
parthea Aug 30, 2023
538710f
address offline review feedback
parthea Aug 31, 2023
d9b85fc
update goldens
parthea Aug 31, 2023
b405a20
make the solution generic
parthea Sep 12, 2023
1dea577
fix grammar
parthea Sep 14, 2023
80eb916
Merge branch 'main' into add-fragment-test-service-config
parthea Sep 15, 2023
6ec252e
update comment
parthea Sep 19, 2023
9c5c4a8
typo
parthea Sep 19, 2023
7073342
add README.rst in tests/fragments/google
parthea Sep 20, 2023
ea94110
update goldens
parthea Sep 20, 2023
f0e3a98
add comment to setup.py.j2 about excluded protobuf versions
parthea Sep 20, 2023
b52bfc5
address review feedback
parthea Sep 20, 2023
1d7f628
update comment
parthea Sep 26, 2023
dc7934d
Address review feedback
parthea Oct 2, 2023
f3b4d8e
add lightweight fragment test
parthea Oct 5, 2023
0384903
remove heavyweight test
parthea Oct 5, 2023
83fccab
Merge branch 'main' into add-fragment-test-service-config
parthea Oct 5, 2023
bcf9652
update test to cater for proto-plus message types
parthea Oct 6, 2023
b3d6dfa
cater for subsequent items in repeated fields
parthea Oct 6, 2023
ed64d30
remove new line
parthea Oct 6, 2023
ff55725
revert WORKSPACE
parthea Oct 6, 2023
91404e6
Update comment
parthea Oct 6, 2023
f9be1b4
update comment
parthea Oct 6, 2023
b7fc321
address review feedback
parthea Oct 6, 2023
402fb17
address review feedback
parthea Oct 6, 2023
ae23824
update comment
parthea Oct 6, 2023
350006c
address review feedback
parthea Oct 7, 2023
e689b13
update test in ads-templates
parthea Oct 7, 2023
2a7f516
add comment
parthea Oct 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1048,15 +1048,23 @@ def test_{{ method.name|snake_case }}_rest(request_type):
# Wrap the value into a proper Response obj
response_value = Response()
response_value.status_code = 200
{% if method.void %}
{% if method.void %}
json_return_value = ''
{% elif method.lro %}
json_return_value = json_format.MessageToJson(return_value)
{% elif method.server_streaming %}
json_return_value = "[{}]".format({{ method.output.ident }}.to_json(return_value))
{% else %}
json_return_value = {{ method.output.ident }}.to_json(return_value)
{% endif %}
{% else %}

{% if method.output.ident.is_proto_plus_type %}
pb_return_value = {{ method.output.ident }}.pb(return_value)
{% else %}
pb_return_value = return_value
parthea marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
json_return_value = json_format.MessageToJson(pb_return_value)
{% endif %}
{% if method.server_streaming %}
json_return_value = "[{}]".format(json_return_value)
{% endif %}

response_value._content = json_return_value.encode('UTF-8')
req.return_value = response_value
{% if method.client_streaming %}
Expand Down Expand Up @@ -1391,16 +1399,22 @@ def test_{{ method.name|snake_case }}_rest_flattened():
# Wrap the value into a proper Response obj
response_value = Response()
response_value.status_code = 200
{% if method.void %}
{% if method.void %}
json_return_value = ''
{% elif method.lro %}
json_return_value = json_format.MessageToJson(return_value)
{% elif method.server_streaming %}
json_return_value = "[{}]".format({{ method.output.ident }}.to_json(return_value))
{% else %}
json_return_value = {{ method.output.ident }}.to_json(return_value)
{% endif %}
{% else %}

{% if method.output.ident.is_proto_plus_type %}
pb_return_value = {{ method.output.ident }}.pb(return_value)
{% else %}
pb_return_value = return_value
{% endif %}
json_return_value = json_format.MessageToJson(pb_return_value)
{% endif %}
{% if method.server_streaming %}
json_return_value = "[{}]".format(json_return_value)
{% endif %}
response_value._content = json_return_value.encode('UTF-8')
req.return_value = response_value

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,50 @@ def test_{{ method_name }}_rest(request_type):
{% if not field.oneof or field.proto3_optional %}
{# ignore oneof fields that might conflict with sample_request #}
request_init["{{ field.name }}"] = {{ field.merged_mock_value(method.http_options[0].sample_request(method).get(field.name)) }}
# The version of a generated dependency at runtime may differ compared to the one at time of generation
parthea marked this conversation as resolved.
Show resolved Hide resolved
# Delete any keys which are not present in the current runtime dependency
# See https://github.com/googleapis/gapic-generator-python/issues/1748
if hasattr({{ method.input.ident }}.meta.fields["{{ field.name }}"].message, "DESCRIPTOR"):
keys_to_delete = []

# Get all subfields for the message
subfield_names = [
vchudnov-g marked this conversation as resolved.
Show resolved Hide resolved
(field.name, subfield.name)
for field in {{ method.input.ident }}.meta.fields["{{ field.name }}"].message.DESCRIPTOR.fields
if field.message_type
for subfield in field.message_type.fields
]

# For each item in the sample request, create a list of sub fields which are not present at runtime
for key, value in request_init["{{ field.name }}"].items():
result = None
is_repeated = False
# For repeated fields
if isinstance(value, list) and len(value):
is_repeated = True
result = value[0]
# For fields where the type is another message
if isinstance(value, dict):
result = value

if result:
for nested_key in result.keys():
if (key, nested_key) not in subfield_names:
keys_to_delete.append(
{"key": key, "nested_key": nested_key, "is_repeated": is_repeated}
)

# Remove fields from the sample request which are not present in the runtime version of the dependency
for key_to_delete in keys_to_delete:
if key_to_delete.get("nested_key"):
if key_to_delete.get("is_repeated"):
del request_init["{{ field.name }}"][key_to_delete.get("key")][0][
key_to_delete.get("nested_key")
]
else:
del request_init["{{ field.name }}"][key_to_delete.get("key")][
key_to_delete.get("nested_key")
]
{% endif %}
{% endfor %}
request = request_type(**request_init)
Expand Down Expand Up @@ -1242,12 +1286,6 @@ def test_{{ method_name }}_rest_bad_request(transport: str = 'rest', request_typ

# send a request that will satisfy transcoding
request_init = {{ method.http_options[0].sample_request(method) }}
{% for field in method.body_fields.values() %}
{% if not field.oneof or field.proto3_optional %}
{# ignore oneof fields that might conflict with sample_request #}
request_init["{{ field.name }}"] = {{ field.merged_mock_value(method.http_options[0].sample_request(method).get(field.name)) }}
parthea marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% endfor %}
request = request_type(**request_init)
{% if method.client_streaming %}
requests = [request]
Expand Down
17 changes: 14 additions & 3 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,20 @@ def __call__(self, frag):
)

# Install the generated fragment library.
# Note: install into the tempdir to prevent issues
# with running pip concurrently.
self.session.install(tmp_dir, "-e", ".", "-t", tmp_dir, "-qqq")
if self.use_ads_templates:
self.session.install(tmp_dir, "-e", ".", "-qqq")
else:
# Use the constraints file for the specific python runtime version
parthea marked this conversation as resolved.
Show resolved Hide resolved
parthea marked this conversation as resolved.
Show resolved Hide resolved
# We do this to make sure that we're testing against the lowest
# supported version of a dependency.
# This is needed to recreate the issue reported in
# https://github.com/googleapis/gapic-generator-python/issues/1748
# The ads templates do not have constraints files.
constraints_path = str(
f"{tmp_dir}/testing/constraints-{self.session.python}.txt"
)
self.session.install(tmp_dir, "-e", ".", "-qqq", "-r", constraints_path)

# Run the fragment's generated unit tests.
# Don't bother parallelizing them: we already parallelize
# the fragments, and there usually aren't too many tests per fragment.
Expand Down
6 changes: 4 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@
version = "1.11.5"
release_status = "Development Status :: 5 - Production/Stable"
dependencies = [
# Esnure that the lower bounds of these dependencies match what we have in the
parthea marked this conversation as resolved.
Show resolved Hide resolved
vchudnov-g marked this conversation as resolved.
Show resolved Hide resolved
# templated setup.py.j2 here https://github.com/googleapis/gapic-generator-python/blob/main/gapic/templates/setup.py.j2
parthea marked this conversation as resolved.
Show resolved Hide resolved
"click >= 6.7",
"google-api-core >= 2.8.0",
"google-api-core[grpc] >= 1.34.0, <3.0.0dev,!=2.0.*,!=2.1.*,!=2.2.*,!=2.3.*,!=2.4.*,!=2.5.*,!=2.6.*,!=2.7.*,!=2.8.*,!=2.9.*,!=2.10.*",
"googleapis-common-protos >= 1.55.0",
"grpcio >= 1.24.3",
"jinja2 >= 2.10",
"protobuf >= 3.18.0, < 5.0.0dev",
"protobuf>=3.19.5,<5.0.0dev,!=3.20.0,!=3.20.1,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5",
parthea marked this conversation as resolved.
Show resolved Hide resolved
vchudnov-g marked this conversation as resolved.
Show resolved Hide resolved
"pypandoc >= 1.4",
"PyYAML >= 5.1.1",
"grpc-google-iam-v1 >= 0.12.4, < 1.0.0dev",
Expand Down
Loading