Skip to content

Reproducer test for #335 loose substring match in queryTargetsDependingOnModules#346

Open
tinder-maxwellelliott wants to merge 2 commits into
masterfrom
claude/reproducer-issue-335-substring-match
Open

Reproducer test for #335 loose substring match in queryTargetsDependingOnModules#346
tinder-maxwellelliott wants to merge 2 commits into
masterfrom
claude/reproducer-issue-335-substring-match

Conversation

@tinder-maxwellelliott
Copy link
Copy Markdown
Collaborator

Summary

  • Adds an @Ignored unit reproducer for #335 fix Update bazel-diff-example.sh #3 (loose substring match in CalculateImpactedTargetsInteractor.queryTargetsDependingOnModules). This is a follow-up to #340, which covered the parser-robustness piece (fix Typos #1) of the same issue.
  • The current code is:
val moduleRepos = allTargets.keys
    .filter { it.startsWith("@@") && it.contains(moduleName) }
    .map { it.substring(2).substringBefore("//") }
    .toSet()

it.contains(moduleName) is a very loose match. A single changed module like aspect_bazel_lib matches every canonical repo whose name starts with that string — @@aspect_bazel_lib+, @@aspect_bazel_lib++toolchains+bats_toolchains, etc. Each match becomes its own serial bazel query rdeps(//..., @@<repo>//...) subprocess. On the workspace in #335 that fan-out produced ~5,000 subprocesses and took multiple hours.

  • New test class CalculateImpactedTargetsInteractorIssue335Test:
    • Wires its own koin module with a mock BazelQueryService that captures every query expression.
    • Simulates a workspace with one module (aspect_bazel_lib) plus two module-extension repos whose canonical names begin with the same apparent name.
    • Asserts that resolving the changed module yields exactly one rdeps query, scoped to the actual changed repo.

The asked-for fix in #335 is either to match the canonical repo key exactly (e.g. aspect_bazel_lib+ — the part before any further +/~), or to look up the repo via bazel mod dump_repo_mapping instead of a substring scan. Drop @Ignore once that lands.

This is a test-only change. cli/BUILD adds the new kt_jvm_test target.

Test plan

  • bazel build //cli:CalculateImpactedTargetsInteractorIssue335Test succeeds.
  • bazel test //cli:CalculateImpactedTargetsInteractorIssue335TestPASSED in 0.5s (the new test is @Ignored and skipped).
  • After the match is tightened, remove @Ignore and re-run — exactly one rdeps query should be captured.

🤖 Generated with Claude Code

…endingOnModules

Issue #335 lists multiple potential fixes; my first PR (#340) covered
fix #1 (parser robustness). This PR adds a reproducer for fix #3: the
loose substring match in
CalculateImpactedTargetsInteractor.queryTargetsDependingOnModules.

The current code is:

  val moduleRepos = allTargets.keys
      .filter { it.startsWith("@@") && it.contains(moduleName) }
      .map { it.substring(2).substringBefore("//") }
      .toSet()

`it.contains(moduleName)` is a very loose match. A single changed
module like `aspect_bazel_lib` matches every canonical repo whose name
starts with that string -- e.g. `@@aspect_bazel_lib+`,
`@@aspect_bazel_lib++toolchains+bats_toolchains`, etc. Each match
becomes its own serial `bazel query rdeps(//..., @@<repo>//...)`
subprocess. On the workspace in #335 that fan-out produced ~5,000
subprocesses and took multiple hours.

New test class CalculateImpactedTargetsInteractorIssue335Test:
- Wires its own koin module with a mock BazelQueryService that captures
  every query expression.
- Simulates a workspace with one module (aspect_bazel_lib) plus two
  module-extension repos whose canonical names begin with the same
  apparent name.
- Asserts that resolving the changed module yields exactly ONE rdeps
  query, scoped to the actual changed repo.

@ignore'd until the match is tightened (match canonical repo key
exactly, or look up via `bazel mod dump_repo_mapping`). cli/BUILD wires
up the new kt_jvm_test target.

Refs #335

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tinder-maxwellelliott tinder-maxwellelliott force-pushed the claude/reproducer-issue-335-substring-match branch from 647fa53 to 5fd35f4 Compare May 13, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant