From 2f5ca7706f3d9a8a7e32e8bb2f689cf763374836 Mon Sep 17 00:00:00 2001 From: Greg Estren Date: Fri, 17 Jun 2016 00:02:06 +0000 Subject: 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 --- .../com/google/devtools/build/lib/analysis/BuildViewTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/test/java/com/google') 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 -- cgit v1.2.3