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: preserve default values in x-goog-request-params #1250

Merged
merged 5 commits into from
Sep 8, 2022

Conversation

alexander-fenster
Copy link
Contributor

@alexander-fenster alexander-fenster commented Sep 8, 2022

Note that this PR is against update-showcase-baseline branch, which, when merged, will make it green since the updated Showcase Compliance will now produce a valid library.

The three changes in this PR across all the baseline tests are:

  1. In the generated client, in the place where we build x-goog-request-params header, use ?? instead of ||. The difference is small: if a value is 0, it's still defined, so should not be overridden by ''. It suddenly becomes important when x-goog-request-params contains values that are not strings.

  2. In tests, again, because some values to be placed in x-goog-request-params are not strings in compliance.proto, the whole idea of type-appropriate default value (to fill the fake request) appears, using some protobuf.js reflection type magic.

  3. The new echo.proto shows how complicated the routing header settings could be, so I needed to simplify the generated unit tests to make it still check the logic but not becoming the change detector test.

@alexander-fenster alexander-fenster merged commit a8fae19 into update-showcase-baseline Sep 8, 2022
@alexander-fenster alexander-fenster deleted the initialize-types branch September 8, 2022 21:38
alexander-fenster added a commit that referenced this pull request Sep 8, 2022
* test: update Showcase baseline to the latest version 0.25.0

* fix: preserve default values in x-goog-request-params (#1250)

* fix: preserve default values in x-goog-request-params header

* test: updated baselines for compliance

* fix: do not change logic for legacy proto load

* test: generate simpler test for routing headers

* fix: make unit tests actually check headers
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