Report cycles involving aspects correctly.

This involved refactoring BuildViewTestCase a bit so that its behavior is closer to that of Bazel with --experimental_interleave_loading_and_analysis.

RELNOTES:

--
MOS_MIGRATED_REVID=124816624

3 files changed, 35 insertions, 3 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 2c2adf7e8c..51ba85f8e8 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -887,7 +887,6 @@ public class BuildViewTest extends BuildViewTestBase {
           .matches("Analysis of target '//foo:(java|cpp)' failed; build aborted.*");
     }
     assertContainsEvent("cycle in dependency graph");
-    assertContainsEvent("This cycle occurred because of a configuration option");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
index 26c7666b21..4e0bfc2290 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
@@ -186,6 +186,40 @@ public class CircularDependencyTest extends BuildViewTestCase {
         "genrule(name='b', srcs=['c'], tools=['c'], outs=['b.out'], cmd=':')",
         "genrule(name='c', srcs=['b.out'], outs=[], cmd=':')");
     getConfiguredTarget("//x:b"); // doesn't crash!
-    assertContainsEvent("in genrule rule //x:b: cycle in dependency graph");
+    assertContainsEvent("cycle in dependency graph");
+  }
+
+  @Test
+  public void testAspectCycle() throws Exception {
+    reporter.removeHandler(failFastHandler);
+    scratch.file("x/BUILD",
+        "load('//x:x.bzl', 'aspected', 'plain')",
+        // Using data= makes the dependency graph clearer because then the aspect does not propagate
+        // from aspectdep through a to b (and c)
+        "plain(name = 'a', noaspect_deps = [':b'])",
+        "aspected(name = 'b', aspect_deps = ['c'])",
+        "plain(name = 'c')",
+        "plain(name = 'aspectdep', aspect_deps = ['a'])");
+
+    scratch.file("x/x.bzl",
+        "def _impl(ctx):",
+        "    return struct()",
+        "",
+        "rule_aspect = aspect(",
+        "    implementation = _impl,",
+        "    attr_aspects = ['aspect_deps'],",
+        "    attrs = { '_implicit': attr.label(default = Label('//x:aspectdep')) })",
+        "",
+        "plain = rule(",
+        "    implementation = _impl,",
+        "    attrs = { 'aspect_deps': attr.label_list(), 'noaspect_deps': attr.label_list() })",
+        "",
+        "aspected = rule(",
+        "    implementation = _impl,",
+        "    attrs = { 'aspect_deps': attr.label_list(aspects = [rule_aspect]) })");
+
+    getConfiguredTarget("//x:a");
+    assertContainsEvent("cycle in dependency graph");
+    assertContainsEvent("//x:c with aspect //x:x.bzl%rule_aspect");
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 84ece3d89f..9460d95ab3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -652,7 +652,6 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
    */
   protected ConfiguredTarget getConfiguredTarget(Label label, BuildConfiguration config)
       throws NoSuchPackageException, NoSuchTargetException, InterruptedException {
-    ensureTargetsVisited(label);
     return view.getConfiguredTargetForTesting(
         reporter, BlazeTestUtils.convertLabel(label), config);
   }