fix(tests/spanner): Fix swallowing of spanner integration test failures#2908
fix(tests/spanner): Fix swallowing of spanner integration test failures#2908anubhav756 wants to merge 1 commit intomainfrom
Conversation
![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 updates the Spanner integration test step in the Cloud Build configuration to explicitly exit with a non-zero status on failure. Feedback indicates that this explicit error handling is redundant as Cloud Build naturally inherits the exit code of the final command, and suggests reverting to a simpler command for consistency with other test steps.
| "Spanner" \ | ||
| spanner \ | ||
| spanner || echo "Integration tests failed." | ||
| spanner || { echo "Integration tests failed."; exit 1; } |
There was a problem hiding this comment.
The explicit error handling || { echo ...; exit 1; } is redundant here. In a Cloud Build args block using bash -c, the exit code of the shell is determined by the last command executed in the script. Since this test command is the final operation in its branch of the if statement, the step will naturally fail if the tests fail. Removing this logic would make the Spanner step consistent with all other integration test steps in this file (e.g., cloud-sql-pg, alloydb, bigquery, etc.), which simply call the test script directly.
spanner
This PR fixes an issue with the CI/CD setup which caused failures from spanner integration tests to be swallowed, meaning the errors were just logged and the test ran green. This PR fixes that by updating the command needed to run the spanner test.
anubhav756
force-pushed
the
anubhav-spanner-swallow
branch
from
bb7fc39 to
46219d3
Compare
Yuan325
added
the
do not merge
|
needs to wait till error is fixed before merging this |
Thanks for the label! Also for ref, here's the PR for strengthening cleanup of spanner graphs and tables so this issue doesn't occur going forward: #2904 |
2 participants
Description
This PR resolves an issue in our CI/CD where Spanner integration test failures were being incorrectly suppressed. Previously, test errors were only logged while the pipeline continued to report a passing status.
This is fixed by updating the execution command for the Spanner tests to ensure failure exit codes are properly caught by the runner.
Note
Because this PR successfully exposes previously hidden failures, the Spanner integration tests will currently fail on this branch due to a pre-existing issue. Fixing the underlying Spanner test is out of scope for this PR and will be handled separately.