Fix keep-going build with an existing cycle by only setting errorDepKey in a non-keep-going build. Setting errorDepKey was only mostly harmless, to mix memes. (Actually, it was quite harmful.)

--
MOS_MIGRATED_REVID=118410594

3 files changed, 87 insertions, 0 deletions

diff --git a/src/test/java/com/google/devtools/build/skyframe/CycleInfoSubject.java b/src/test/java/com/google/devtools/build/skyframe/CycleInfoSubject.java
new file mode 100644
index 0000000000..3598909e11
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skyframe/CycleInfoSubject.java
@@ -0,0 +1,39 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.skyframe;
+
+import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.IterableSubject;
+import com.google.common.truth.Subject;
+import com.google.common.truth.Truth;
+
+import javax.annotation.Nullable;
+
+/**
+ * {@link Subject} for {@link CycleInfo}. Please add to this class if you need more functionality!
+ */
+public class CycleInfoSubject extends Subject<CycleInfoSubject, CycleInfo> {
+  CycleInfoSubject(FailureStrategy failureStrategy, @Nullable CycleInfo cycleInfo) {
+    super(failureStrategy, cycleInfo);
+  }
+
+  public IterableSubject hasPathToCycleThat() {
+    return Truth.assertThat(getSubject().getPathToCycle())
+        .named("Path to cycle in " + getDisplaySubject());
+  }
+
+  public IterableSubject hasCycleThat() {
+    return Truth.assertThat(getSubject().getCycle()).named("Cycle in " + getDisplaySubject());
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/skyframe/CycleInfoSubjectFactory.java b/src/test/java/com/google/devtools/build/skyframe/CycleInfoSubjectFactory.java
new file mode 100644
index 0000000000..45d9d218ad
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/skyframe/CycleInfoSubjectFactory.java
@@ -0,0 +1,30 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.skyframe;
+
+import com.google.common.truth.FailureStrategy;
+import com.google.common.truth.SubjectFactory;
+import com.google.common.truth.Truth;
+
+/** {@link SubjectFactory} for {@link CycleInfo}, providing {@link CycleInfoSubject}. */
+public class CycleInfoSubjectFactory extends SubjectFactory<CycleInfoSubject, CycleInfo> {
+  public static CycleInfoSubject assertThat(CycleInfo cycleInfo) {
+    return Truth.assertAbout(new CycleInfoSubjectFactory()).that(cycleInfo);
+  }
+
+  @Override
+  public CycleInfoSubject getSubject(FailureStrategy failureStrategy, CycleInfo cycleInfo) {
+    return new CycleInfoSubject(failureStrategy, cycleInfo);
+  }
+}
diff --git a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
index fb9bc023fc..b50aea806c 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -907,6 +907,24 @@ public class MemoizingEvaluatorTest {
     assertThat(cycleInfo.getPathToCycle()).containsExactly(topKey, midKey).inOrder();
   }
 
+  @Test
+  public void keepGoingCycleAlreadyPresent() throws Exception {
+    SkyKey selfEdge = GraphTester.toSkyKey("selfEdge");
+    tester.getOrCreate(selfEdge).addDependency(selfEdge).setComputedValue(CONCATENATE);
+    EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ true, selfEdge);
+    assertThatEvaluationResult(result).hasError();
+    CycleInfo cycleInfo = Iterables.getOnlyElement(result.getError(selfEdge).getCycleInfo());
+    CycleInfoSubjectFactory.assertThat(cycleInfo).hasCycleThat().containsExactly(selfEdge);
+    CycleInfoSubjectFactory.assertThat(cycleInfo).hasPathToCycleThat().isEmpty();
+    SkyKey parent = GraphTester.toSkyKey("parent");
+    tester.getOrCreate(parent).addDependency(selfEdge).setComputedValue(CONCATENATE);
+    EvaluationResult<StringValue> result2 = tester.eval(/*keepGoing=*/ true, parent);
+    assertThatEvaluationResult(result).hasError();
+    CycleInfo cycleInfo2 = Iterables.getOnlyElement(result2.getError(parent).getCycleInfo());
+    CycleInfoSubjectFactory.assertThat(cycleInfo2).hasCycleThat().containsExactly(selfEdge);
+    CycleInfoSubjectFactory.assertThat(cycleInfo2).hasPathToCycleThat().containsExactly(parent);
+  }
+
   private void changeCycle(boolean keepGoing) throws Exception {
     initializeTester();
     SkyKey aKey = GraphTester.toSkyKey("a");