tests(google-cloud-compute): subtle fix to compute system test test_zero_values - #18089
Conversation
There was a problem hiding this comment.
Code Review
This pull request simplifies the exception message assertion in the test_zero_values smoke test by removing the conditional check for Protobuf version 3. Feedback indicates that removing this check will cause test failures in environments running Protobuf v3, and suggests keeping the conditional check while updating the fallback string to match the substring instead of the exact tuple representation.
| "bad argument type for built-in operation" | ||
| ), |
There was a problem hiding this comment.
Removing the check for PROTOBUF_VERSION[0] == "3" will cause this test to fail in environments running Protobuf v3, where the exception message is "0 has type int, but expected one of: bytes, unicode" and does not contain "bad argument type for built-in operation".
We should keep the conditional check but update the else branch to look for the substring instead of the exact tuple representation.
"0 has type int, but expected one of: bytes, unicode"
if PROTOBUF_VERSION[0] == "3"
else "bad argument type for built-in operation"
),There was a problem hiding this comment.
All libraries in this repository require Protobuf 6.x
test_zero_valuesis failing in newer environments becauseproto-plusnow wraps underlyingTypeErrorsin a more descriptive error message(e.g., "Failed to set field...")following #17682. This broke our rigid assertion which was looking for the exact raw tuple representation('bad argument type for built-in operation',).This PR updates the assertion to look for the core error substring instead, making the test robust to exception wrapping.