Give MemoizingEvaluatorTest subclasses a bit more control over how transient errors are used in some of the test cases.

RELNOTES: None
PiperOrigin-RevId: 159275483

1 files changed, 47 insertions, 23 deletions

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 314d4ac05d..7e21d9ee18 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -1173,11 +1173,9 @@ public class MemoizingEvaluatorTest {
     }
   }
 
-  /**
-   * {@link ParallelEvaluator} can be configured to not store errors alongside recovered values.
-   * In that case, transient errors that are recovered from do not make the parent transient.
-   */
-  protected void parentOfCycleAndTransientInternal(boolean errorsStoredAlongsideValues)
+  /** {@link ParallelEvaluator} can be configured to not store errors alongside recovered values. */
+  protected void parentOfCycleAndErrorInternal(
+      boolean errorsStoredAlongsideValues, boolean useTransientError)
       throws Exception {
     initializeTester();
     SkyKey cycleKey1 = GraphTester.toSkyKey("cycleKey1");
@@ -1186,7 +1184,12 @@ public class MemoizingEvaluatorTest {
     SkyKey errorKey = GraphTester.toSkyKey("errorKey");
     tester.getOrCreate(cycleKey1).addDependency(cycleKey2).setComputedValue(COPY);
     tester.getOrCreate(cycleKey2).addDependency(cycleKey1).setComputedValue(COPY);
-    tester.getOrCreate(errorKey).setHasTransientError(true);
+    TestFunction errorFunction = tester.getOrCreate(errorKey);
+    if (useTransientError) {
+      errorFunction.setHasTransientError(true);
+    } else {
+      errorFunction.setHasError(true);
+    }
     tester.getOrCreate(mid).addErrorDependency(errorKey, new StringValue("recovered"))
         .setComputedValue(COPY);
     SkyKey top = GraphTester.toSkyKey("top");
@@ -1198,12 +1201,18 @@ public class MemoizingEvaluatorTest {
     ErrorInfo errorInfo = evalResult.getError(top);
     assertThat(topEvaluated.getCount()).isEqualTo(1);
     if (errorsStoredAlongsideValues) {
-      // The parent should be transitively transient, since it transitively depends on a transient
-      // error.
-      assertThat(errorInfo.isTransitivelyTransient()).isTrue();
+      if (useTransientError) {
+        // The parent should be transitively transient, since it transitively depends on a transient
+        // error.
+        assertThat(errorInfo.isTransitivelyTransient()).isTrue();
+      } else {
+        assertThatErrorInfo(errorInfo).isNotTransient();
+      }
       assertThat(errorInfo.getException()).hasMessage(NODE_TYPE.getName() + ":errorKey");
       assertThat(errorInfo.getRootCauseOfException()).isEqualTo(errorKey);
     } else {
+      // When errors are not stored alongside values, transient errors that are recovered from do
+      // not make the parent transient
       assertThatErrorInfo(errorInfo).isNotTransient();
       assertThatErrorInfo(errorInfo).hasExceptionThat().isNull();
     }
@@ -1222,8 +1231,9 @@ public class MemoizingEvaluatorTest {
   }
 
   @Test
-  public void parentOfCycleAndTransient() throws Exception {
-    parentOfCycleAndTransientInternal(/*errorsStoredAlongsideValues=*/ true);
+  public void parentOfCycleAndError() throws Exception {
+    parentOfCycleAndErrorInternal(
+        /*errorsStoredAlongsideValues=*/ true, /*useTransientError=*/ true);
   }
 
   /**
@@ -3862,32 +3872,46 @@ public class MemoizingEvaluatorTest {
     assertThat(newVal).isEqualTo(val);
   }
 
-  @Test
-  public void persistentErrorsNotRerun() throws Exception {
+  protected void runTestPersistentErrorsNotRerun(boolean includeTransientError) throws Exception {
     SkyKey topKey = GraphTester.toSkyKey("top");
     SkyKey transientErrorKey = GraphTester.toSkyKey("transientError");
     SkyKey persistentErrorKey1 = GraphTester.toSkyKey("persistentError1");
     SkyKey persistentErrorKey2 = GraphTester.toSkyKey("persistentError2");
 
-    tester.getOrCreate(topKey)
-          .addErrorDependency(transientErrorKey, new StringValue("doesn't matter"))
+    TestFunction topFunction = tester.getOrCreate(topKey)
           .addErrorDependency(persistentErrorKey1, new StringValue("doesn't matter"))
           .setHasError(true);
     tester.getOrCreate(persistentErrorKey1).setHasError(true);
-    tester.getOrCreate(transientErrorKey)
-          .addErrorDependency(persistentErrorKey2, new StringValue("doesn't matter"))
-          .setHasTransientError(true);
+    if (includeTransientError) {
+      topFunction.addErrorDependency(transientErrorKey, new StringValue("doesn't matter"));
+      tester.getOrCreate(transientErrorKey)
+            .addErrorDependency(persistentErrorKey2, new StringValue("doesn't matter"))
+            .setHasTransientError(true);
+    }
     tester.getOrCreate(persistentErrorKey2).setHasError(true);
 
     tester.evalAndGetError(topKey);
-    assertThat(tester.getEnqueuedValues()).containsExactly(
-        topKey, transientErrorKey, persistentErrorKey1, persistentErrorKey2);
+    if (includeTransientError) {
+      assertThat(tester.getEnqueuedValues()).containsExactly(
+          topKey, transientErrorKey, persistentErrorKey1, persistentErrorKey2);
+    } else {
+      assertThat(tester.getEnqueuedValues()).containsExactly(topKey, persistentErrorKey1);
+    }
 
     tester.invalidate();
     tester.invalidateTransientErrors();
     tester.evalAndGetError(topKey);
-    // TODO(bazel-team): We can do better here once we implement change pruning for errors.
-    assertThat(tester.getEnqueuedValues()).containsExactly(topKey, transientErrorKey);
+    if (includeTransientError) {
+      // TODO(bazel-team): We can do better here once we implement change pruning for errors.
+      assertThat(tester.getEnqueuedValues()).containsExactly(topKey, transientErrorKey);
+    } else {
+      assertThat(tester.getEnqueuedValues()).isEmpty();
+    }
+  }
+
+  @Test
+  public void persistentErrorsNotRerun() throws Exception {
+    runTestPersistentErrorsNotRerun(/*includeTransientError=*/ true);
   }
 
   /**
@@ -4431,7 +4455,7 @@ public class MemoizingEvaluatorTest {
   @Test
   public void errorChanged() throws Exception {
     SkyKey error = GraphTester.skyKey("error");
-    tester.getOrCreate(error).setHasTransientError(true);
+    tester.getOrCreate(error).setHasError(true);
     assertThatErrorInfo(tester.evalAndGetError(error)).hasExceptionThat().isNotNull();
     tester.getOrCreate(error, /*markAsModified=*/ true);
     tester.invalidate();