Skip to content

[DRAFT] chore: improve system tests - #18206

Draft
daniel-sanche wants to merge 3 commits into
googleapis:mainfrom
daniel-sanche:improve_stuck_system_tests
Draft

[DRAFT] chore: improve system tests#18206
daniel-sanche wants to merge 3 commits into
googleapis:mainfrom
daniel-sanche:improve_stuck_system_tests

Conversation

@daniel-sanche

Copy link
Copy Markdown
Contributor

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .kokoro/system.sh
Comment on lines 129 to 132
local retval=0
local failed_count=0
local timed_out_count=0
local succeeded_count=0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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

Comment thread .kokoro/system.sh
for failed in "$LOG_DIR"/*.failed; do
if [ -f "$failed" ]; then
basename "$failed" .failed
local pkg=$(basename "$failed" .failed)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Remove the local keyword here since pkg is now declared at the top of the function.

Suggested change
local pkg=$(basename "$failed" .failed)
pkg=$(basename "$failed" .failed)

Comment thread .kokoro/system.sh
done
for failed in "$LOG_DIR"/*.failed; do
if [ -f "$failed" ]; then
local pkg=$(basename "$failed" .failed)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Remove the local keyword here since pkg is now declared at the top of the function.

Suggested change
local pkg=$(basename "$failed" .failed)
pkg=$(basename "$failed" .failed)

@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants