chore(ci): optimize kokoro system tests with concurrency#17444
Conversation
There was a problem hiding this comment.
Code Review
This pull request parallelizes package testing in '.kokoro/system.sh' using 'xargs' to run up to 8 tests concurrently. However, a critical issue was identified where 'xargs -I {}' treats the entire space-separated string of packages as a single argument, which will cause the tests to fail. A suggestion was provided to output each package on a new line using 'printf' before passing them to 'xargs'.
| 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.
Using xargs -I {} changes the delimiter to newline only, meaning it ignores spaces and treats the entire line as a single item. Since echo "$PACKAGES_TO_TEST" outputs all packages on a single space-separated line, xargs will execute the command only once, passing the entire string of packages as a single argument (e.g., run_package_test "pkg1 pkg2 pkg3"). This will cause the tests to fail.
To fix this, output each package on a new line using printf "%s\n" $PACKAGES_TO_TEST (unquoted so that it splits on whitespace) and remove the redundant -n 1 flag.
| echo "$PACKAGES_TO_TEST" | xargs -n 1 -P 8 -I {} bash -c 'run_package_test "{}" > ".logs/{}.log" 2>&1 || touch ".logs/{}.failed"' | |
| printf "%s\n" $PACKAGES_TO_TEST | xargs -P 8 -I {} bash -c 'run_package_test "{}" > ".logs/{}.log" 2>&1 || touch ".logs/{}.failed"' |
daniel-sanche
left a comment
There was a problem hiding this comment.
I did some similar experimenting in #17438
Can you make some small changes to firestore, bigtable, etc so we can see this in action? We can revert them after if needed (I'm thinking maybe we should add some tags to trigger a full test run)
Replaced the sequential
run_package_testloop with a fan-out execution usingxargs -P 8.Results in: #17446