Reproducer test for #197 wrapped external repo file change not detected#343
Open
tinder-maxwellelliott wants to merge 1 commit into
Open
Reproducer test for #197 wrapped external repo file change not detected#343tinder-maxwellelliott wants to merge 1 commit into
tinder-maxwellelliott wants to merge 1 commit into
Conversation
…tected The simple case (a main-repo target consuming a file from a single external repo) works on current bazel-diff. The unfixed shape is the one @Ahajha called out: a target inside one external repo is re-wrapped by ANOTHER external repo, and the main repo consumes only the wrapping repo. When the inner repo's source file changes, the main-repo consumer should be reported as impacted -- today it is not, unless the user manually enumerates every wrapping external repo in --fineGrainedHashExternalRepos. The new `wrapped_external_repo` fixture wires this up with bzlmod: inner_repo -> filegroup wrapping data.txt middle_repo -> alias re-exporting @inner_repo//:all_files //:consumer -> genrule consuming @middle_repo//:wrapped Confirmed by hand against the built CLI: --fineGrainedHashExternalRepos=@inner_repo -> impacted: @inner_repo//:all_files, @inner_repo//:data.txt -> //:consumer NOT impacted (bug) --fineGrainedHashExternalRepos=@inner_repo,@middle_repo -> impacted includes //:consumer (workaround proves the chain works when every wrapper is enumerated) The reproducer test asserts that listing only the source-of-truth repo is enough. It is @ignore'd until propagation through alias/wrapper external repos works. Refs #197 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
311be6f to
6b91f03
Compare
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
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.
Summary
@Ignored E2E reproducer test for #197, specifically the unfixed shape @Ahajha called out in his comment: a target inside one external repo is re-wrapped by ANOTHER external repo, and the main repo consumes only the wrapping repo.cli/src/test/resources/workspaces/wrapped_external_repo/wires this up with bzlmod local_path_override:inner_repo— filegroup wrappingdata.txtmiddle_repo— alias re-exporting@inner_repo//:all_files//:consumer— genrule consuming@middle_repo//:wrapped--fineGrainedHashExternalRepos=@inner_repo→ impacted is{@inner_repo//:all_files, @inner_repo//:data.txt};//:consumeris not impacted.--fineGrainedHashExternalRepos=@inner_repo,@middle_repo→ impacted includes//:consumer, proving the chain works once every wrapper is enumerated.@inner_repo) is enough — bazel-diff should follow the dep chain through alias/wrapper external repos without manual enumeration. The asserted target is//:consumer.The simple "main target → one external repo's source file" case already works on current bazel-diff (probably fixed since the issue was filed). This PR captures the remaining, more subtle shape.
This is a test-only change. The reproducer is
@org.junit.Ignored so CI stays green; the annotation documents the issue it tracks. The workspace fixture follows the same pattern as the existingdistance_metrics,cquery_failing_target, andmacro_invalidationworkspaces.Test plan
bazel build //cli:cli-test-libsucceeds.bazel query 'deps(//:consumer)'from the new workspace returns@inner_repo//:data.txt(the chain is intact).bazel test //cli:E2ETestis unchanged (the new test is@Ignored and skipped).inner_repo/data.txt, confirms the bug (see Summary above).@Ignoreand re-run the test — it should pass without the user enumerating every wrapping repo.🤖 Generated with Claude Code