fix: avoid running split ITs unexpectedly on unrelated changes (#12029) - #14217
Draft
lqiu96 wants to merge 1 commit into
Draft
fix: avoid running split ITs unexpectedly on unrelated changes (#12029)#14217lqiu96 wants to merge 1 commit into
lqiu96 wants to merge 1 commit into
Conversation
lqiu96
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request optimizes Kokoro CI scripts by replacing slow Maven evaluation commands (mvn help:evaluate) with fast, native Bash and sed parsing of pom.xml files, significantly reducing CI boot times. It also introduces a new is_module_modified helper to check for module-specific changes. Feedback highlights a critical bug where is_module_modified bypasses global override checks (such as parent POM changes or TEST_ALL_MODULES), which would cause tests to be skipped when they should run. Additionally, improvements are suggested to make the unit tests more robust by resolving paths relative to the script's directory instead of relying on the current working directory.
lqiu96
force-pushed
the
fix-split-it-module-detection
branch
from
August 31, 2026 15:35
42ce494 to
c66781c
Compare
lqiu96
changed the title
lqiu96
added a commit
to lqiu96/google-cloud-java
that referenced
this pull request
Aug 31, 2026
This PR is part 2 of 2 in a stacked series: 1. googleapis#14217: fix: avoid running split ITs unexpectedly on unrelated changes (googleapis#12029) 2. This PR: perf(ci): replace mvn help:evaluate with native bash and sed extraction ## Problem In Kokoro CI scripts, mvn help:evaluate was used to parse POM values: - In generate_modified_modules_list (.kokoro/common.sh), evaluating project.modules launched a full JVM and evaluated the monorepo POMs, taking 20–30+ seconds on every single CI run. - In downstream-build.sh (.kokoro/presubmit/downstream-build.sh), evaluating gapic-showcase.version launched another Maven JVM process. ## Changes - Pure-Bash Module Extraction: Updated generate_modified_modules_list to use extract_pom_modules pom.xml, extracting all 270 modules in ~0.02s without JVM boot overhead. - Sed Showcase Version Extraction: Updated downstream-build.sh to parse <gapic-showcase.version> directly using sed with fail-fast validation. - Unit Tests: Added automated unit tests in .kokoro/common_test.sh for extract_pom_modules and generate_modified_modules_list. - Refactoring & Best Practices: Scoped all temporary variables as local, used read -r module <<< "${module}" for pure-bash whitespace trimming, and anchored paths with ${scriptDir}/..
lqiu96
force-pushed
the
fix-split-it-module-detection
branch
from
August 31, 2026 15:53
c66781c to
4a6a76b
Compare
lqiu96
added a commit
to lqiu96/google-cloud-java
that referenced
this pull request
Aug 31, 2026
This PR is part 2 of 2 in a stacked series: 1. googleapis#14217: fix: avoid running split ITs unexpectedly on unrelated changes (googleapis#12029) 2. This PR: perf(ci): replace mvn help:evaluate with native bash and sed extraction In Kokoro CI scripts, mvn help:evaluate was used to parse POM values: - In generate_modified_modules_list (.kokoro/common.sh), evaluating project.modules launched a full JVM and evaluated the monorepo POMs, taking 20–30+ seconds on every single CI run. - In downstream-build.sh (.kokoro/presubmit/downstream-build.sh), evaluating gapic-showcase.version launched another Maven JVM process. - Pure-Bash Module Extraction: Updated generate_modified_modules_list to use extract_pom_modules pom.xml, extracting all 270 modules in ~0.02s without JVM boot overhead. - Sed Showcase Version Extraction: Updated downstream-build.sh to parse <gapic-showcase.version> directly using sed with fail-fast validation. - Unit Tests: Added automated unit tests in .kokoro/common_test.sh for extract_pom_modules and generate_modified_modules_list. - Refactoring & Best Practices: Scoped all temporary variables as local, used read -r module <<< "${module}" for pure-bash whitespace trimming, and anchored paths with ${scriptDir}/..
lqiu96
force-pushed
the
fix-split-it-module-detection
branch
from
August 31, 2026 16:02
4a6a76b to
a15b6a5
Compare
lqiu96
added a commit
to lqiu96/google-cloud-java
that referenced
this pull request
Aug 31, 2026
This PR is part 2 of 2 in a stacked series: 1. googleapis#14217: fix: avoid running split ITs unexpectedly on unrelated changes (googleapis#12029) 2. This PR: perf(ci): replace mvn help:evaluate with native bash and sed extraction In Kokoro CI scripts, mvn help:evaluate was used to parse POM values: - In generate_modified_modules_list (.kokoro/common.sh), evaluating project.modules launched a full JVM and evaluated the monorepo POMs, taking 20–30+ seconds on every single CI run. - In downstream-build.sh (.kokoro/presubmit/downstream-build.sh), evaluating gapic-showcase.version launched another Maven JVM process. - Pure-Bash Module Extraction: Updated generate_modified_modules_list to use extract_pom_modules pom.xml, extracting all 270 modules in ~0.02s without JVM boot overhead. - Sed Showcase Version Extraction: Updated downstream-build.sh to parse <gapic-showcase.version> directly using sed with fail-fast validation. - Unit Tests: Added automated unit tests in .kokoro/common_test.sh for extract_pom_modules and generate_modified_modules_list. - Refactoring & Best Practices: Scoped all temporary variables as local, used read -r module <<< "${module}" for pure-bash whitespace trimming, and anchored paths with ${scriptDir}/..
lqiu96
added a commit
to lqiu96/google-cloud-java
that referenced
this pull request
Aug 31, 2026
…eapis#12029) This PR is part 1 of 2 in a stacked series: 1. This PR (googleapis#14217): fix: avoid running split ITs unexpectedly on unrelated changes (googleapis#12029) 2. googleapis#14218: perf(ci): replace mvn help:evaluate with native bash and sed extraction ## Problem In split integration and GraalVM tests (e.g. integration-single and graalvm-single), module modification detection previously checked if the module name was present as a substring in modified_module_list. For modules with shared prefixes (such as java-bigquery vs java-bigquerystorage, java-bigquery-jdbc, or java-bigqueryconnection), modifying one module could trigger tests for other prefix-sharing modules. ## Changes - **Modular Helper Functions with Single Responsibility**: - `populate_modified_files`: Encapsulates diff retrieval, caching in `modified_files`, and configuring git `safe.directory` for Docker container ownership mismatches. - `should_test_all_modules`: Purely checks global overrides that require testing all modules (parent POMs, core shared dependencies, and `TEST_ALL_MODULES="true"`). - `is_module_modified`: Strictly checks if files within a specific module directory were modified (`^${module}/`), avoiding prefix collisions. - **Explicit Job Control in Split Jobs**: - In `.kokoro/build.sh` (`integration-single` and `graalvm-single`), runs tests if either monorepo-wide testing is required or the specific module was modified: `! should_test_all_modules && ! is_module_modified "${BUILD_SUBDIR}"` -> skip. - **Updated generate_modified_modules_list**: - Reuses `populate_modified_files` and `should_test_all_modules`. - **Unit Tests**: - Added unit tests in `.kokoro/common_test.sh` for `is_module_modified` and `should_test_all_modules`. Fixes googleapis#12029
lqiu96
force-pushed
the
fix-split-it-module-detection
branch
from
August 31, 2026 16:20
a15b6a5 to
54fb41a
Compare
lqiu96
added a commit
to lqiu96/google-cloud-java
that referenced
this pull request
Aug 31, 2026
This PR is part 2 of 2 in a stacked series: 1. googleapis#14217: fix: avoid running split ITs unexpectedly on unrelated changes (googleapis#12029) 2. This PR: perf(ci): replace mvn help:evaluate with native bash and sed extraction In Kokoro CI scripts, mvn help:evaluate was used to parse POM values: - In generate_modified_modules_list (.kokoro/common.sh), evaluating project.modules launched a full JVM and evaluated the monorepo POMs, taking 20–30+ seconds on every single CI run. - In downstream-build.sh (.kokoro/presubmit/downstream-build.sh), evaluating gapic-showcase.version launched another Maven JVM process. - Pure-Bash Module Extraction: Updated generate_modified_modules_list to use extract_pom_modules pom.xml, extracting all 270 modules in ~0.02s without JVM boot overhead. - Sed Showcase Version Extraction: Updated downstream-build.sh to parse <gapic-showcase.version> directly using sed with fail-fast validation. - Unit Tests: Added automated unit tests in .kokoro/common_test.sh for extract_pom_modules and generate_modified_modules_list. - Refactoring & Best Practices: Scoped all temporary variables as local, used read -r module <<< "${module}" for pure-bash whitespace trimming, and anchored paths with ${scriptDir}/..
…eapis#12029) This PR is part 1 of 2 in a stacked series: 1. This PR (googleapis#14217): fix: avoid running split ITs unexpectedly on unrelated changes (googleapis#12029) 2. googleapis#14218: perf(ci): replace mvn help:evaluate with native bash and sed extraction ## Problem In split integration and GraalVM tests (e.g. integration-single and graalvm-single), module modification detection previously checked if the module name was present as a substring in modified_module_list. For modules with shared prefixes (such as java-bigquery vs java-bigquerystorage, java-bigquery-jdbc, or java-bigqueryconnection), modifying one module could trigger tests for other prefix-sharing modules. ## Changes - **Modular Helper Functions with Single Responsibility**: - `populate_modified_files`: Encapsulates diff retrieval, caching in `modified_files`, and configuring git `safe.directory` for Docker container ownership mismatches. - `should_test_all_modules`: Purely checks global overrides that require testing all modules (parent POMs, core shared dependencies, and `TEST_ALL_MODULES="true"`). - `is_module_modified`: Strictly checks if files within a specific module directory were modified (`^${module}/`), avoiding prefix collisions. - **Explicit Job Control in Split Jobs**: - In `.kokoro/build.sh` (`integration-single` and `graalvm-single`), runs tests if either monorepo-wide testing is required or the specific module was modified: `! should_test_all_modules && ! is_module_modified "${BUILD_SUBDIR}"` -> skip. - **Updated generate_modified_modules_list**: - Reuses `populate_modified_files` and `should_test_all_modules`. - **Unit Tests**: - Added unit tests in `.kokoro/common_test.sh` for `is_module_modified` and `should_test_all_modules`. Fixes googleapis#12029
lqiu96
force-pushed
the
fix-split-it-module-detection
branch
from
August 31, 2026 16:28
54fb41a to
16b1841
Compare
lqiu96
added a commit
to lqiu96/google-cloud-java
that referenced
this pull request
Aug 31, 2026
This PR is part 2 of 2 in a stacked series: 1. googleapis#14217: fix: avoid running split ITs unexpectedly on unrelated changes (googleapis#12029) 2. This PR: perf(ci): replace mvn help:evaluate with native bash and sed extraction In Kokoro CI scripts, mvn help:evaluate was used to parse POM values: - In generate_modified_modules_list (.kokoro/common.sh), evaluating project.modules launched a full JVM and evaluated the monorepo POMs, taking 20–30+ seconds on every single CI run. - In downstream-build.sh (.kokoro/presubmit/downstream-build.sh), evaluating gapic-showcase.version launched another Maven JVM process. - Pure-Bash Module Extraction: Updated generate_modified_modules_list to use extract_pom_modules pom.xml, extracting all 270 modules in ~0.02s without JVM boot overhead. - Sed Showcase Version Extraction: Updated downstream-build.sh to parse <gapic-showcase.version> directly using sed with fail-fast validation. - Unit Tests: Added automated unit tests in .kokoro/common_test.sh for extract_pom_modules and generate_modified_modules_list. - Refactoring & Best Practices: Scoped all temporary variables as local, used read -r module <<< "${module}" for pure-bash whitespace trimming, and anchored paths with ${scriptDir}/..
lqiu96
marked this pull request as draft
August 31, 2026 17:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12029 (b/487770623)
This PR is part 1 of 2 in a stacked series:
fix: avoid running split ITs unexpectedly on unrelated changes (#12029)perf(ci): replace mvn help:evaluate with native bash and sed extractionProblem
In split integration and GraalVM tests (e.g.
integration-singleandgraalvm-single), module modification detection previously checked if the module name was present as a substring inmodified_module_list. For modules with shared prefixes (such asjava-bigqueryvsjava-bigquerystorage,java-bigquery-jdbc, orjava-bigqueryconnection), modifying one module could trigger tests for other prefix-sharing modules.Changes
is_module_modified): Addedis_module_modifiedin.kokoro/common.shusing exact directory prefix matching (^${module}/).is_module_modifiedrespects parent POM modifications (google-cloud-(pom|jar)-parent/pom.xml), shared dependency modifications (sdk-platform-java/java-shared-dependencies), andTEST_ALL_MODULES="true"so that dependency and parent updates properly verify downstream integration suites.integration-singleandgraalvm-singlein.kokoro/build.shto checkis_module_modified "${BUILD_SUBDIR}"directly.shared_dependencies_modifiedin.kokoro/common.shto matchsdk-platform-java/java-shared-dependencies..kokoro/common_test.shforis_module_modifiedcovering empty inputs, prefix collision prevention, parent pom modification, shared dependencies modification, andTEST_ALL_MODULES.