chore: migrate samples to google-cloud-python system test infrastructure#2768
Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the samples directory to tests/system and updates the corresponding Kokoro configuration files, test scripts, Makefile, and other tooling to reference the new path. The review feedback highlights a critical issue where periodic builds rewinding to older releases will fail because tests/system does not exist yet; a dynamic fallback to samples is suggested. Additionally, the reviewer recommends updating TRAMPOLINE_BUILD_FILE in periodic-head.cfg files to reflect the renamed script, and adding safety checks to ensure KOKORO_GFILE_DIR is non-empty before checking for files within it.
| # Exit early if samples don't exist | ||
| if ! find tests/system -name 'requirements.txt' | grep -q .; then | ||
| echo "No tests run. './tests/system/**/requirements.txt' not found" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
During periodic builds, the repository is checked out at the LATEST_RELEASE tag (which was created before this migration). At that tag, the tests/system directory does not exist, but samples does. As a result, find tests/system will fail, and the script will silently exit with 0 without running any tests.
To prevent this, we should dynamically fall back to samples if tests/system is not present.
| # Exit early if samples don't exist | |
| if ! find tests/system -name 'requirements.txt' | grep -q .; then | |
| echo "No tests run. './tests/system/**/requirements.txt' not found" | |
| exit 0 | |
| fi | |
| # Exit early if tests don't exist | |
| TEST_DIR="tests/system" | |
| if [ ! -d "$TEST_DIR" ] && [ -d "samples" ]; then | |
| TEST_DIR="samples" | |
| fi | |
| if ! find "$TEST_DIR" -name 'requirements.txt' | grep -q .; then | |
| echo "No tests run. './$TEST_DIR/**/requirements.txt' not found" | |
| exit 0 | |
| fi |
| set +e | ||
| RTN=0 | ||
| ROOT=$(pwd) | ||
| for file in tests/system/**/requirements.txt; do |
| shopt -s globstar | ||
|
|
||
| exec .kokoro/test-samples-impl.sh | ||
| exec .kokoro/test-system-impl.sh |
There was a problem hiding this comment.
Since .kokoro/test-samples-against-head.sh has been renamed to .kokoro/test-system-against-head.sh, please make sure to update the TRAMPOLINE_BUILD_FILE environment variable in all periodic-head.cfg files (e.g., .kokoro/system/python3.10/periodic-head.cfg, etc.) to point to the new filename. Currently, they still reference the old .kokoro/test-samples-against-head.sh path, which will cause those builds to fail.
| if [[ -f "${KOKORO_GFILE_DIR}/project-id.json" ]]; then | ||
| export PROJECT_ID=$(cat "${KOKORO_GFILE_DIR}/project-id.json") | ||
| export GOOGLE_CLOUD_PROJECT="${PROJECT_ID}" | ||
| gcloud config set project "$PROJECT_ID" | ||
| fi |
There was a problem hiding this comment.
It is safer to ensure KOKORO_GFILE_DIR is non-empty before checking for the existence of project-id.json to avoid checking the root directory /project-id.json if the variable is unset.
| if [[ -f "${KOKORO_GFILE_DIR}/project-id.json" ]]; then | |
| export PROJECT_ID=$(cat "${KOKORO_GFILE_DIR}/project-id.json") | |
| export GOOGLE_CLOUD_PROJECT="${PROJECT_ID}" | |
| gcloud config set project "$PROJECT_ID" | |
| fi | |
| if [[ -n "${KOKORO_GFILE_DIR}" && -f "${KOKORO_GFILE_DIR}/project-id.json" ]]; then | |
| export PROJECT_ID=$(cat "${KOKORO_GFILE_DIR}/project-id.json") | |
| export GOOGLE_CLOUD_PROJECT="${PROJECT_ID}" | |
| gcloud config set project "$PROJECT_ID" | |
| fi |
| if [[ -f "${KOKORO_GFILE_DIR}/service-account.json" ]]; then | ||
| export GOOGLE_APPLICATION_CREDENTIALS="${KOKORO_GFILE_DIR}/service-account.json" | ||
| gcloud auth activate-service-account --key-file="${GOOGLE_APPLICATION_CREDENTIALS}" | ||
| fi |
There was a problem hiding this comment.
It is safer to ensure KOKORO_GFILE_DIR is non-empty before checking for the existence of service-account.json to avoid checking the root directory /service-account.json if the variable is unset.
| if [[ -f "${KOKORO_GFILE_DIR}/service-account.json" ]]; then | |
| export GOOGLE_APPLICATION_CREDENTIALS="${KOKORO_GFILE_DIR}/service-account.json" | |
| gcloud auth activate-service-account --key-file="${GOOGLE_APPLICATION_CREDENTIALS}" | |
| fi | |
| if [[ -n "${KOKORO_GFILE_DIR}" && -f "${KOKORO_GFILE_DIR}/service-account.json" ]]; then | |
| export GOOGLE_APPLICATION_CREDENTIALS="${KOKORO_GFILE_DIR}/service-account.json" | |
| gcloud auth activate-service-account --key-file="${GOOGLE_APPLICATION_CREDENTIALS}" | |
| fi |
This PR migrates the sample testing infrastructure away from the broken/legacy python-docs-samples secrets setup. It adopts the standard python-multi docker image and trampoline.sh execution flow used by the rest of google-cloud-python. Additionally, it permanently drops the .kokoro configurations and required GitHub status checks for the unsupported Python 3.7, 3.8, and 3.9 environments.
Fixes: #2767