ci: avoid running split ITs unexpectedly on unrelated changes (#12029) (#14217)

Fixes #12029 (b/487770623)

This PR is part 1 of 2 in a stacked series:
1. This PR (#14217): `fix: avoid running split ITs unexpectedly on
unrelated changes (#12029)`
2. #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
- **Module Boundary Anchoring (`is_module_modified`)**: Added
`is_module_modified` in `.kokoro/common.sh` using exact directory prefix
matching (`^${module}/`).
- **Global Overrides**: `is_module_modified` respects parent POM
modifications (`google-cloud-(pom|jar)-parent/pom.xml`), shared
dependency modifications (`sdk-platform-java/java-shared-dependencies`),
and `TEST_ALL_MODULES="true"` so that dependency and parent updates
properly verify downstream integration suites.
- **Short-Circuit in Split Jobs**: Updated `integration-single` and
`graalvm-single` in `.kokoro/build.sh` to check `is_module_modified
"${BUILD_SUBDIR}"` directly.
- **Updated Shared Dependencies Path**: Updated
`shared_dependencies_modified` in `.kokoro/common.sh` to match
`sdk-platform-java/java-shared-dependencies`.
- **Unit Tests**: Added unit test coverage in `.kokoro/common_test.sh`
for `is_module_modified` covering empty inputs, prefix collision
prevention, parent pom modification, shared dependencies modification,
and `TEST_ALL_MODULES`.

Author: lqiu96

.kokoro/build.sh

@@ -119,10 +119,11 @@ case ${JOB_TYPE} in
     fi
     ;;
   integration-single)
-    generate_modified_modules_list false
     if [[ "$(release_please_snapshot_pull_request)" == "true" ]]; then
       echo "Not running integration checks -- this is Release Please SNAPSHOT pull request."
-    elif [[ ! " ${modified_module_list[*]} " =~ " ${BUILD_SUBDIR} " ]]; then
+    # Run tests if either global overrides require testing all modules (e.g. parent POM or
+    # shared dependencies) OR if this specific module was modified. Otherwise skip.
+    elif ! should_test_all_modules && ! is_module_modified "${BUILD_SUBDIR}"; then
       echo "${BUILD_SUBDIR} not modified, skipping split integration test"
     else
       echo "${BUILD_SUBDIR} modified, running split integration test"
@@ -188,10 +189,11 @@ case ${JOB_TYPE} in
     fi
     ;;
   graalvm-single)
-    generate_modified_modules_list false
     if [[ "$(release_please_snapshot_pull_request)" == "true" ]]; then
       echo "Not running GraalVM checks -- this is Release Please SNAPSHOT pull request."
-    elif [[ ! " ${modified_module_list[*]} " =~ " ${BUILD_SUBDIR} " ]]; then
+    # Run tests if either global overrides require testing all modules (e.g. parent POM or
+    # shared dependencies) OR if this specific module was modified. Otherwise skip.
+    elif ! should_test_all_modules && ! is_module_modified "${BUILD_SUBDIR}"; then
       echo "${BUILD_SUBDIR} not modified, skipping split GraalVM test"
     else
       echo "${BUILD_SUBDIR} modified, running split GraalVM test"

.kokoro/common.sh

@@ -231,21 +231,56 @@ function release_please_snapshot_pull_request() {
   fi
 }
 
-# Sets bash variables for maven_modules and modified_module_list
-# maven_modules is the list of all maven submodules of the root pom
-# modified_module_list is the subset of maven_modules that have been touched
-# in the current pull request
+# Returns the list of modified files in the current PR diff via git diff.
+function get_modified_files() {
+  # In Kokoro Docker containers, the build runs as root (UID 0) while repository files
+  # belong to the host user (UID 1000). Git 2.35.2+ flags this UID mismatch as 'dubious ownership'
+  # and aborts git commands; safe.directory allows Git to operate in this directory.
+  git config --global --add safe.directory "$(realpath .)" 2>/dev/null || true
+
+  # '${VAR:-DEFAULT}' uses $VAR if set and non-empty, otherwise falls back to DEFAULT.
+  # This allows developers to run these scripts locally outside the Kokoro CI environment.
+  local target_branch="${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH:-origin/main}"
+  local target_commit="${KOKORO_GITHUB_PULL_REQUEST_COMMIT:-HEAD}"
+
+  # 'git diff A...B' (triple-dot) diffs between the merge-base (common ancestor) of
+  # target_branch and target_commit, listing only files changed in this branch.
+  git diff --name-only "${target_branch}...${target_commit}"
+}
+
+# Determines if the entire monorepo must be tested.
+#
+# Monorepo-wide testing is triggered under three conditions:
+# 1. TEST_ALL_MODULES is set to "true" (used by nightly and scheduled CI builds).
+# 2. Root parent POMs (google-cloud-jar-parent or google-cloud-pom-parent) are modified,
+#    as changes to parent POMs affect shared dependency versions and compiler/build plugins.
+# 3. Core shared dependencies (sdk-platform-java/java-shared-dependencies) are modified,
+#    as gax, auth, and transport changes can break downstream client library integration tests.
+function should_test_all_modules() {
+  local files
+  files=$(get_modified_files)
+
+  # '<<< STRING' is a Bash "here-string" that feeds the string variable directly to
+  # stdin of grep, avoiding an external subshell pipeline (like 'echo "$var" | grep').
+  if [[ "${TEST_ALL_MODULES}" == "true" ]] || \
+     grep -q -E '^google-cloud-(pom|jar)-parent/pom.xml$' <<< "${files}" || \
+     grep -q -E '^sdk-platform-java/java-shared-dependencies/' <<< "${files}"; then
+    return 0
+  fi
+  return 1
+}
+
+# Generates the list of modified Maven modules for batch integration/GraalVM test jobs.
+# Sets global variables:
+# - maven_modules: list of all Maven modules defined in the root POM.
+# - modified_module_list: modules that need to be tested for the current PR.
 #
-# The first positional argument is a value true/false. If true (default), then
-# exclude modules from the global exclusion list.
+# Positional parameter $1 (default "true") specifies whether to filter out modules
+# defined in the 'excluded_modules' array.
 function generate_modified_modules_list() {
-  # Find the files changed from when the PR branched to the last commit
-  # Filter for java modules and get all the unique elements
-  # grep returns 1 (error code) and exits the pipeline if there is no match
-  # If there is no match, it will return true so the rest of the commands can run
-  git config --global --add safe.directory $(realpath .)
-  modified_files=$(git diff --name-only "${KOKORO_GITHUB_PULL_REQUEST_TARGET_BRANCH}...${KOKORO_GITHUB_PULL_REQUEST_COMMIT}")
-  printf "Modified files:\n%s\n" "${modified_files}"
+  local files
+  files=$(get_modified_files)
+  printf "Modified files:\n%s\n" "${files}"
 
   # Generate the list of valid maven modules
   maven_modules_list=$(mvn help:evaluate -Dexpression=project.modules | grep '<.*>.*</.*>' | sed -e 's/<.*>\(.*\)<\/.*>/\1/g')
@@ -265,14 +300,12 @@ function generate_modified_modules_list() {
   fi
 
   modified_module_list=()
-  # If either parent pom.xml is touched, run ITs on all the modules
-  parent_pom_modified=$(echo "${modified_files}" | grep -E '^google-cloud-(pom|jar)-parent/pom.xml$' || true)
-  shared_dependencies_modified=$(echo "${modified_files}" | grep -E '^java-shared-dependencies' || true)
-  if [[ (-n $parent_pom_modified) || (-n $shared_dependencies_modified) || ("${TEST_ALL_MODULES}" == "true") ]]; then
+  # If either parent pom.xml or core shared dependency is touched, run ITs on all the modules
+  if should_test_all_modules; then
     modified_module_list=(${maven_modules[*]})
     echo "Testing the entire monorepo"
   else
-    modules=$(echo "${modified_files}" | grep -E '(google-auth|java)-.*' || true)
+    modules=$(echo "${files}" | grep -E '(google-auth|java)-.*' || true)
     printf "Files in java modules:\n%s\n" "${modules}"
     if [[ -n $modules ]]; then
       modules=$(echo "${modules}" | cut -d '/' -f1 | sort -u)
@@ -287,6 +320,23 @@ function generate_modified_modules_list() {
   fi
 }
 
+# Checks if files within a specific module directory were modified in the PR diff.
+#
+# Uses exact directory prefix matching ('^${module}/') to prevent substring collisions
+# where modifying one module triggers tests for another module that shares its prefix
+# (e.g. java-bigquery vs java-bigquerystorage).
+function is_module_modified() {
+  local module="$1"
+  if [[ -z "${module}" ]]; then
+    return 1
+  fi
+
+  local files
+  files=$(get_modified_files)
+  # '<<< "${files}"' feeds the diff string directly to grep via stdin.
+  grep -q -E "^${module}/" <<< "${files}"
+}
+
 # Filters the modified_module_list to only include modules that contain
 # integration test files (matching IT*.java or *IT.java in src/test/java).
 # Not all modules will have ITs written and there is not need to test

.kokoro/common_test.sh

@@ -19,6 +19,12 @@ source "$scriptDir/common.sh"
 mkdir -p target
 cd target
 
+# Mock get_modified_files for unit testing to avoid running Git commands.
+# Individual tests set TEST_MODIFIED_FILES to simulate changed files.
+function get_modified_files() {
+  printf '%s\n' "${TEST_MODIFIED_FILES:-}"
+}
+
 function test_find_all_poms_with_versioned_dependency {
   mkdir -p test_find_all_poms_with_dependency
   pushd test_find_all_poms_with_dependency
@@ -75,6 +81,120 @@ function test_parse_pom_version {
   popd
 }
 
+# Tests that is_module_modified strictly matches the module directory prefix,
+# preventing prefix collisions (e.g. java-bigquery vs java-bigquerystorage).
+function test_is_module_modified {
+  if is_module_modified ""; then
+    echo "is_module_modified should return non-zero for empty module name"
+    exit 1
+  fi
+
+  # Touching 'java-bigquery' should not match 'java-bigquerystorage', 'java-bigquery-jdbc', etc.
+  TEST_MODIFIED_FILES="java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/BigQuery.java
+java-bigquery/pom.xml"
+
+  if ! is_module_modified "java-bigquery"; then
+    echo "is_module_modified failed to detect java-bigquery modification"
+    exit 1
+  fi
+  if is_module_modified "java-bigquerystorage"; then
+    echo "is_module_modified incorrectly matched java-bigquerystorage for java-bigquery"
+    exit 1
+  fi
+  if is_module_modified "java-bigquery-jdbc"; then
+    echo "is_module_modified incorrectly matched java-bigquery-jdbc for java-bigquery"
+    exit 1
+  fi
+  if is_module_modified "java-bigqueryconnection"; then
+    echo "is_module_modified incorrectly matched java-bigqueryconnection for java-bigquery"
+    exit 1
+  fi
+
+  # Touching 'java-bigquerystorage' should match only java-bigquerystorage
+  TEST_MODIFIED_FILES="java-bigquerystorage/google-cloud-bigquerystorage/src/main/java/Foo.java"
+  if is_module_modified "java-bigquery"; then
+    echo "is_module_modified incorrectly matched java-bigquery for java-bigquerystorage"
+    exit 1
+  fi
+  if ! is_module_modified "java-bigquerystorage"; then
+    echo "is_module_modified failed to detect java-bigquerystorage modification"
+    exit 1
+  fi
+
+  # External / root files should not be considered module modifications
+  TEST_MODIFIED_FILES="google-cloud-jar-parent/pom.xml
+sdk-platform-java/java-shared-dependencies/pom.xml"
+  if is_module_modified "java-bigquery"; then
+    echo "is_module_modified should return false for java-bigquery when only external files changed"
+    exit 1
+  fi
+
+  unset TEST_MODIFIED_FILES
+}
+
+# Test should_test_all_modules triggers properly on global changes.
+function test_should_test_all_modules {
+  # When only a normal library is modified, should return false (1)
+  TEST_MODIFIED_FILES="java-bigquery/pom.xml"
+  if should_test_all_modules; then
+    echo "should_test_all_modules should return false for single module change"
+    exit 1
+  fi
+
+  # Root jar parent pom
+  TEST_MODIFIED_FILES="google-cloud-jar-parent/pom.xml"
+  if ! should_test_all_modules; then
+    echo "should_test_all_modules should return true for google-cloud-jar-parent change"
+    exit 1
+  fi
+
+  # Root pom parent
+  TEST_MODIFIED_FILES="google-cloud-pom-parent/pom.xml"
+  if ! should_test_all_modules; then
+    echo "should_test_all_modules should return true for google-cloud-pom-parent change"
+    exit 1
+  fi
+
+  # Core shared dependencies
+  TEST_MODIFIED_FILES="sdk-platform-java/java-shared-dependencies/pom.xml"
+  if ! should_test_all_modules; then
+    echo "should_test_all_modules should return true for java-shared-dependencies change"
+    exit 1
+  fi
+
+  # Prefix collision check: sibling paths starting with java-shared-dependencies must not match
+  TEST_MODIFIED_FILES="sdk-platform-java/java-shared-dependencies-bom/pom.xml"
+  if should_test_all_modules; then
+    echo "should_test_all_modules should return false for java-shared-dependencies prefix match"
+    exit 1
+  fi
+
+  # TEST_ALL_MODULES=true
+  TEST_MODIFIED_FILES=""
+  if ! TEST_ALL_MODULES="true" should_test_all_modules; then
+    echo "should_test_all_modules should return true when TEST_ALL_MODULES is true"
+    exit 1
+  fi
+
+  unset TEST_MODIFIED_FILES
+}
+
+# Test mock get_modified_files returns simulated files.
+function test_mock_get_modified_files {
+  TEST_MODIFIED_FILES="dummy/file.txt"
+  local files
+  files=$(get_modified_files)
+  if [[ "${files}" != "dummy/file.txt" ]]; then
+    echo "mock get_modified_files failed to return expected files"
+    exit 1
+  fi
+  unset TEST_MODIFIED_FILES
+}
+
 test_find_all_poms_with_versioned_dependency
 test_update_pom_dependency
 test_parse_pom_version
+test_mock_get_modified_files
+test_should_test_all_modules
+test_is_module_modified
+