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: handle message bodies #1117

Merged
merged 2 commits into from
Dec 30, 2021

Conversation

software-dov
Copy link
Contributor

Some methods with http annotations have body fields that are message types.
Previously, generated unit tests did not handle this well.

This is a fix for that: generate a reasonable mock value for the
message, represented as a dict.

Some methods with http annotations have body fields that are message types.
Previously, generated unit tests did not handle this well.

This is a fix for that: generate a reasonable mock value for the
message, represented as a dict.
Copy link
Contributor

@vam-google vam-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just one question and please fix the tests before pushing.

@@ -0,0 +1,53 @@
// Copyright (C) 2021 Google LLC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file used? If yes, can you please explain how (I could not find any referrences to it from other parts of this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used in the fragment tests. For each proto in this directory, we generate a GAPIC library and run its generated unit tests. This stresses both the templates and the actual logic of the generated surface. See noxfile.py:fragment for more detail on how the fragment tests work.
This particular fragment is the minimum required reproduction for the issues that compute was having.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants