test: run the bigquery system tests in google cloud build#8690
test: run the bigquery system tests in google cloud build#8690danieljbruce wants to merge 7 commits into
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 introduces a new Cloud Build configuration (cloudbuild.yaml) to automate dependency installation, system testing, and coverage reporting for the handwritten/bigquery Node.js project. The reviewer provided valuable feedback to improve the build script, including using the standard binary path for nyc to avoid brittleness, cleaning up verbose placeholder comments and mock echo statements, and reducing the exceptionally long 3-hour timeout to a safer 1-hour limit to prevent excessive resource consumption in case of hanging tests.
| if [ -f ./node_modules/nyc/bin/nyc.js ]; then | ||
| ./node_modules/nyc/bin/nyc.js report || true # `|| true` prevents build failure if nyc report itself exits non-zero |
There was a problem hiding this comment.
Using the internal path ./node_modules/nyc/bin/nyc.js is brittle as the internal structure of the package can change between versions. It is safer and more standard to reference the binary in ./node_modules/.bin/nyc.
if [ -f ./node_modules/.bin/nyc ]; then
./node_modules/.bin/nyc report || true # || true prevents build failure if nyc report itself exits non-zeroThere was a problem hiding this comment.
I think it is better if we just stay in alignment with what it was for the kokoro tests. Less likely we get an unexpected surprise that way.
| # The original codecov.sh script from Kokoro needs to be made available to GCB. | ||
| # Options: | ||
| # a) Commit codecov.sh into your repo (e.g., .kokoro/codecov.sh) and call it: | ||
| # if [ -f .kokoro/codecov.sh ]; then . ./.kokoro/codecov.sh; fi | ||
| # b) Replicate its functionality directly in this step. | ||
| # c) Store it in a GCS bucket and fetch it. | ||
| echo "Codecov reporting (if desired) would be integrated here." |
There was a problem hiding this comment.
These verbose placeholder comments and the mock echo statement should be cleaned up for production. Replacing them with a simple TODO comment keeps the configuration file clean and maintainable.
# TODO: Integrate Codecov reporting here if desired.There was a problem hiding this comment.
I think it is best if we just stay in alignment with the template for consistency
| substitutions: | ||
| _GCP_PROJECT_ID: 'long-door-651' | ||
|
|
||
| timeout: '10800s' |
There was a problem hiding this comment.
The timeout is set to 10800s (3 hours), which is exceptionally long for standard system tests. If a test hangs or enters an infinite loop, this can lead to high resource consumption and costs. Consider reducing this to a more reasonable limit, such as 3600s (1 hour).
timeout: '3600s'There was a problem hiding this comment.
This change was made.
danieljbruce
changed the title
1 participant
Description
This pull request adds a yaml file to instruct the Bigquery system test CI check to work with the new Cloud Build trigger thereby making the new CI check effectively run our system tests. It turns out that no extra code modifications are required to allow the test to run in the new environment.
Impact
Leverages the strengths of running system tests in GCB rather than relying on kokoro for system tests.
Testing
This pull request tells the tests how to work with the new check to the continuous integration pipeline for the Bigquery system tests thus improves the effectiveness of that test.
Next Steps