Test parallel execute system tests#17446
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Kokoro system test script to run package tests in parallel using xargs and redirects their outputs to log files, while also touching setup.py files across multiple packages to trigger the tests. Feedback on these changes highlights a potential bug where RETVAL is not initialized, which could cause the script to incorrectly exit with a failure status when all tests pass. Additionally, running tests in parallel with redirected output could lead to CI inactivity timeouts; printing start and completion messages for each package is recommended to maintain console activity.
| if [ -n "$PACKAGES_TO_TEST" ]; then | ||
| mkdir -p .logs |
There was a problem hiding this comment.
Since the direct execution of run_package_test inside the loop was removed, RETVAL is no longer set during the package iteration. If RETVAL is not initialized to 0 at the beginning of the script, and all tests pass, exit ${RETVAL} will exit with the status of the last executed command.
In this new implementation, the last executed command is [ -f "$failed" ] (which evaluates to false and returns 1 when there are no failures). This would cause the script to incorrectly exit with a failure status (1) even when all tests pass.
To prevent this, we should explicitly initialize RETVAL=0.
| if [ -n "$PACKAGES_TO_TEST" ]; then | |
| mkdir -p .logs | |
| if [ -n "$PACKAGES_TO_TEST" ]; then | |
| RETVAL=0 | |
| mkdir -p .logs |
| export -f run_package_test | ||
| export system_test_script PROJECT_ROOT KOKORO_GFILE_DIR | ||
|
|
||
| echo "$PACKAGES_TO_TEST" | xargs -n 1 -P 8 -I {} bash -c 'run_package_test "{}" > ".logs/{}.log" 2>&1 || touch ".logs/{}.failed"' |
There was a problem hiding this comment.
Running all system tests in parallel with all output redirected to log files means there will be absolutely no output printed to the console while the tests are running. In CI environments (like Kokoro), this can trigger inactivity timeouts if the tests run for a long time without producing stdout/stderr.
Additionally, it makes it difficult to monitor progress or know which packages are currently being tested.
We can print a start and completion message for each package to keep the CI connection alive and provide visibility into the progress.
| echo "$PACKAGES_TO_TEST" | xargs -n 1 -P 8 -I {} bash -c 'run_package_test "{}" > ".logs/{}.log" 2>&1 || touch ".logs/{}.failed"' | |
| echo "$PACKAGES_TO_TEST" | xargs -n 1 -P 8 -I {} bash -c 'echo "Starting {}..."; run_package_test "{}" > ".logs/{}.log" 2>&1 && echo "SUCCESS: {}" || { echo "FAILED: {}"; touch ".logs/{}.failed"; }' |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕