Adds cycle detection errors when top-level dynamic

configuration creation fails because transitive fragment
visitation hits these cycles.

This makes CircularDependencyTest pass with dynamic
configurations.

It's a little bit unfortunate that BuildViewTestCase
follows a different code path to create configured targets
than production (BuildView.getConfiguredTargetForTesting
vs. BuildView.update). As a result, doing an actual build over
the rules defined in CircularDependencyTest#testTwoCycles
correctly reports the cycle, while the test itself doesn't.
That means the test isn't 100% faithfully testing production
logic.

But I'm not interested in fixing the gap between
BuildView.update and BuildView.getConfiguredTargetForTesting
in this change. That's part of a larger refactoring effort
on the various forked ways of acccessing configured targets
and dependencies in BuildView.

--
MOS_MIGRATED_REVID=125118553

1 files changed, 9 insertions, 1 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 51ba85f8e8..525c55b5fe 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
@@ -478,7 +478,15 @@ public class BuildViewTest extends BuildViewTestBase {
     assertContainsEvent("and referenced by '//foo:bad'");
     assertContainsEvent("in sh_library rule //foo");
     assertContainsEvent("cycle in dependency graph");
-    assertEventCount(3, eventCollector);
+    // Dynamic configurations trigger this error both in configuration trimming (which visits
+    // the transitive target closure) and in the normal configured target cycle detection path.
+    // So we get an additional instance of this check (which varies depending on whether Skyframe
+    // loading phase is enabled).
+    // TODO(gregce): refactor away this variation. Note that the duplicate doesn't make it into
+    // real user output (it only affects tests).
+    if (!getTargetConfiguration().useDynamicConfigurations()) {
+      assertEventCount(3, eventCollector);
+    }
   }
 
   @Test