This repository was archived by the owner on Mar 26, 2026. It is now read-only.
chore(testproxy): Mutate rows bug fix#1234
Merged
danieljbruce merged 9 commits intogoogleapis:mainfrom Feb 16, 2023
Merged
Conversation
Changed the test proxy functionality to match what is in the veneer layer and now the test case in question passes with a slight change in the grpc code returned.
This commit just permits a more concise expression for the entries that should be returned back to the test runner.
product-auto-label
Bot
added
size: m
ruyadorno
reviewed
Feb 10, 2023
Comment on lines
+40
to
+47
| if (error.name === 'PartialFailureError') { | ||
| return { | ||
| status: {code: grpc.status.OK, message: error.message}, | ||
| entry: Array.from(error.errors.entries()).map(([index, entry]) => ({ | ||
| index: index + 1, | ||
| status: entry, | ||
| })), | ||
| }; |
Contributor
There was a problem hiding this comment.
why is the proxy turning partial failures into an "OK" response here?
Contributor
Author
There was a problem hiding this comment.
It meets the needs of the test case and is required for the test case to pass, but this is a good start to a discussion about what the test proxy should do.
Contributor
Author
There was a problem hiding this comment.
Maybe
Line 1540 in 000a353
Contributor
There was a problem hiding this comment.
We shouldn't change the node js client.
You can return the status as is, just ensure the entries are filled correctly. I will update the test case as well as the guide of test proxy writeup.
The test proxy uses a new test path now so these adjustments must be made to work with the test runner properly.
This reverts commit efbdadb.
As per the online discussion, we should simply pass along the error as is so that the corresponding error message and status can be reported.
danieljbruce
added
the
owlbot:run
gcf-owl-bot
Bot
removed
the
owlbot:run
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes a change to the test proxy that allows the test proxy to use the veneer layer instead to make its call to mutateRows. Using the veneer layer to make the call allows additional parsing to be done on the request and the results which ensures the right results are returned to the test runner enabling the test runner to pass its test.
The test in question is TestMutateRows_NoRetry_NonTransientErrors.