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

.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,60 @@ 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
+# Computes and returns the list of modified files via git diff, caching the result
+# in the variable 'modified_files' to avoid repeated Git executions across checks.
+function get_modified_files() {
+  if [[ -z "${modified_files:-}" ]]; then
+    # 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.
+    modified_files=$(git diff --name-only "${target_branch}...${target_commit}")
+  fi
+  printf '%s\n' "${modified_files}"
+}
+
+# Determines if the entire monorepo must be tested.
 #
-# The first positional argument is a value true/false. If true (default), then
-# exclude modules from the global exclusion list.
+# 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.
+#
+# 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 +304,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 +324,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

@@ -75,6 +75,113 @@ 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.
+  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
+  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
+  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 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)
+  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
+  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
+  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
+  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
+
+  # TEST_ALL_MODULES=true
+  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 modified_files
+}
+
+# Test get_modified_files returns cached files when set.
+function test_get_modified_files {
+  modified_files="dummy/file.txt"
+  local files
+  files=$(get_modified_files)
+  if [[ "${files}" != "dummy/file.txt" ]]; then
+    echo "get_modified_files failed to return cached files"
+    exit 1
+  fi
+  unset modified_files
+}
+
 test_find_all_poms_with_versioned_dependency
 test_update_pom_dependency
 test_parse_pom_version
+test_get_modified_files
+test_should_test_all_modules
+test_is_module_modified
+