diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt index 167771c..1405af1 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -1337,6 +1337,75 @@ class E2ETest { assertThat(noKeepGoingExit).isEqualTo(1) } + @Test + fun testUndeclaredWorkspaceReadIsNotImpacted_reproducerForIssue401() { + // Reproducer for https://github.com/Tinder/bazel-diff/issues/401 + // + // Feature request: "always-affected" hashing for targets that perform undeclared + // workspace reads (non-hermetic targets). bazel-diff derives a target's hash purely + // from its DECLARED graph -- srcs, deps, and attributes. A target that reads files it + // does not declare (e.g. a `buildifier_test` that scans every `.bzl` in the repo + // without listing them in `srcs`) therefore gets a STABLE hash even when one of those + // undeclared files changes, so `get-impacted-targets` skips it -- even though the test + // would fail if actually run. + // + // The `always_affected_external` workspace has two genrules: + // //:scanner -- tagged `external`, its action reads `scanned_data.txt` but does NOT + // declare it as a src (the undeclared / non-hermetic read). + // //:hermetic -- a normal target that declares `declared_src.txt` as a src. + // + // Between the two checkouts we edit BOTH files. The hermetic target is correctly + // reported as impacted; the `external`-tagged scanner is NOT -- pinning the current + // behaviour that motivates the feature request. If an `--alwaysAffectedTags` + // (or similar) feature lands, the scanner should start appearing in the impacted set + // and this assertion will flag that the behaviour changed. + val workspaceA = copyTestWorkspace("always_affected_external") + val workspaceB = copyTestWorkspace("always_affected_external") + + // Change the UNDECLARED read consumed by //:scanner and the DECLARED src of //:hermetic. + File(workspaceB, "scanned_data.txt").writeText("v2\n") + File(workspaceB, "declared_src.txt").writeText("goodbye\n") + + val outputDir = temp.newFolder() + val from = File(outputDir, "starting_hashes.json") + val to = File(outputDir, "final_hashes.json") + val impactedTargetsOutput = File(outputDir, "impacted_targets.txt") + + val cli = CommandLine(BazelDiff()) + + assertThat( + cli.execute( + "generate-hashes", "-w", workspaceA.absolutePath, "-b", "bazel", from.absolutePath)) + .isEqualTo(0) + assertThat( + cli.execute( + "generate-hashes", "-w", workspaceB.absolutePath, "-b", "bazel", to.absolutePath)) + .isEqualTo(0) + assertThat( + cli.execute( + "get-impacted-targets", + "-w", + workspaceB.absolutePath, + "-b", + "bazel", + "-sh", + from.absolutePath, + "-fh", + to.absolutePath, + "-o", + impactedTargetsOutput.absolutePath)) + .isEqualTo(0) + + val impacted = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet() + + // The hermetic target declared its changed source, so it is correctly impacted. + assertThat(impacted.contains("//:hermetic")).isEqualTo(true) + + // The `external`-tagged scanner read a file it never declared. Its hash is unchanged, + // so bazel-diff does NOT report it -- this is exactly the gap #401 asks to close. + assertThat(impacted.contains("//:scanner")).isEqualTo(false) + } + /** * Returns the Bazel version triple by running `bazel version`, or null if it cannot be * determined. diff --git a/cli/src/test/resources/workspaces/always_affected_external/BUILD b/cli/src/test/resources/workspaces/always_affected_external/BUILD new file mode 100644 index 0000000..2784500 --- /dev/null +++ b/cli/src/test/resources/workspaces/always_affected_external/BUILD @@ -0,0 +1,36 @@ +# Reproducer workspace for https://github.com/Tinder/bazel-diff/issues/401 +# +# The issue: a target that reads files it does NOT declare as inputs (a +# non-hermetic / "undeclared workspace read") gets a stable hash from +# bazel-diff, because hashing is derived purely from the declared graph +# (srcs / deps / attrs). The canonical real-world example is a +# `buildifier_test` that scans every `.bzl` in the repo without declaring +# any of them: edit only a `.bzl` file and the target's hash is unchanged, +# so `get-impacted-targets` skips it -- even though the test would fail if +# actually run. +# +# `scanner` stands in for that buildifier-style target. It is tagged +# `external` (the tag the feature request proposes keying "always-affected" +# behaviour off of) and its action reads `scanned_data.txt`, but that file +# is NOT in `srcs` -- it is an undeclared read. + +genrule( + name = "scanner", + srcs = [], + outs = ["scanner_result.txt"], + # Reads scanned_data.txt at action time without declaring it as an input. + # This mirrors a linter/buildifier target scanning the workspace. + cmd = "cat $$(dirname $(location scanner_result.txt))/../../../scanned_data.txt > $@ 2>/dev/null || echo scanned > $@", + tags = ["external"], + visibility = ["//visibility:public"], +) + +# A normal, hermetic target for contrast: it declares its source, so editing +# `declared_src.txt` DOES change its hash and it IS reported as impacted. +genrule( + name = "hermetic", + srcs = ["declared_src.txt"], + outs = ["hermetic_result.txt"], + cmd = "cp $< $@", + visibility = ["//visibility:public"], +) diff --git a/cli/src/test/resources/workspaces/always_affected_external/MODULE.bazel b/cli/src/test/resources/workspaces/always_affected_external/MODULE.bazel new file mode 100644 index 0000000..c5025f9 --- /dev/null +++ b/cli/src/test/resources/workspaces/always_affected_external/MODULE.bazel @@ -0,0 +1,4 @@ +module( + name = "always_affected_external", + version = "0.0.0", +) diff --git a/cli/src/test/resources/workspaces/always_affected_external/declared_src.txt b/cli/src/test/resources/workspaces/always_affected_external/declared_src.txt new file mode 100644 index 0000000..ce01362 --- /dev/null +++ b/cli/src/test/resources/workspaces/always_affected_external/declared_src.txt @@ -0,0 +1 @@ +hello diff --git a/cli/src/test/resources/workspaces/always_affected_external/scanned_data.txt b/cli/src/test/resources/workspaces/always_affected_external/scanned_data.txt new file mode 100644 index 0000000..626799f --- /dev/null +++ b/cli/src/test/resources/workspaces/always_affected_external/scanned_data.txt @@ -0,0 +1 @@ +v1