Don't remove a reverse dep after it was added in GraphConcurrencyTest. That never happens during evaluation. Also make sure node entries exist when we add them as reverse deps.

--
MOS_MIGRATED_REVID=106908097

2 files changed, 10 insertions, 8 deletions

diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
index 7995daf008..3711d4b854 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
@@ -123,6 +123,11 @@ public abstract class GraphConcurrencyTest {
     ExecutorService pool = Executors.newFixedThreadPool(numThreads);
     // Add single rdep before transition to done.
     assertEquals(DependencyState.NEEDS_SCHEDULING, entry.addReverseDepAndCheckIfDone(key("rdep")));
+    List<SkyKey> rdepKeys = new ArrayList<>();
+    for (int i = 0; i < numKeys; i++) {
+      rdepKeys.add(key("rdep" + i));
+    }
+    graph.createIfAbsentBatch(rdepKeys);
     for (int i = 0; i < numKeys; i++) {
       final int j = i;
       Runnable r =
@@ -158,11 +163,10 @@ public abstract class GraphConcurrencyTest {
     waitForAddedRdep.await(TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS);
     entry.setValue(new StringValue("foo1"), startingVersion);
     waitForSetValue.countDown();
-    entry.removeReverseDep(key("rdep"));
     wrapper.waitForTasksAndMaybeThrow();
     assertFalse(ExecutorUtil.interruptibleShutdown(pool));
     assertEquals(new StringValue("foo1"), graph.get(key).getValue());
-    assertEquals(numKeys, Iterables.size(graph.get(key).getReverseDeps()));
+    assertEquals(numKeys + 1, Iterables.size(graph.get(key).getReverseDeps()));
 
     graph = getGraph(startingVersion.next());
     NodeEntry sameEntry = Preconditions.checkNotNull(graph.get(key));
@@ -172,7 +176,7 @@ public abstract class GraphConcurrencyTest {
     sameEntry.markRebuildingAndGetAllRemainingDirtyDirectDeps();
     sameEntry.setValue(new StringValue("foo2"), startingVersion.next());
     assertEquals(new StringValue("foo2"), graph.get(key).getValue());
-    assertEquals(numKeys, Iterables.size(graph.get(key).getReverseDeps()));
+    assertEquals(numKeys + 1, Iterables.size(graph.get(key).getReverseDeps()));
   }
 
   // Tests adding inflight nodes with a given key while an existing node with the same key
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 a26c2af9b7..33d338c222 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -80,7 +80,7 @@ import javax.annotation.Nullable;
 @RunWith(JUnit4.class)
 public class MemoizingEvaluatorTest {
 
-  private MemoizingEvaluatorTester tester;
+  protected MemoizingEvaluatorTester tester;
   private EventCollector eventCollector;
   private EventHandler reporter;
   protected MemoizingEvaluator.EmittedEventState emittedEventState;
@@ -3681,10 +3681,8 @@ public class MemoizingEvaluatorTest {
     assertThat(tester.evalAndGet(/*keepGoing=*/ true, inactiveKey)).isEqualTo(val);
   }
 
-  /**
-   * A graph tester that is specific to the memoizing evaluator, with some convenience methods.
-   */
-  private class MemoizingEvaluatorTester extends GraphTester {
+  /** A graph tester that is specific to the memoizing evaluator, with some convenience methods. */
+  protected class MemoizingEvaluatorTester extends GraphTester {
     private RecordingDifferencer differencer;
     private MemoizingEvaluator evaluator;
     private BuildDriver driver;