Move to using "well-known" SkyFunctionNames in tests exercising graph

--
MOS_MIGRATED_REVID=105539869

5 files changed, 26 insertions, 14 deletions

diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
index 59ff33ad48..6dc31eeaf6 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java
@@ -1513,8 +1513,8 @@ public final class ParallelEvaluator implements Evaluator {
     // graph stack.
     while (!toVisit.isEmpty()) {
       SkyKey key = toVisit.pop();
-      NodeEntry entry = graph.get(key);
 
+      NodeEntry entry;
       if (key == CHILDREN_FINISHED) {
         // A marker node means we are done with all children of a node. Since all nodes have
         // errors, we must have found errors in the children when that happens.
@@ -1548,6 +1548,8 @@ public final class ParallelEvaluator implements Evaluator {
         SkyFunctionEnvironment env = new SkyFunctionEnvironment(key, directDeps, visitor);
         env.setError(ErrorInfo.fromChildErrors(key, errorDeps));
         env.commit(/*enqueueParents=*/false);
+      } else {
+        entry = graph.get(key);
       }
 
       Preconditions.checkNotNull(entry, key);
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
index a214bd33de..af7474c506 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionName.java
@@ -38,6 +38,13 @@ public final class SkyFunctionName implements Serializable {
               }
             });
 
+  /**
+   * A well-known key type intended for testing only. The associated SkyKey should have a String
+   * argument.
+   */
+  // Needs to be after the cache is initialized.
+  public static final SkyFunctionName FOR_TESTING = SkyFunctionName.create("FOR_TESTING");
+
   /** Create a SkyFunctionName identified by {@code name}. */
   public static SkyFunctionName create(String name) {
     try {
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 62d5fad9e6..3eb94a3f1b 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java
@@ -47,8 +47,7 @@ import java.util.concurrent.TimeUnit;
 /** Base class for concurrency sanity tests on {@link EvaluableGraph} implementations. */
 public abstract class GraphConcurrencyTest {
 
-  private static final SkyFunctionName SKY_FUNCTION_NAME =
-      SkyFunctionName.create("GraphConcurrencyTestKey");
+  private static final SkyFunctionName SKY_FUNCTION_NAME = SkyFunctionName.FOR_TESTING;
   private ProcessableGraph graph;
   private TestRunnableWrapper wrapper;
 
diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphTester.java b/src/test/java/com/google/devtools/build/skyframe/GraphTester.java
index 04d76db919..f7c2da4989 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphTester.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphTester.java
@@ -45,7 +45,7 @@ import javax.annotation.Nullable;
  */
 public class GraphTester {
 
-  public static final SkyFunctionName NODE_TYPE = SkyFunctionName.create("Type");
+  public static final SkyFunctionName NODE_TYPE = SkyFunctionName.FOR_TESTING;
 
   private final Map<SkyKey, TestFunction> values = new HashMap<>();
   private final Set<SkyKey> modifiedValues = new LinkedHashSet<>();
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 550fea0773..a265c76500 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -654,7 +654,7 @@ public class MemoizingEvaluatorTest {
 
     EvaluationResult<StringValue> result = tester.eval(/*keep_going=*/false, values);
     for (int i = 0; i < values.length; i++) {
-      SkyValue actual = result.get(new SkyKey(GraphTester.NODE_TYPE, values[i]));
+      SkyValue actual = result.get(toSkyKey(values[i]));
       assertEquals(new StringValue("leaf"), actual);
     }
 
@@ -663,7 +663,7 @@ public class MemoizingEvaluatorTest {
       tester.invalidate();
       result = tester.eval(/*keep_going=*/false, values);
       for (int i = 0; i < values.length; i++) {
-        SkyValue actual = result.get(new SkyKey(GraphTester.NODE_TYPE, values[i]));
+        SkyValue actual = result.get(toSkyKey(values[i]));
         assertEquals("Run " + j + ", value " + i, new StringValue("other" + j), actual);
       }
     }
@@ -679,7 +679,7 @@ public class MemoizingEvaluatorTest {
       tester.set(values[i], new StringValue(values[i]));
       expected.append(values[i]);
     }
-    SkyKey rootKey = new SkyKey(GraphTester.NODE_TYPE, "root");
+    SkyKey rootKey = toSkyKey("root");
     TestFunction value = tester.getOrCreate(rootKey)
         .setComputedValue(CONCATENATE);
     for (int i = 0; i < values.length; i++) {
@@ -928,7 +928,7 @@ public class MemoizingEvaluatorTest {
         .that(errorInfo.getCycleInfo())
         .containsExactly(
             new CycleInfo(ImmutableList.of(top), ImmutableList.of(cycleKey1, cycleKey2)));
-    assertThat(errorInfo.getException()).hasMessage("Type:errorKey");
+    assertThat(errorInfo.getException()).hasMessage(NODE_TYPE.getName() + ":errorKey");
     assertThat(errorInfo.getRootCauseOfException()).isEqualTo(errorKey);
   }
 
@@ -1895,7 +1895,7 @@ public class MemoizingEvaluatorTest {
       tester.set(values[i], new StringValue(valueName));
       expected.append(valueName);
     }
-    SkyKey topKey = new SkyKey(GraphTester.NODE_TYPE, "top");
+    SkyKey topKey = toSkyKey("top");
     TestFunction value = tester.getOrCreate(topKey)
         .setComputedValue(CONCATENATE);
     for (int i = 0; i < values.length; i++) {
@@ -3145,7 +3145,9 @@ public class MemoizingEvaluatorTest {
       tester.evalAndGet("value");
       Assert.fail("injection over value with deps should have failed");
     } catch (IllegalStateException e) {
-      assertEquals("existing entry for Type:value has deps: [Type:other]", e.getMessage());
+      assertThat(e).hasMessage(
+          "existing entry for " + NODE_TYPE.getName() + ":value has deps: "
+              + "[" + NODE_TYPE.getName() + ":other]");
     }
   }
 
@@ -3162,7 +3164,9 @@ public class MemoizingEvaluatorTest {
       tester.evalAndGet("value");
       Assert.fail("injection over value with deps should have failed");
     } catch (IllegalStateException e) {
-      assertEquals("existing entry for Type:value has deps: [Type:other]", e.getMessage());
+      assertThat(e).hasMessage(
+          "existing entry for " + NODE_TYPE.getName() + ":value has deps: "
+              + "[" + NODE_TYPE.getName() + ":other]");
     }
   }
 
@@ -3648,7 +3652,7 @@ public class MemoizingEvaluatorTest {
 
     public SkyValue evalAndGet(boolean keepGoing, String key)
         throws InterruptedException {
-      return evalAndGet(keepGoing, new SkyKey(NODE_TYPE, key));
+      return evalAndGet(keepGoing, toSkyKey(key));
     }
 
     public SkyValue evalAndGet(String key) throws InterruptedException {
@@ -3671,7 +3675,7 @@ public class MemoizingEvaluatorTest {
     }
 
     public ErrorInfo evalAndGetError(String key) throws InterruptedException {
-      return evalAndGetError(new SkyKey(NODE_TYPE, key));
+      return evalAndGetError(toSkyKey(key));
     }
 
     @Nullable
@@ -3681,7 +3685,7 @@ public class MemoizingEvaluatorTest {
 
     @Nullable
     public SkyValue getExistingValue(String key) {
-      return getExistingValue(new SkyKey(NODE_TYPE, key));
+      return getExistingValue(toSkyKey(key));
     }
   }
 }