[build] validate affected test targets against the current build graph

[titusfortner]

🔗 Related Issues

💥 What does this PR do?

The Check Targets job selects which tests to run from a cached test-file index (a snapshot of trunk) and writes them to bazel-targets.txt; the per-language jobs (Java, Python, Ruby, .NET, Rust, Grid) feed that list to bazel test/bazel query. Because the index is a snapshot, it drifts from the branch under test:

• A renamed or removed target left a label in the list that no longer exists, so the language job failed outright on "no such target".
• A target added or renamed via a BUILD-only change wasn't in the snapshot, so CI silently skipped tests that should have run.

Check Targets now validates the selected targets against the current checkout before the language jobs act on them.

🔧 Implementation Notes

• Validation only queries packages that still exist on disk, so it can't itself error on a deleted package.
• If the validation query can't run for any reason, the full target list is kept — erring toward running extra tests, never toward skipping ones that should run.
• The cached index is still trusted for the file→test mapping; this only reconciles its output.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s): Claude Code
  • What was generated: implementation and tests
  • I reviewed all AI output and can explain the change

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Validate affected Bazel test targets against current build graph

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Validate affected Bazel test labels against the current checkout’s build graph.
• Prune stale targets from the trunk snapshot index, but keep all on validation failure.
• Treat BUILD-file changes as package-wide to catch added/renamed targets.

Diagram

graph TD
  A["CI: Check Targets"] --> B["git diff changed files"] --> D["Select affected targets"] --> E["Validate targets in graph"] --> F[("bazel-targets.txt")] --> G["CI: Language test jobs"]
  C[("Cached test index (trunk)")] --> D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Rebuild the test-file index on every PR run

• ➕ Always accurate mapping for the exact checkout; no drift issues.
• ➕ Potentially reduces need for fallback queries and validation heuristics.
• ➖ Expensive: requires querying all tests and deps each run.
• ➖ Longer CI times and higher Bazel load; defeats the purpose of caching.

2. Filter targets in each language job (pre-step bazel query)

• ➕ Keeps Check Targets simple; each job guarantees its own input validity.
• ➕ Can tailor filtering per job (e.g., per binding scope).
• ➖ Duplicated logic across jobs; more maintenance and inconsistency risk.
• ➖ Repeats Bazel queries multiple times per workflow run.

Recommendation: The PR’s approach (single validation step in Check Targets that prunes stale labels but fails open) is the best tradeoff: it preserves the cached index’s performance benefits while preventing hard failures on removed/renamed targets and reducing silent under-testing on BUILD-only changes.

Files changed (1) +33 / -3

Bug fix (1) +33 / -3

bazel.rakePrune stale affected test targets and widen BUILD-file fallback queries +33/-3

Prune stale affected test targets and widen BUILD-file fallback queries

• Affected-target computation now runs a Bazel query over only existing on-disk packages to drop stale test labels produced by the trunk snapshot index, logging dropped labels and failing open (keep all) if the validation query errors. The unindexed-file fallback query now treats BUILD/BUILD.bazel changes as package-wide (//pkg:*) since BUILD files won’t appear in srcs, ensuring added/renamed targets are discovered.

rake_tasks/bazel.rake

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

1. prune_stale_targets missing unit tests 📘 Rule violation ☼ Reliability

Description

New target-validation logic was added (prune_stale_targets/live_test_labels and BUILD-file query
scoping) without any corresponding tests in this PR. This increases risk of CI regressions (e.g.,
accidentally dropping valid targets or failing to include BUILD-only changes) without fast feedback.

Code

rake_tasks/bazel.rake[R185-217]

+  prune_stale_targets(affected.to_a)
+end
+
+# The index is a trunk snapshot, so renamed/removed targets leave stale labels that break
+# `bazel test`. Drop any label whose test is gone; keep all if the check can't run.
+def prune_stale_targets(labels)
+  live = live_test_labels(labels)
+  return labels if live.nil?
+
+  kept = labels.select { |l| live.include?(l) }
+  (labels - kept).each { |l| puts "  Dropping stale target not in graph: #{l}" }
+  kept
+end
+
+# Test labels that still exist in `labels`' packages. Only on-disk packages are queried, so
+# `//pkg:*` can't error on a deleted package. Returns nil if the query itself fails.
+def live_test_labels(labels)
+  packages = labels.filter_map { |l| l[%r{\A//([^:]*)}, 1] }.uniq
+  packages.select! { |pkg| File.exist?(File.join(pkg, 'BUILD.bazel')) || File.exist?(File.join(pkg, 'BUILD')) }
+  return Set.new if packages.empty?
+
+  live = Set.new
+  query = packages.map { |pkg| "//#{pkg}:*" }.join(' + ')
+  Bazel.execute('query', ['--output=label'], "kind(_test, #{query})") do |out|
+    live = out.lines.map(&:strip).select { |l| l.start_with?('//') }.to_set
+  end
+  live
+rescue StandardError => e
+  puts "  Warning: keeping all targets; stale-target check failed: #{e.message}"
+  nil
end

def query_unindexed_file(filepath)

Evidence

PR Compliance ID 4 requires adding/updating tests when feasible. The PR adds new behavior for
pruning stale Bazel targets and changing query scope for BUILD files, but no tests are added/updated
alongside these new code paths.

AGENTS.md: Add/Update Tests for Changes; Prefer Small Unit Tests and Avoid Mocks
rake_tasks/bazel.rake[185-243]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New logic in `rake_tasks/bazel.rake` changes how affected Bazel test targets are selected/validated, but this PR does not add tests to cover the new behaviors.

## Issue Context
The PR introduces:
- pruning of stale targets via `prune_stale_targets`/`live_test_labels`
- special handling for BUILD/BUILD.bazel changes via package-wide query scope
These are correctness-sensitive and should be validated with small unit tests.

## Fix Focus Areas
- rake_tasks/bazel.rake[185-243]
- rb/spec/unit/rake_tasks/bazel_rake_spec.rb[1-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[Copilot]

Pull request overview

This PR hardens Selenium’s CI “affected Bazel targets” selection by reconciling the cached trunk-based test index output with the current checkout’s Bazel build graph, preventing CI failures from stale labels and reducing the chance of silently skipping tests after BUILD-only changes.

Changes:

• Prunes stale _test labels produced by the cached index by querying the current checkout’s build graph (falling back to keeping all labels if validation fails).
• Expands unindexed-file handling so BUILD file changes query the whole package (instead of attr(srcs, ...)) to catch added/renamed targets in that package.