tests(Bigtable): Fix conformance test failures - #15932
amanda-tarafa merged 2 commits into
Conversation
See b/563376582.
product-auto-label
Bot
added
the
api: bigtable
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the conformance test proxy implementation and its execution script. In CloudBigtableV2TestProxyImpl.cs, the BulkMutateRows method is updated to properly filter failed entries and set the overall status based on the first failure. In bigtable-conformance.sh, the script is cleaned up to run the proxy in the background and clean it up on exit. Feedback includes adding a null check for entry.Status to prevent a potential NullReferenceException and avoiding the redirection of standard error to /dev/null in the script's cleanup trap.
492cba3 to
f83bc9e
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the BulkMutateRows implementation in the C# conformance test proxy to correctly filter and report failed entries, and refactors the bigtable-conformance.sh script to improve process management and run previously skipped tests. Feedback on the changes highlights a potential NullReferenceException when accessing entry.Status and suggests cloning the status object to avoid shared mutable state.
|
This is ready for review, one commit at a time. Fixes b/563376582. |
efevans
left a comment
efevans
left a comment
There was a problem hiding this comment.
Generally LGTM, was the root issue that the request used to throw and exception but that changed due to an upstream?
Nope, that these two tests that failed are new (from September 18) and are the only ones that are checking the error code. I just pasted the whole explanation on the internal issue. |
2 participants
No description provided.