[DRAFT] chore: improve system tests - #18206
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Code Review
This pull request increases the Kokoro build timeout to 6 hours and introduces a per-package timeout mechanism (defaulting to 35 minutes) using the timeout command in .kokoro/system.sh. It also adds tracking and reporting for timed-out packages, and triggers system tests across multiple packages. The reviewer feedback recommends declaring the pkg and timed_out variables as local at the top of the reap_parallel_results function in .kokoro/system.sh to prevent global namespace pollution and duplicate declarations.
Sorry, something went wrong.
| local retval=0 | ||
| local failed_count=0 | ||
| local timed_out_count=0 | ||
| local succeeded_count=0 |
There was a problem hiding this comment.
To prevent global namespace pollution and avoid duplicate local declarations, declare pkg and timed_out as local variables at the top of the reap_parallel_results function.
| local retval=0 | |
| local failed_count=0 | |
| local timed_out_count=0 | |
| local succeeded_count=0 | |
| local retval=0 | |
| local failed_count=0 | |
| local timed_out_count=0 | |
| local succeeded_count=0 | |
| local pkg | |
| local timed_out |
Sorry, something went wrong.
| for failed in "$LOG_DIR"/*.failed; do | ||
| if [ -f "$failed" ]; then | ||
| basename "$failed" .failed | ||
| local pkg=$(basename "$failed" .failed) |
There was a problem hiding this comment.
Remove the local keyword here since pkg is now declared at the top of the function.
| local pkg=$(basename "$failed" .failed) | |
| pkg=$(basename "$failed" .failed) |
Sorry, something went wrong.
| done | ||
| for failed in "$LOG_DIR"/*.failed; do | ||
| if [ -f "$failed" ]; then | ||
| local pkg=$(basename "$failed" .failed) |
There was a problem hiding this comment.
Remove the local keyword here since pkg is now declared at the top of the function.
| local pkg=$(basename "$failed" .failed) | |
| pkg=$(basename "$failed" .failed) |
Sorry, something went wrong.
Reviewers
1 more reviewerAt least 1 approving review is required to merge this pull request.
The system tests in #18013 were running for 3 hours before timing out. This PR touches the same packages, to experiment at ways to improve reliability for the next release