Remove all root cause data from ErrorInfo, both the "principal" root cause and the nested set. PiperOrigin-RevId: 342703936
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java index 6720f51..48a0d1d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java
@@ -163,8 +163,7 @@ + ", SkyKey: " + firstSkyKeyAndException.getFirst(), transitiveExceptions, - catastrophic), - skyKey); + catastrophic)); } // This should only happen when all error handling is done. @@ -242,8 +241,8 @@ private final boolean catastrophic; - ArtifactNestedSetFunctionException(ArtifactNestedSetEvalException e, SkyKey child) { - super(e, child); + ArtifactNestedSetFunctionException(ArtifactNestedSetEvalException e) { + super(e, Transience.PERSISTENT); this.catastrophic = e.isCatastrophic(); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 956a4d7..91f0ae3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -2173,11 +2173,14 @@ EvaluationResult<SkyValue> newlyLoaded = evaluateSkyKeys(eventHandler, buildSettingPackageKeys, true); if (newlyLoaded.hasError()) { + Map.Entry<SkyKey, ErrorInfo> errorEntry = + Preconditions.checkNotNull( + Iterables.getFirst(newlyLoaded.errorMap().entrySet(), null), newlyLoaded); throw new TransitionException( new NoSuchPackageException( - ((PackageValue.Key) newlyLoaded.getError().getRootCauseOfException()).argument(), + ((PackageValue.Key) errorEntry.getKey()).argument(), "Unable to find build setting package", - newlyLoaded.getError().getException())); + errorEntry.getValue().getException())); } buildSettingPackageKeys.forEach( k -> buildSettingPackages.put(k, (PackageValue) newlyLoaded.get(k))); @@ -2572,16 +2575,16 @@ throw new BuildFileContainsErrorsException( pkgName, "Cycle encountered while loading package " + pkgName); } - Throwable e = error.getException(); + Throwable e = Preconditions.checkNotNull(error.getException(), "%s %s", pkgName, error); // PackageFunction should be catching, swallowing, and rethrowing all transitive // errors as NoSuchPackageExceptions or constructing packages with errors, since we're in // keep_going mode. - Throwables.propagateIfInstanceOf(e, NoSuchPackageException.class); + Throwables.throwIfInstanceOf(e, NoSuchPackageException.class); throw new IllegalStateException( "Unexpected Exception type from PackageValue for '" + pkgName - + "'' with root causes: " - + error.getRootCauses().toList().toString(), + + "'' with error: " + + error, e); } return result.get(key).getPackage();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index d6c9c02..333c9bf 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -410,11 +410,7 @@ return (NoSuchPackageException) e; } throw new IllegalStateException( - "Unexpected Exception type from PackageValue for '" - + pkgId - + "'' with root causes: " - + error.getRootCauses().toList().toString(), - e); + "Unexpected Exception type from PackageValue for '" + pkgId + "'' with error: " + error, e); } private BuildDriver makeFreshDriver() {
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java index 50578ef..13a72dc 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractExceptionalParallelEvaluator.java
@@ -485,7 +485,7 @@ parent, parentEntry.getTemporaryDirectDeps(), bubbleErrorInfo, - ImmutableSet.<SkyKey>of(), + ImmutableSet.of(), evaluatorContext); externalInterrupt = externalInterrupt || Thread.currentThread().isInterrupted(); boolean completedRun = false; @@ -504,21 +504,19 @@ // Clear interrupted status. We're not listening to interrupts here. Thread.interrupted(); ReifiedSkyFunctionException reifiedBuilderException = - new ReifiedSkyFunctionException(builderException, parent); - if (reifiedBuilderException.getRootCauseSkyKey().equals(parent)) { - error = - ErrorInfo.fromException(reifiedBuilderException, /*isTransitivelyTransient=*/ false); - Pair<NestedSet<TaggedEvents>, NestedSet<Postable>> eventsAndPostables = - env.buildAndReportEventsAndPostables(parentEntry, /*expectDoneDeps=*/ false); - ValueWithMetadata valueWithMetadata = - ValueWithMetadata.error( - ErrorInfo.fromChildErrors(errorKey, ImmutableSet.of(error)), - eventsAndPostables.first, - eventsAndPostables.second); - replay(valueWithMetadata); - bubbleErrorInfo.put(errorKey, valueWithMetadata); - continue; - } + new ReifiedSkyFunctionException(builderException); + error = + ErrorInfo.fromException(reifiedBuilderException, /*isTransitivelyTransient=*/ false); + Pair<NestedSet<TaggedEvents>, NestedSet<Postable>> eventsAndPostables = + env.buildAndReportEventsAndPostables(parentEntry, /*expectDoneDeps=*/ false); + ValueWithMetadata valueWithMetadata = + ValueWithMetadata.error( + ErrorInfo.fromChildErrors(errorKey, ImmutableSet.of(error)), + eventsAndPostables.first, + eventsAndPostables.second); + replay(valueWithMetadata); + bubbleErrorInfo.put(errorKey, valueWithMetadata); + continue; } finally { // Clear interrupted status. We're not listening to interrupts here. Thread.interrupted();
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java index 6332b15..cb3def0 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
@@ -491,15 +491,13 @@ } } catch (final SkyFunctionException builderException) { ReifiedSkyFunctionException reifiedBuilderException = - new ReifiedSkyFunctionException(builderException, skyKey); + new ReifiedSkyFunctionException(builderException); // In keep-going mode, we do not let SkyFunctions complete with a thrown error if they // have missing deps. Instead, we wait until their deps are done and restart the // SkyFunction, so we can have a definitive error and definitive graph structure, thus // avoiding non-determinism. It's completely reasonable for SkyFunctions to throw eagerly // because they do not know if they are in keep-going mode. - // Propagated transitive errors are treated the same as missing deps. - if ((!evaluatorContext.keepGoing() || !env.valuesMissing()) - && reifiedBuilderException.getRootCauseSkyKey().equals(skyKey)) { + if (!evaluatorContext.keepGoing() || !env.valuesMissing()) { boolean shouldFailFast = !evaluatorContext.keepGoing() || builderException.isCatastrophic(); if (shouldFailFast) {
diff --git a/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java b/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java index 409e8eb..141c47e 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java +++ b/src/main/java/com/google/devtools/build/skyframe/ErrorInfo.java
@@ -17,9 +17,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.collect.nestedset.NestedSet; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException; import java.util.Collection; import java.util.Objects; @@ -33,15 +30,12 @@ public class ErrorInfo { /** Create an ErrorInfo from a {@link ReifiedSkyFunctionException}. */ - public static ErrorInfo fromException(ReifiedSkyFunctionException skyFunctionException, - boolean isTransitivelyTransient) { - SkyKey rootCauseSkyKey = skyFunctionException.getRootCauseSkyKey(); + public static ErrorInfo fromException( + ReifiedSkyFunctionException skyFunctionException, boolean isTransitivelyTransient) { Exception rootCauseException = skyFunctionException.getCause(); return new ErrorInfo( - NestedSetBuilder.create(Order.STABLE_ORDER, rootCauseSkyKey), Preconditions.checkNotNull(rootCauseException, "Cause is null"), - rootCauseSkyKey, - /*cycles=*/ ImmutableList.<CycleInfo>of(), + /*cycles=*/ ImmutableList.of(), skyFunctionException.isTransient(), isTransitivelyTransient || skyFunctionException.isTransient(), skyFunctionException.isCatastrophic()); @@ -50,9 +44,7 @@ /** Create an ErrorInfo from a {@link CycleInfo}. */ public static ErrorInfo fromCycle(CycleInfo cycleInfo) { return new ErrorInfo( - /*rootCauses=*/ NestedSetBuilder.<SkyKey>emptySet(Order.STABLE_ORDER), /*exception=*/ null, - /*rootCauseOfException=*/ null, ImmutableList.of(cycleInfo), /*isDirectlyTransient=*/ false, /*isTransitivelyTransient=*/ false, @@ -65,38 +57,29 @@ Preconditions.checkState( !childErrors.isEmpty(), "childErrors may not be empty %s", currentValue); - NestedSetBuilder<SkyKey> rootCausesBuilder = NestedSetBuilder.stableOrder(); ImmutableList.Builder<CycleInfo> cycleBuilder = ImmutableList.builder(); Exception firstException = null; - SkyKey firstChildKey = null; boolean isTransitivelyTransient = false; boolean isCatastrophic = false; for (ErrorInfo child : childErrors) { if (firstException == null) { // Arbitrarily pick the first error. firstException = child.getException(); - firstChildKey = child.getRootCauseOfException(); } - rootCausesBuilder.addTransitive(child.rootCauses); cycleBuilder.addAll(CycleInfo.prepareCycles(currentValue, child.cycles)); isTransitivelyTransient |= child.isTransitivelyTransient(); isCatastrophic |= child.isCatastrophic(); } return new ErrorInfo( - rootCausesBuilder.build(), firstException, - firstChildKey, cycleBuilder.build(), /*isDirectlyTransient=*/ false, isTransitivelyTransient, isCatastrophic); } - private final NestedSet<SkyKey> rootCauses; - @Nullable private final Exception exception; - private final SkyKey rootCauseOfException; private final ImmutableList<CycleInfo> cycles; @@ -105,22 +88,15 @@ private final boolean isCatastrophic; public ErrorInfo( - NestedSet<SkyKey> rootCauses, @Nullable Exception exception, - SkyKey rootCauseOfException, ImmutableList<CycleInfo> cycles, boolean isDirectlyTransient, boolean isTransitivelyTransient, boolean isCatastrophic) { Preconditions.checkState(exception != null || !Iterables.isEmpty(cycles), "At least one of exception and cycles must be non-null/empty, respectively"); - Preconditions.checkState((exception == null) == (rootCauseOfException == null), - "exception and rootCauseOfException must both be null or non-null, got %s %s", - exception, rootCauseOfException); - this.rootCauses = rootCauses; this.exception = exception; - this.rootCauseOfException = rootCauseOfException; this.cycles = cycles; this.isDirectlyTransient = isDirectlyTransient; this.isTransitivelyTransient = isTransitivelyTransient; @@ -137,14 +113,6 @@ } ErrorInfo other = (ErrorInfo) obj; - if (rootCauses != other.rootCauses) { - if (rootCauses == null || other.rootCauses == null) { - return false; - } - if (!rootCauses.shallowEquals(other.rootCauses)) { - return false; - } - } if (!Objects.equals(cycles, other.cycles)) { return false; @@ -165,10 +133,6 @@ } } - if (!Objects.equals(rootCauseOfException, other.rootCauseOfException)) { - return false; - } - return isDirectlyTransient == other.isDirectlyTransient && isTransitivelyTransient == other.isTransitivelyTransient && isCatastrophic == other.isCatastrophic; @@ -179,38 +143,24 @@ return Objects.hash( exception == null ? null : exception.getClass(), exception == null ? "" : exception.getMessage(), - rootCauseOfException, cycles, isDirectlyTransient, isTransitivelyTransient, - isCatastrophic, - rootCauses == null ? 0 : rootCauses.shallowHashCode()); + isCatastrophic); } @Override public String toString() { return MoreObjects.toStringHelper(this) .add("exception", exception) - .add("rootCauses", rootCauses) .add("cycles", cycles) .add("isCatastrophic", isCatastrophic) - .add("rootCauseOfException", rootCauseOfException) .add("isDirectlyTransient", isDirectlyTransient) .add("isTransitivelyTransient", isTransitivelyTransient) .toString(); } /** - * The root causes of a value that failed to build are its descendant values that failed to build. - * If a value's descendants all built successfully, but it failed to, its root cause will be - * itself. If a value depends on a cycle, but has no other errors, this method will return the - * empty set. - */ - public NestedSet<SkyKey> getRootCauses() { - return rootCauses; - } - - /** * The exception thrown when building a value. May be null if value's only error is depending * on a cycle. * @@ -222,10 +172,6 @@ return exception; } - public SkyKey getRootCauseOfException() { - return rootCauseOfException; - } - /** * Any cycles found when building this value. *
diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java b/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java index f9fc32a..84a69db 100644 --- a/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java +++ b/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java
@@ -87,19 +87,16 @@ } /** - * Returns {@link Map} of {@link SkyKey}s to {@link ErrorInfo}. Note that currently some - * of the returned SkyKeys may not be the ones requested by the user. Moreover, the SkyKey - * is not necessarily the cause of the error -- it is just the value that was being evaluated - * when the error was discovered. For the cause of the error, use - * {@link ErrorInfo#getRootCauses()} on each ErrorInfo. + * Returns {@link Map} of {@link SkyKey}s to {@link ErrorInfo}. Note that currently some of the + * returned SkyKeys may not be the ones requested by the user. Moreover, the SkyKey is not + * necessarily the cause of the error -- it is just the value that was being evaluated when the + * error was discovered. */ public Map<SkyKey, ErrorInfo> errorMap() { return ImmutableMap.copyOf(errorMap); } - /** - * @param key {@link SkyKey} to get {@link ErrorInfo} for. - */ + /** Returns {@link ErrorInfo} for given {@code key} which must be present in errors. */ public ErrorInfo getError(SkyKey key) { return Preconditions.checkNotNull(errorMap, key).get(key); }
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java index 725b5e1..f2106aa 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionException.java
@@ -14,7 +14,6 @@ package com.google.devtools.build.skyframe; import com.google.common.base.Preconditions; -import javax.annotation.Nullable; /** * Base class of exceptions thrown by {@link SkyFunction#compute} on failure. @@ -60,27 +59,11 @@ } private final Transience transience; - @Nullable private final SkyKey rootCause; public SkyFunctionException(Exception cause, Transience transience) { - this(cause, transience, null); - } - - /** Used to rethrow a child error that the parent cannot handle. */ - public SkyFunctionException(Exception cause, SkyKey childKey) { - this(cause, Transience.PERSISTENT, childKey); - } - - private SkyFunctionException(Exception cause, Transience transience, SkyKey rootCause) { super(Preconditions.checkNotNull(cause)); SkyFunctionException.validateExceptionType(cause.getClass()); this.transience = transience; - this.rootCause = rootCause; - } - - @Nullable - public final SkyKey getRootCauseSkyKey() { - return rootCause; } public final boolean isTransient() { @@ -123,17 +106,15 @@ private final boolean isCatastrophic; private final SkyFunctionException originalException; - public ReifiedSkyFunctionException(SkyFunctionException e, SkyKey key) { - this(e, e.transience, key, e.getRootCauseSkyKey(), e.isCatastrophic()); + public ReifiedSkyFunctionException(SkyFunctionException e) { + this(e, e.transience, e.isCatastrophic()); } protected ReifiedSkyFunctionException( SkyFunctionException e, Transience transience, - SkyKey key, - @Nullable SkyKey rootCauseSkyKey, boolean isCatastrophic) { - super(e.getCause(), transience, rootCauseSkyKey == null ? key : rootCauseSkyKey); + super(e.getCause(), transience); this.isCatastrophic = isCatastrophic; this.originalException = e; }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BzlCompileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/BzlCompileFunctionTest.java index 813699f..9733087 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BzlCompileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BzlCompileFunctionTest.java
@@ -90,7 +90,6 @@ assertThat(result.hasError()).isTrue(); ErrorInfo errorInfo = result.getError(skyKey); Throwable e = errorInfo.getException(); - assertThat(errorInfo.getRootCauseOfException()).isEqualTo(skyKey); assertThat(e).isInstanceOf(NoSuchPackageException.class); assertThat(e).hasMessageThat().contains("bork"); }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java index 885b728..72a486d 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -679,7 +679,6 @@ getSkyframeExecutor(), skyKey, /*keepGoing=*/ false, reporter); assertThat(result.hasError()).isTrue(); ErrorInfo errorInfo = result.getError(skyKey); - assertThat(errorInfo.getRootCauseOfException()).isEqualTo(skyKey); assertThat(errorInfo.getException()) .hasMessageThat() .isEqualTo( @@ -1307,9 +1306,7 @@ .contains("Symlink cycle: /workspace/foo/cycle"); // And appropriate Skyframe root cause (N.B. since we want PackageFunction to rethrow in // situations like this, we want the PackageValue node to be its own root cause). - assertThatEvaluationResult(result) - .hasErrorEntryForKeyThat(pkgKey) - .rootCauseOfExceptionIs(pkgKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(pkgKey); // Then, when we modify the BUILD file so as to force package loading, scratch.overwriteFile( @@ -1347,9 +1344,6 @@ .hasExceptionThat() .hasMessageThat() .contains("Symlink cycle: /workspace/foo/cycle"); - assertThatEvaluationResult(result) - .hasErrorEntryForKeyThat(pkgKey) - .rootCauseOfExceptionIs(pkgKey); // Thus showing that clean and incremental package loading have the same semantics in the // presence of a symlink cycle encountered during glob evaluation. }
diff --git a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java index b9b1482..085f3de 100644 --- a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java +++ b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoSubject.java
@@ -40,10 +40,6 @@ return check("getCycleInfo()").that(actual.getCycleInfo()); } - public void rootCauseOfExceptionIs(SkyKey key) { - check("getRootCauseOfException()").that(actual.getRootCauseOfException()).isEqualTo(key); - } - public void isTransient() { if (!actual.isTransitivelyTransient()) { failWithActual(simpleFact("expected to be transient"));
diff --git a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java index 1298e64..b2e58f5 100644 --- a/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/ErrorInfoTest.java
@@ -18,8 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException; import java.io.IOException; import org.junit.Test; @@ -49,17 +47,14 @@ private static void runTestFromException( boolean isDirectlyTransient, boolean isTransitivelyTransient) { Exception exception = new IOException("ehhhhh"); - SkyKey causeOfException = GraphTester.toSkyKey("CAUSE, 1234"); DummySkyFunctionException dummyException = new DummySkyFunctionException(exception, isDirectlyTransient, /*isCatastrophic=*/ false); - ErrorInfo errorInfo = ErrorInfo.fromException( - new ReifiedSkyFunctionException(dummyException, causeOfException), - isTransitivelyTransient); + ErrorInfo errorInfo = + ErrorInfo.fromException( + new ReifiedSkyFunctionException(dummyException), isTransitivelyTransient); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(causeOfException); assertThat(errorInfo.getException()).isSameInstanceAs(exception); - assertThat(errorInfo.getRootCauseOfException()).isSameInstanceAs(causeOfException); assertThat(errorInfo.getCycleInfo()).isEmpty(); assertThat(errorInfo.isDirectlyTransient()).isEqualTo(isDirectlyTransient); assertThat(errorInfo.isTransitivelyTransient()).isEqualTo( @@ -96,9 +91,7 @@ ErrorInfo errorInfo = ErrorInfo.fromCycle(cycle); - assertThat(errorInfo.getRootCauses().toList()).isEmpty(); assertThat(errorInfo.getException()).isNull(); - assertThat(errorInfo.getRootCauseOfException()).isNull(); assertThat(errorInfo.isTransitivelyTransient()).isFalse(); assertThat(errorInfo.isCatastrophic()).isFalse(); } @@ -112,35 +105,29 @@ ErrorInfo cycleErrorInfo = ErrorInfo.fromCycle(cycle); Exception exception1 = new IOException("ehhhhh"); - SkyKey causeOfException1 = GraphTester.toSkyKey("CAUSE1, 1234"); DummySkyFunctionException dummyException1 = new DummySkyFunctionException(exception1, /*isTransient=*/ true, /*isCatastrophic=*/ false); - ErrorInfo exceptionErrorInfo1 = ErrorInfo.fromException( - new ReifiedSkyFunctionException(dummyException1, causeOfException1), - /*isTransitivelyTransient=*/ false); + ErrorInfo exceptionErrorInfo1 = + ErrorInfo.fromException( + new ReifiedSkyFunctionException(dummyException1), /*isTransitivelyTransient=*/ false); // N.B this ErrorInfo will be catastrophic. Exception exception2 = new IOException("blahhhhh"); - SkyKey causeOfException2 = GraphTester.toSkyKey("CAUSE2, 5678"); DummySkyFunctionException dummyException2 = new DummySkyFunctionException(exception2, /*isTransient=*/ false, /*isCatastrophic=*/ true); - ErrorInfo exceptionErrorInfo2 = ErrorInfo.fromException( - new ReifiedSkyFunctionException(dummyException2, causeOfException2), - /*isTransitivelyTransient=*/ false); + ErrorInfo exceptionErrorInfo2 = + ErrorInfo.fromException( + new ReifiedSkyFunctionException(dummyException2), /*isTransitivelyTransient=*/ false); SkyKey currentKey = GraphTester.toSkyKey("CURRENT, 9876"); ErrorInfo errorInfo = ErrorInfo.fromChildErrors( currentKey, ImmutableList.of(cycleErrorInfo, exceptionErrorInfo1, exceptionErrorInfo2)); - assertThat(errorInfo.getRootCauses().toList()) - .containsExactly(causeOfException1, causeOfException2); - // For simplicity we test the current implementation detail that we choose the first non-null - // (exception, cause) pair that we encounter. This isn't necessarily a requirement of the - // interface, but it makes the test convenient and is a way to document the current behavior. + // exception that we encounter. This isn't necessarily a requirement of the interface, but it + // makes the test convenient and is a way to document the current behavior. assertThat(errorInfo.getException()).isSameInstanceAs(exception1); - assertThat(errorInfo.getRootCauseOfException()).isSameInstanceAs(causeOfException1); assertThat(errorInfo.getCycleInfo()).containsExactly( new CycleInfo( @@ -156,37 +143,9 @@ IllegalStateException e = assertThrows( IllegalStateException.class, - () -> - new ErrorInfo( - NestedSetBuilder.<SkyKey>emptySet(Order.COMPILE_ORDER), - /*exception=*/ null, - /*rootCauseOfException=*/ null, - ImmutableList.<CycleInfo>of(), - false, - false, - false)); + () -> new ErrorInfo(/*exception=*/ null, ImmutableList.of(), false, false, false)); assertThat(e) .hasMessageThat() .isEqualTo("At least one of exception and cycles must be non-null/empty, respectively"); } - - @Test - public void testCannotCreateErrorInfoWithExceptionButNoRootCause() { - // Brittle, but confirms we failed for the right reason. - IllegalStateException e = - assertThrows( - IllegalStateException.class, - () -> - new ErrorInfo( - NestedSetBuilder.<SkyKey>emptySet(Order.COMPILE_ORDER), - new IOException("foo"), - /*rootCauseOfException=*/ null, - ImmutableList.<CycleInfo>of(), - false, - false, - false)); - assertThat(e) - .hasMessageThat() - .startsWith("exception and rootCauseOfException must both be null or non-null"); - } }
diff --git a/src/test/java/com/google/devtools/build/skyframe/EvaluationResultSubject.java b/src/test/java/com/google/devtools/build/skyframe/EvaluationResultSubject.java index 21e3d90..7e28641 100644 --- a/src/test/java/com/google/devtools/build/skyframe/EvaluationResultSubject.java +++ b/src/test/java/com/google/devtools/build/skyframe/EvaluationResultSubject.java
@@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.truth.FailureMetadata; import com.google.common.truth.IterableSubject; +import com.google.common.truth.MapSubject; import com.google.common.truth.Subject; /** @@ -64,4 +65,15 @@ return check("reverseDeps(%s)", child) .that(actual.getWalkableGraph().getReverseDeps(ImmutableList.of(child)).get(child)); } + + public MapSubject hasErrorMapThat() { + return check("errorMap()").that(actual.errorMap()); + } + + public ErrorInfoSubject hasSingletonErrorThat(SkyKey key) { + hasError(); + hasErrorMapThat().hasSize(1); + check("keyNames()").that(actual.keyNames()).isEmpty(); + return hasErrorEntryForKeyThat(key); + } }
diff --git a/src/test/java/com/google/devtools/build/skyframe/GenericFunctionException.java b/src/test/java/com/google/devtools/build/skyframe/GenericFunctionException.java index e2c67c4..389b371 100644 --- a/src/test/java/com/google/devtools/build/skyframe/GenericFunctionException.java +++ b/src/test/java/com/google/devtools/build/skyframe/GenericFunctionException.java
@@ -21,7 +21,7 @@ super(e, transience); } - public GenericFunctionException(SomeErrorException e, SkyKey childKey) { - super(e, childKey); + public GenericFunctionException(SomeErrorException e) { + this(e, Transience.PERSISTENT); } }
diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java index 012d783..7e84438 100644 --- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java
@@ -47,9 +47,8 @@ @RunWith(JUnit4.class) public class InMemoryNodeEntryTest { private static final NestedSet<TaggedEvents> NO_EVENTS = - NestedSetBuilder.<TaggedEvents>emptySet(Order.STABLE_ORDER); - private static final NestedSet<Postable> NO_POSTS = - NestedSetBuilder.<Postable>emptySet(Order.STABLE_ORDER); + NestedSetBuilder.emptySet(Order.STABLE_ORDER); + private static final NestedSet<Postable> NO_POSTS = NestedSetBuilder.emptySet(Order.STABLE_ORDER); private static SkyKey key(String name) { return GraphTester.toSkyKey(name); @@ -137,9 +136,9 @@ NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); - ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( - new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT), - key("cause")); + ReifiedSkyFunctionException exception = + new ReifiedSkyFunctionException( + new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); assertThat(setValue(entry, /*value=*/null, errorInfo, /*graphVersion=*/0L)).isEmpty(); assertThat(entry.isDone()).isTrue(); @@ -152,9 +151,9 @@ NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. entry.markRebuilding(); - ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( - new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT), - key("cause")); + ReifiedSkyFunctionException exception = + new ReifiedSkyFunctionException( + new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); setValue(entry, new SkyValue() {}, errorInfo, /*graphVersion=*/0L); assertThat(entry.isDone()).isTrue(); @@ -429,9 +428,8 @@ assertThrows( "Cannot add same dep twice", IllegalStateException.class, - () -> - // We only check for duplicates when we request all the reverse deps. - entry.getReverseDepsForDoneEntry()); + // We only check for duplicates when we request all the reverse deps. + entry::getReverseDepsForDoneEntry); } @Test @@ -446,9 +444,8 @@ assertThrows( "Cannot add same dep twice", IllegalStateException.class, - () -> - // We only check for duplicates when we request all the reverse deps. - entry.getReverseDepsForDoneEntry()); + // We only check for duplicates when we request all the reverse deps. + entry::getReverseDepsForDoneEntry); } @Test @@ -551,12 +548,15 @@ entry.signalDep(ONE_VERSION, /*childForDebugging=*/ null); assertThat(entry.getDirtyState()).isEqualTo(NodeEntry.DirtyState.NEEDS_REBUILDING); assertThatNodeEntry(entry).hasTemporaryDirectDepsThat().containsExactly(dep); - ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( - new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT), - key("cause")); + ReifiedSkyFunctionException exception = + new ReifiedSkyFunctionException( + new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); entry.markRebuilding(); - setValue(entry, new IntegerValue(5), ErrorInfo.fromException(exception, false), - /*graphVersion=*/1L); + setValue( + entry, + new IntegerValue(5), + ErrorInfo.fromException(exception, false), + /*graphVersion=*/ 1L); assertThat(entry.isDone()).isTrue(); assertWithMessage("Version increments when setValue changes") .that(entry.getVersion()) @@ -613,9 +613,9 @@ SkyKey dep = key("dep"); addTemporaryDirectDep(entry, dep); entry.signalDep(ZERO_VERSION, dep); - ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( - new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT), - key("cause")); + ReifiedSkyFunctionException exception = + new ReifiedSkyFunctionException( + new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); ErrorInfo errorInfo = ErrorInfo.fromException(exception, false); setValue(entry, /*value=*/null, errorInfo, /*graphVersion=*/0L); entry.markDirty(DirtyType.DIRTY); @@ -693,9 +693,9 @@ entry.signalDep(ZERO_VERSION, dep); // Oops! Evaluation terminated with an error, but we're going to set this entry's value anyway. entry.removeUnfinishedDeps(ImmutableSet.of(dep2, dep3, dep5)); - ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( - new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT), - key("key")); + ReifiedSkyFunctionException exception = + new ReifiedSkyFunctionException( + new GenericFunctionException(new SomeErrorException("oops"), Transience.PERSISTENT)); setValue(entry, null, ErrorInfo.fromException(exception, false), 0L); entry.markDirty(DirtyType.DIRTY); entry.addReverseDepAndCheckIfDone(null); // Restart evaluation.
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 5c327ca..27108e7 100644 --- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -144,10 +144,6 @@ return true; } - protected boolean rootCausesStored() { - return true; - } - protected boolean cyclesDetected() { return true; } @@ -284,9 +280,7 @@ tester.getOrCreate("otherValue1").setConstantValue(new StringValue("otherVal1")); EvaluationResult<SkyValue> result = tester.eval(false, "top"); - assertThat(result.hasError()).isTrue(); - assertThat(result.getError().getRootCauses().toList()).containsExactly(toSkyKey("badValue")); - assertThat(result.keyNames()).isEmpty(); + assertThatEvaluationResult(result).hasSingletonErrorThat(toSkyKey("top")); } @Test @@ -574,14 +568,15 @@ for (int i = 0; i < 2; i++) { initializeReporter(); EvaluationResult<StringValue> result = tester.eval(false, "top"); - assertThat(result.hasError()).isTrue(); - if (rootCausesStored()) { - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(topKey); - } - assertThat(result.getError(topKey).getException()) + assertThatEvaluationResult(result) + .hasSingletonErrorThat(topKey) + .hasExceptionThat() .hasMessageThat() .isEqualTo(topKey.toString()); - assertThat(result.getError(topKey).getException()).isInstanceOf(SomeErrorException.class); + assertThatEvaluationResult(result) + .hasSingletonErrorThat(topKey) + .hasExceptionThat() + .isInstanceOf(SomeErrorException.class); if (i == 0 || eventsStored()) { assertThatEvents(eventCollector).containsExactly("warn-dep"); } @@ -595,14 +590,15 @@ for (int i = 0; i < 2; i++) { initializeReporter(); EvaluationResult<StringValue> result = tester.eval(false, "top"); - assertThat(result.hasError()).isTrue(); - if (rootCausesStored()) { - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(topKey); - } - assertThat(result.getError(topKey).getException()) + assertThatEvaluationResult(result) + .hasSingletonErrorThat(topKey) + .hasExceptionThat() .hasMessageThat() .isEqualTo(topKey.toString()); - assertThat(result.getError(topKey).getException()).isInstanceOf(SomeErrorException.class); + assertThatEvaluationResult(result) + .hasSingletonErrorThat(topKey) + .hasExceptionThat() + .isInstanceOf(SomeErrorException.class); if (i == 0 || eventsStored()) { assertThatEvents(eventCollector).containsExactly("warning msg"); } @@ -616,14 +612,15 @@ for (int i = 0; i < 2; i++) { initializeReporter(); EvaluationResult<StringValue> result = tester.eval(i == 0, "top"); - assertThat(result.hasError()).isTrue(); - if (rootCausesStored()) { - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(topKey); - } - assertThat(result.getError(topKey).getException()) + assertThatEvaluationResult(result) + .hasSingletonErrorThat(topKey) + .hasExceptionThat() .hasMessageThat() .isEqualTo(topKey.toString()); - assertThat(result.getError(topKey).getException()).isInstanceOf(SomeErrorException.class); + assertThatEvaluationResult(result) + .hasSingletonErrorThat(topKey) + .hasExceptionThat() + .isInstanceOf(SomeErrorException.class); if (i == 0 || eventsStored()) { assertThatEvents(eventCollector).containsExactly("warning msg"); } @@ -764,15 +761,11 @@ tester.getOrCreate(errorKey).setHasError(true); tester.getOrCreate(topKey).addDependency(errorKey).setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ true, topKey); - assertThatEvaluationResult(result) - .hasErrorEntryForKeyThat(topKey) - .rootCauseOfExceptionIs(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); tester.getOrCreate(topKey, /*markAsModified=*/ true); tester.invalidate(); result = tester.eval(/*keepGoing=*/ false, topKey); - assertThatEvaluationResult(result) - .hasErrorEntryForKeyThat(topKey) - .rootCauseOfExceptionIs(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); } @Test @@ -1374,7 +1367,6 @@ assertThat(errorInfo.getException()) .hasMessageThat() .isEqualTo(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 @@ -1759,7 +1751,7 @@ // slowKey starts -> errorKey finishes, written to graph -> slowKey finishes & (Visitor aborts) // -> topKey builds. EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.getError().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); // Make sure midKey didn't finish building. assertThat(tester.getExistingValue(midKey)).isNull(); // Give slowKey a nice ordinary builder. @@ -1799,7 +1791,7 @@ // slowKey starts -> errorKey finishes, written to graph -> slowKey finishes & (Visitor aborts) // -> topKey builds. EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.getError().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); // Make sure midKey didn't finish building. assertThat(tester.getExistingValue(midKey)).isNull(); // Give slowKey a nice ordinary builder. @@ -1835,9 +1827,7 @@ } else { result = tester.eval(/*keepGoing=*/ false, slowFailKey, successKey); } - assertThatEvaluationResult(result) - .hasErrorEntryForKeyThat(slowFailKey) - .rootCauseOfExceptionIs(slowFailKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(slowFailKey); assertThat(result.values()).containsExactly(new StringValue("yippee")); } @@ -1879,7 +1869,7 @@ // slowKey starts -> errorKey finishes, written to graph -> slowKey finishes & (Visitor aborts) // -> topKey builds. EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.getError().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); // Make sure midKey didn't finish building. assertThat(tester.getExistingValue(midKey)).isNull(); // Give slowKey a nice ordinary builder. @@ -1941,13 +1931,10 @@ tester.getOrCreate(parentKey).addErrorDependency(errorKey, new StringValue("recovered")) .setHasError(true); // Prime the graph by putting the error value in it beforehand. - assertThat(tester.evalAndGetError(/*keepGoing=*/ true, errorKey).getRootCauses().toList()) - .containsExactly(errorKey); - EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, parentKey); + assertThat(tester.evalAndGetError(/*keepGoing=*/ true, errorKey)).isNotNull(); // Request the parent. - assertThat(result.getError(parentKey).getRootCauses().toList()) - .containsExactly(parentKey) - .inOrder(); + EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ false, parentKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parentKey); // Change the error value to no longer throw. tester.set(errorKey, new StringValue("reformed")).setHasError(false); tester.getOrCreate(parentKey, /*markAsModified=*/false).setHasError(false) @@ -1983,15 +1970,11 @@ SkyKey topKey = GraphTester.toSkyKey("top"); tester.getOrCreate(topKey).addDependency(leafKey).setHasError(true); // Build top -- it has an error. - assertThat(tester.evalAndGetError(/*keepGoing=*/ true, topKey).getRootCauses().toList()) - .containsExactly(topKey) - .inOrder(); + tester.evalAndGetError(/*keepGoing=*/ true, topKey); // Invalidate top via leaf, and rebuild. tester.set(leafKey, new StringValue("leaf2")); tester.invalidate(); - assertThat(tester.evalAndGetError(/*keepGoing=*/ true, topKey).getRootCauses().toList()) - .containsExactly(topKey) - .inOrder(); + tester.evalAndGetError(/*keepGoing=*/ true, topKey); } /** Regression test for crash bug. */ @@ -2025,11 +2008,10 @@ tester.getOrCreate(parentKey).addErrorDependency(midKey, new StringValue("don't use this")) .setComputedValue(COPY); // Prime the graph by evaluating the mid-level value. It shouldn't be stored in the graph - // because - // it was only called during the bubbling-up phase. - EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, midKey); + // because it was only called during the bubbling-up phase. + EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ false, midKey); assertThat(result.get(midKey)).isNull(); - assertThat(result.getError().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(midKey); // In a keepGoing build, midKey should be re-evaluated. assertThat(((StringValue) tester.evalAndGet(/*keepGoing=*/ true, parentKey)).getValue()) .isEqualTo("recovered"); @@ -2080,7 +2062,7 @@ // Assert that build fails and "error" really is in error. EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, topKey); assertThat(result.hasError()).isTrue(); - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(topKey); // Ensure that evaluation succeeds if errorKey does not throw an error. tester.getOrCreate(errorKey).setBuilder(null); @@ -2126,16 +2108,10 @@ }); EvaluationResult<SkyValue> evaluationResult = tester.eval(/*keepGoing=*/ true, groupDepA, depC); - assertThat(evaluationResult.hasError()).isTrue(); assertThat(((SkyKeyValue) evaluationResult.get(groupDepA)).key).isEqualTo(depC); - assertThat(evaluationResult.getError(depC).getRootCauses().toList()) - .containsExactly(depC) - .inOrder(); + assertThatEvaluationResult(evaluationResult).hasErrorEntryForKeyThat(depC); evaluationResult = tester.eval(/*keepGoing=*/false, topKey); - assertThat(evaluationResult.hasError()).isTrue(); - assertThat(evaluationResult.getError(topKey).getRootCauses().toList()) - .containsExactly(topKey) - .inOrder(); + assertThatEvaluationResult(evaluationResult).hasErrorEntryForKeyThat(topKey); tester.set(groupDepA, new SkyKeyValue(groupDepB)); tester.getOrCreate(depC, /*markAsModified=*/true); @@ -2292,18 +2268,17 @@ delayTopSignaling.set(true); result = tester.eval(/*keepGoing=*/ false, top, otherTop); if (throwError) { - assertThat(result.hasError()).isTrue(); + assertThatEvaluationResult(result).hasError(); assertThat(result.keyNames()).isEmpty(); // No successfully evaluated values. - ErrorInfo errorInfo = result.getError(top); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(firstKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(top); assertWithMessage( "on the incremental build, top's builder should have only been used in error " + "bubbling") .that(numTopInvocations.get()) .isEqualTo(3); } else { - assertThat(result.get(top)).isEqualTo(new StringValue("top")); - assertThat(result.hasError()).isFalse(); + assertThatEvaluationResult(result).hasEntryThat(top).isEqualTo(new StringValue("top")); + assertThatEvaluationResult(result).hasNoError(); assertWithMessage( "on the incremental build, top's builder should have only been executed once in " + "normal evaluation") @@ -3517,11 +3492,7 @@ tester.getOrCreate(top, /*markAsModified=*/false).setHasError(true); tester.invalidate(); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, top); - assertWithMessage("value should not have completed evaluation").that(result.get(top)).isNull(); - assertWithMessage( - "The error thrown by leaf should have been swallowed by the error thrown by top") - .that(result.getError().getRootCauses().toList()) - .containsExactly(top); + assertThatEvaluationResult(result).hasSingletonErrorThat(top); } @Test @@ -3544,11 +3515,7 @@ tester.getOrCreate(top, /*markAsModified=*/false).setHasError(true); tester.invalidate(); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, top); - assertWithMessage("value should not have completed evaluation").that(result.get(top)).isNull(); - assertWithMessage( - "The error thrown by leaf should have been swallowed by the error thrown by top") - .that(result.getError().getRootCauses().toList()) - .containsExactly(top); + assertThatEvaluationResult(result).hasSingletonErrorThat(top); } @Test @@ -3572,14 +3539,14 @@ SkyKey topKey = GraphTester.toSkyKey("top"); tester.getOrCreate(topKey).addDependency(error).setComputedValue(COPY); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(error); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); tester.getOrCreate(error).setHasError(false); StringValue val = new StringValue("reformed"); tester.set(error, val); tester.invalidate(); result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.get(topKey)).isEqualTo(val); - assertThat(result.hasError()).isFalse(); + assertThatEvaluationResult(result).hasEntryThat(topKey).isEqualTo(val); + assertThatEvaluationResult(result).hasNoError(); } /** Regression test for crash bug. */ @@ -3607,7 +3574,7 @@ tester.getOrCreate(topKey).setBuilder(errorFunction); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, topKey); tester.invalidateTransientErrors(); - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(topKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); tester.getOrCreate(error).setHasTransientError(false); StringValue reformed = new StringValue("reformed"); tester.set(error, reformed); @@ -3619,8 +3586,8 @@ tester.invalidate(); tester.invalidateTransientErrors(); result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.get(topKey)).isEqualTo(reformed); - assertThat(result.hasError()).isFalse(); + assertThatEvaluationResult(result).hasEntryThat(topKey).isEqualTo(reformed); + assertThatEvaluationResult(result).hasNoError(); } /** @@ -3705,15 +3672,15 @@ }; tester.getOrCreate(topKey).setBuilder(recoveryErrorFunction); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(topKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); tester.getOrCreate(error).setHasError(false); StringValue reformed = new StringValue("reformed"); tester.set(error, reformed); tester.getOrCreate(topKey).setBuilder(null).addDependency(error).setComputedValue(COPY); tester.invalidate(); result = tester.eval(/*keepGoing=*/false, topKey); - assertThat(result.get(topKey)).isEqualTo(reformed); - assertThat(result.hasError()).isFalse(); + assertThatEvaluationResult(result).hasEntryThat(topKey).isEqualTo(reformed); + assertThatEvaluationResult(result).hasNoError(); } /** @@ -3729,13 +3696,11 @@ tester.getOrCreate(midKey).addDependency(badKey).setComputedValue(CONCATENATE); tester.getOrCreate(badKey).setHasError(true); EvaluationResult<SkyValue> result = tester.eval(/*keepGoing=*/ false, topKey, midKey); - assertThat(result.getError(midKey).getRootCauses().toList()).containsExactly(badKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(midKey); // Do it again with keepGoing. We should also see an error for the top key this time. result = tester.eval(/*keepGoing=*/ true, topKey, midKey); - if (rootCausesStored()) { - assertThat(result.getError(midKey).getRootCauses().toList()).containsExactly(badKey); - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(badKey); - } + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(midKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(topKey); } @Test @@ -3750,13 +3715,14 @@ // When the error value throws, the propagation will cause an interrupted exception in parent. EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ false, parentKey); assertThat(result.keyNames()).isEmpty(); - Map.Entry<SkyKey, ErrorInfo> error = Iterables.getOnlyElement(result.errorMap().entrySet()); - assertThat(error.getKey()).isEqualTo(parentKey); - assertThat(error.getValue().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasErrorMapThat().hasSize(1); + assertThatEvaluationResult(result).hasErrorMapThat().containsKey(parentKey); assertThat(Thread.interrupted()).isFalse(); result = tester.eval(/*keepGoing=*/ true, parentKey); - assertThat(result.errorMap()).isEmpty(); - assertThat(result.get(parentKey).getValue()).isEqualTo("recovered"); + assertThatEvaluationResult(result).hasNoError(); + assertThatEvaluationResult(result) + .hasEntryThat(parentKey) + .isEqualTo(new StringValue("recovered")); } /** @@ -3808,21 +3774,13 @@ tester.getOrCreate(midKey).addDependency(badKey).setComputedValue(CONCATENATE); tester.getOrCreate(badKey).setHasError(true); EvaluationResult<SkyValue> result = tester.eval(/*keepGoing=*/ false, topKey, midKey); - assertThat(result.getError(midKey).getRootCauses().toList()).containsExactly(badKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(midKey); waitForSecondCall.set(true); result = tester.eval(/*keepGoing=*/ true, topKey, midKey); assertThat(firstThread.get()).isNotNull(); assertThat(otherThreadWinning.getCount()).isEqualTo(0); - assertThatEvaluationResult(result).hasErrorEntryForKeyThat(midKey).isNotNull(); - assertThatEvaluationResult(result).hasErrorEntryForKeyThat(topKey).isNotNull(); - if (rootCausesStored()) { - assertThatEvaluationResult(result) - .hasErrorEntryForKeyThat(midKey) - .rootCauseOfExceptionIs(badKey); - assertThatEvaluationResult(result) - .hasErrorEntryForKeyThat(topKey) - .rootCauseOfExceptionIs(badKey); - } + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(midKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(topKey); } @Test @@ -3837,13 +3795,12 @@ .setComputedValue(CONCATENATE) .addDependency("after"); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ false, parentKey); - assertThat(result.keyNames()).isEmpty(); - Map.Entry<SkyKey, ErrorInfo> error = Iterables.getOnlyElement(result.errorMap().entrySet()); - assertThat(error.getKey()).isEqualTo(parentKey); - assertThat(error.getValue().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parentKey); result = tester.eval(/*keepGoing=*/ true, parentKey); - assertThat(result.errorMap()).isEmpty(); - assertThat(result.get(parentKey).getValue()).isEqualTo("recoveredafter"); + assertThatEvaluationResult(result).hasNoError(); + assertThatEvaluationResult(result) + .hasEntryThat(parentKey) + .isEqualTo(new StringValue("recoveredafter")); } @Test @@ -3935,9 +3892,8 @@ EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/ false, errorKey, otherErrorKey); - // Then the result reports that an error occurred because of errorKey, + // Then the result reports that an error occurred, assertThat(result.hasError()).isTrue(); - assertThat(result.getError().getRootCauseOfException()).isEqualTo(errorKey); // And no value is committed for otherErrorKey, assertThat(tester.driver.getExistingErrorForTesting(otherErrorKey)).isNull(); @@ -3971,8 +3927,7 @@ tester.getOrCreate(newParent).addDependency(errorKey).setComputedValue(CONCATENATE); tester.invalidate(); EvaluationResult<StringValue> result = tester.eval(/*keepGoing=*/false, newParent); - ErrorInfo error = result.getError(newParent); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(newParent); } @Test @@ -3999,14 +3954,11 @@ tester .getOrCreate(parent) .addDependency(child) - .setComputedValue(new ValueComputer() { - @Override - public SkyValue compute(Map<SkyKey, SkyValue> deps, Environment env) - throws InterruptedException { - parentEvaluated.incrementAndGet(); - return new StringValue(val); - } - }); + .setComputedValue( + (deps, env) -> { + parentEvaluated.incrementAndGet(); + return new StringValue(val); + }); assertStringValue(val, tester.evalAndGet( /*keepGoing=*/false, parent)); assertThat(parentEvaluated.get()).isEqualTo(1); if (hasEvent) { @@ -4209,13 +4161,11 @@ tester.getOrCreate(errorKey).setHasTransientError(true); ErrorInfo errorInfo = tester.evalAndGetError(/*keepGoing=*/ true, errorKey); assertThat(errorInfo).isNotNull(); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(errorKey); // Re-evaluates to same thing when errors are invalidated tester.invalidateTransientErrors(); errorInfo = tester.evalAndGetError(/*keepGoing=*/ true, errorKey); assertThat(errorInfo).isNotNull(); StringValue value = new StringValue("reformed"); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(errorKey); tester.getOrCreate(errorKey, /*markAsModified=*/false).setHasTransientError(false) .setConstantValue(value); tester.invalidateTransientErrors(); @@ -4839,9 +4789,6 @@ .hasErrorEntryForKeyThat(topKey) .hasExceptionThat() .isNotNull(); - assertThatEvaluationResult(result2) - .hasErrorEntryForKeyThat(topKey) - .rootCauseOfExceptionIs(errorKey); } @Test @@ -5406,9 +5353,8 @@ public ErrorInfo evalAndGetError(boolean keepGoing, SkyKey key) throws InterruptedException { EvaluationResult<StringValue> evaluationResult = eval(keepGoing, key); - ErrorInfo result = evaluationResult.getError(key); - assertWithMessage(evaluationResult.toString()).that(result).isNotNull(); - return result; + assertThatEvaluationResult(evaluationResult).hasErrorEntryForKeyThat(key); + return evaluationResult.getError(key); } public ErrorInfo evalAndGetError(boolean keepGoing, String key) throws InterruptedException {
diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java index 4e1483c..19253b1 100644 --- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -825,8 +825,7 @@ tester.getOrCreate(parentErrorKey).addDependency("a").addDependency(errorKey) .setComputedValue(CONCATENATE); tester.getOrCreate(errorKey).setHasError(true); - ErrorInfo error = evalValueInError(parentErrorKey); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey); + evalValueInError(parentErrorKey); } @Test @@ -843,10 +842,8 @@ tester.getOrCreate(errorFreeKey).addDependency("a").addDependency("b") .setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = eval(true, parentErrorKey, errorFreeKey); - ErrorInfo error = result.getError(parentErrorKey); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey); - StringValue abValue = result.get(errorFreeKey); - assertThat(abValue.getValue()).isEqualTo("ab"); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(parentErrorKey); + assertThatEvaluationResult(result).hasEntryThat(errorFreeKey).isEqualTo(new StringValue("ab")); } @Test @@ -911,8 +908,7 @@ SkyKey topKey = GraphTester.toSkyKey("top"); tester.getOrCreate(topKey).addDependency(catastropheKey).setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = eval(keepGoing, topKey, otherKey); - ErrorInfo error = result.getError(topKey); - assertThat(error.getRootCauses().toList()).containsExactly(catastropheKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(topKey); if (keepGoing) { assertThat(result.getCatastrophe()).isSameInstanceAs(catastrophe); } @@ -1002,12 +998,8 @@ EvaluationResult<StringValue> result = eval(/*keepGoing=*/true, parentKey, childKey); // Child is guaranteed to complete successfully before parent can run (and fail), // since parent depends on it. - StringValue childValue = result.get(childKey); - assertThat(childValue).isNotNull(); - assertThat(childValue.getValue()).isEqualTo("onions"); - ErrorInfo error = result.getError(parentKey); - assertThat(error).isNotNull(); - assertThat(error.getRootCauses().toList()).containsExactly(parentKey); + assertThatEvaluationResult(result).hasEntryThat(childKey).isEqualTo(new StringValue("onions")); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(parentKey); } @Test @@ -1015,12 +1007,10 @@ graph = new InMemoryGraphImpl(); SkyKey errorKey = GraphTester.toSkyKey("error"); tester.getOrCreate(errorKey).setHasError(true); - ErrorInfo error = evalValueInError(errorKey); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey); + evalValueInError(errorKey); SkyKey parentKey = GraphTester.toSkyKey("parent"); tester.getOrCreate(parentKey).addDependency("error").setComputedValue(CONCATENATE); - error = evalValueInError(parentKey); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey); + evalValueInError(parentKey); } @Test @@ -1031,8 +1021,7 @@ tester.getOrCreate(errorKey).setHasError(true); tester.getOrCreate("mid").addDependency(errorKey).setComputedValue(CONCATENATE); tester.getOrCreate(parentKey).addDependency("mid").setComputedValue(CONCATENATE); - ErrorInfo error = evalValueInError(parentKey); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey); + evalValueInError(parentKey); } @Test @@ -1074,8 +1063,7 @@ tester.getOrCreate(parentKey) .addDependency("mid").addDependency(errorKey2).addDependency(errorKey3) .setComputedValue(CONCATENATE); - ErrorInfo error = evalValueInError(parentKey); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey, errorKey2, errorKey3); + evalValueInError(parentKey); } @Test @@ -1087,9 +1075,7 @@ tester.getOrCreate("mid").addDependency(errorKey).setComputedValue(CONCATENATE); tester.getOrCreate(parentKey).addDependency("mid").setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = eval(false, ImmutableList.of(parentKey)); - Map.Entry<SkyKey, ErrorInfo> error = Iterables.getOnlyElement(result.errorMap().entrySet()); - assertThat(error.getKey()).isEqualTo(parentKey); - assertThat(error.getValue().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parentKey); } @Test @@ -1123,13 +1109,14 @@ tester.getOrCreate(errorKey).setHasError(true); SkyKey parentKey = GraphTester.toSkyKey("parent"); tester.getOrCreate(parentKey).addDependency(errorKey); - ErrorInfo error = evalValueInError(parentKey); - assertThat(error.getRootCauses().toList()).containsExactly(errorKey); + evalValueInError(parentKey); SkyKey[] list = { parentKey }; EvaluationResult<StringValue> result = eval(false, list); - ErrorInfo errorInfo = result.getError(); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(errorKey); - assertThat(errorInfo.getException()).hasMessageThat().isEqualTo(errorKey.toString()); + assertThatEvaluationResult(result) + .hasSingletonErrorThat(parentKey) + .hasExceptionThat() + .hasMessageThat() + .isEqualTo(errorKey.toString()); } @Test @@ -1403,18 +1390,12 @@ SkyKey errorKey = GraphTester.toSkyKey("my_error_value"); tester.getOrCreate(errorKey).setHasError(true); EvaluationResult<StringValue> result = eval(false, ImmutableList.of(errorKey)); - assertThat(result.keyNames()).isEmpty(); - assertThat(result.errorMap().keySet()).containsExactly(errorKey); - ErrorInfo errorInfo = result.getError(); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(errorKey); // Update value. But builder won't rebuild it. tester.getOrCreate(errorKey).setHasError(false); tester.set(errorKey, new StringValue("no error?")); result = eval(false, ImmutableList.of(errorKey)); - assertThat(result.keyNames()).isEmpty(); - assertThat(result.errorMap().keySet()).containsExactly(errorKey); - errorInfo = result.getError(); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(errorKey); } /** @@ -1534,18 +1515,12 @@ SkyKey errorKey = GraphTester.toSkyKey("my_error_value"); tester.getOrCreate(errorKey).setHasError(true); EvaluationResult<StringValue> result = eval(true, ImmutableList.of(errorKey)); - assertThat(result.keyNames()).isEmpty(); - assertThat(result.errorMap().keySet()).containsExactly(errorKey); - ErrorInfo errorInfo = result.getError(); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(errorKey); // Update value. But builder won't rebuild it. tester.getOrCreate(errorKey).setHasError(false); tester.set(errorKey, new StringValue("no error?")); result = eval(true, ImmutableList.of(errorKey)); - assertThat(result.keyNames()).isEmpty(); - assertThat(result.errorMap().keySet()).containsExactly(errorKey); - errorInfo = result.getError(); - assertThat(errorInfo.getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(errorKey); } @Test @@ -1561,10 +1536,7 @@ assertThat(result.errorMap()).isEmpty(); assertThat(result.get(parentKey).getValue()).isEqualTo("recoveredafter"); result = eval(/*keepGoing=*/false, ImmutableList.of(parentKey)); - assertThat(result.keyNames()).isEmpty(); - Map.Entry<SkyKey, ErrorInfo> error = Iterables.getOnlyElement(result.errorMap().entrySet()); - assertThat(error.getKey()).isEqualTo(parentKey); - assertThat(error.getValue().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parentKey); } @Test @@ -1577,10 +1549,7 @@ .setHasError(true); EvaluationResult<StringValue> result = eval( /*keepGoing=*/false, ImmutableList.of(parentErrorKey)); - assertThat(result.keyNames()).isEmpty(); - Map.Entry<SkyKey, ErrorInfo> error = Iterables.getOnlyElement(result.errorMap().entrySet()); - assertThat(error.getKey()).isEqualTo(parentErrorKey); - assertThat(error.getValue().getRootCauses().toList()).containsExactly(parentErrorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parentErrorKey); } @Test @@ -1593,10 +1562,7 @@ .setHasError(true); EvaluationResult<StringValue> result = eval( /*keepGoing=*/true, ImmutableList.of(parentErrorKey)); - assertThat(result.keyNames()).isEmpty(); - Map.Entry<SkyKey, ErrorInfo> error = Iterables.getOnlyElement(result.errorMap().entrySet()); - assertThat(error.getKey()).isEqualTo(parentErrorKey); - assertThat(error.getValue().getRootCauses().toList()).containsExactly(parentErrorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parentErrorKey); } @Test @@ -1630,10 +1596,7 @@ tester.getOrCreate(topKey).addDependency(parentErrorKey).addDependency("after") .setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = eval(/*keepGoing=*/false, ImmutableList.of(topKey)); - assertThat(result.keyNames()).isEmpty(); - Map.Entry<SkyKey, ErrorInfo> error = Iterables.getOnlyElement(result.errorMap().entrySet()); - assertThat(error.getKey()).isEqualTo(topKey); - assertThat(error.getValue().getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); } @Test @@ -1744,16 +1707,11 @@ null, /*waitForException=*/false, null, ImmutableSet.<SkyKey>of())); EvaluationResult<StringValue> result = eval(keepGoing, ImmutableSet.of(topKey)); - assertThat(result.errorMap().keySet()).containsExactly(topKey); - Iterable<CycleInfo> cycleInfos = result.getError(topKey).getCycleInfo(); - if (keepGoing) { - // The error thrown will only be recorded in keep_going mode. - assertThat(result.getError().getRootCauses().toList()).containsExactly(errorKey); - } - assertThat(cycleInfos).isNotEmpty(); - CycleInfo cycleInfo = Iterables.getOnlyElement(cycleInfos); - assertThat(cycleInfo.getPathToCycle()).containsExactly(topKey); - assertThat(cycleInfo.getCycle()).containsExactly(midKey, cycleKey); + assertThatEvaluationResult(result) + .hasSingletonErrorThat(topKey) + .hasCycleInfoThat() + .containsExactly( + new CycleInfo(ImmutableList.of(topKey), ImmutableList.of(midKey, cycleKey))); } @Test @@ -1845,16 +1803,13 @@ EvaluationResult<StringValue> result = eval(keepGoing, ImmutableSet.of(topKey, otherTop)); if (keepGoing) { - assertThat(result.errorMap().keySet()).containsExactly(otherTop, topKey); - assertThat(result.getError(otherTop).getRootCauses().toList()).containsExactly(otherTop); - // The error thrown will only be recorded in keep_going mode. - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasErrorMapThat().hasSize(2); } - Iterable<CycleInfo> cycleInfos = result.getError(topKey).getCycleInfo(); - assertThat(cycleInfos).isNotEmpty(); - CycleInfo cycleInfo = Iterables.getOnlyElement(cycleInfos); - assertThat(cycleInfo.getPathToCycle()).containsExactly(topKey); - assertThat(cycleInfo.getCycle()).containsExactly(midKey, cycleKey); + assertThatEvaluationResult(result) + .hasErrorEntryForKeyThat(topKey) + .hasCycleInfoThat() + .containsExactly( + new CycleInfo(ImmutableList.of(topKey), ImmutableList.of(midKey, cycleKey))); } @Test @@ -1935,9 +1890,7 @@ tester.getOrCreate(topKey).addErrorDependency(errorKey, new StringValue("recovered")) .setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = eval(keepGoing, ImmutableList.of(topKey)); - assertThat(result.keyNames()).isEmpty(); - assertThat(result.getError(topKey).getException()).isSameInstanceAs(exception); - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); } /** @@ -2005,13 +1958,10 @@ .setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = eval(keepGoing, ImmutableList.of(topKey)); if (!keepGoing) { - assertThat(result.keyNames()).isEmpty(); - assertThat(result.getError(topKey).getException()).isEqualTo(topException); - assertThat(result.getError(topKey).getRootCauses().toList()).containsExactly(topKey); - assertThatEvaluationResult(result).hasError(); + assertThatEvaluationResult(result).hasSingletonErrorThat(topKey); } else { assertThatEvaluationResult(result).hasNoError(); - assertThat(result.get(topKey)).isSameInstanceAs(topValue); + assertThatEvaluationResult(result).hasEntryThat(topKey).isSameInstanceAs(topValue); } } @@ -2358,8 +2308,7 @@ } EvaluationResult<StringValue> result = eval(/*keepGoing=*/false, ImmutableList.of(parentKey)); assertThat(numParentInvocations.get()).isEqualTo(2); - assertThat(result.hasError()).isTrue(); - assertThat(result.getError().getRootCauseOfException()).isEqualTo(childKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(parentKey); } @Test @@ -2405,8 +2354,7 @@ .setBuilder( new SkyFunction() { @Override - public SkyValue compute(SkyKey skyKey, Environment env) - throws SkyFunctionException, InterruptedException { + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { int invocations = numOtherParentInvocations.incrementAndGet(); assertWithMessage("otherParentKey should not be restarted") .that(invocations) @@ -2458,8 +2406,7 @@ .setBuilder( new SkyFunction() { @Override - public SkyValue compute(SkyKey skyKey, Environment env) - throws SkyFunctionException, InterruptedException { + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { int invocations = numErrorParentInvocations.incrementAndGet(); try { SkyValue value = env.getValueOrThrow(errorKey, SomeErrorException.class); @@ -2488,33 +2435,28 @@ graph = new NotifyingHelper.NotifyingProcessableGraph( new InMemoryGraphImpl(), - new Listener() { - @Override - public void accept(SkyKey key, EventType type, Order order, Object context) { - if (key.equals(errorKey) && type == EventType.SET_VALUE && order == Order.AFTER) { - errorCommitted.countDown(); - TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions( - otherParentSignaled, "otherParent didn't get signaled in time"); - // We try to give some time for ParallelEvaluator to incorrectly re-evaluate - // 'otherParentKey'. This test case is testing for a real race condition and the - // 10ms time was chosen experimentally to give a true positive rate of 99.8% - // (without a sleep it has a 1% true positive rate). There's no good way to do - // this without sleeping. We *could* introspect ParallelEvaulator's - // AbstractQueueVisitor to see if the re-evaluation has been enqueued, but that's - // relying on pretty low-level implementation details. - Uninterruptibles.sleepUninterruptibly(10, TimeUnit.MILLISECONDS); - } - if (key.equals(otherParentKey) - && type == EventType.SIGNAL - && order == Order.AFTER) { - otherParentSignaled.countDown(); - } + (key, type, order, context) -> { + if (key.equals(errorKey) && type == EventType.SET_VALUE && order == Order.AFTER) { + errorCommitted.countDown(); + TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions( + otherParentSignaled, "otherParent didn't get signaled in time"); + // We try to give some time for ParallelEvaluator to incorrectly re-evaluate + // 'otherParentKey'. This test case is testing for a real race condition and the + // 10ms time was chosen experimentally to give a true positive rate of 99.8% + // (without a sleep it has a 1% true positive rate). There's no good way to do + // this without sleeping. We *could* introspect ParallelEvaulator's + // AbstractQueueVisitor to see if the re-evaluation has been enqueued, but that's + // relying on pretty low-level implementation details. + Uninterruptibles.sleepUninterruptibly(10, TimeUnit.MILLISECONDS); + } + if (key.equals(otherParentKey) && type == EventType.SIGNAL && order == Order.AFTER) { + otherParentSignaled.countDown(); } }); EvaluationResult<StringValue> result = eval(/*keepGoing=*/false, ImmutableList.of(otherParentKey, errorParentKey)); assertThat(result.hasError()).isTrue(); - assertThat(result.getError().getRootCauseOfException()).isEqualTo(errorKey); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(errorParentKey); } @Test @@ -2530,11 +2472,9 @@ new StringValue("parent2")); tester.getOrCreate(errorKey).setHasError(true); EvaluationResult<StringValue> result = eval(/*keepGoing=*/true, ImmutableList.of(parent1Key)); - assertThat(result.hasError()).isTrue(); - assertThat(result.getError().getRootCauseOfException()).isEqualTo(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parent1Key); result = eval(/*keepGoing=*/false, ImmutableList.of(parent2Key)); - assertThat(result.hasError()).isTrue(); - assertThat(result.getError(parent2Key).getRootCauseOfException()).isEqualTo(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(parent2Key); } @Test @@ -2544,8 +2484,7 @@ SkyKey errorKey = GraphTester.toSkyKey("error"); tester.getOrCreate(errorKey).setHasError(true); EvaluationResult<StringValue> result = eval(/*keepGoing=*/true, ImmutableList.of(errorKey)); - assertThat(result.hasError()).isTrue(); - assertThat(result.getError().getRootCauseOfException()).isEqualTo(errorKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(errorKey); SkyKey rogueKey = GraphTester.toSkyKey("rogue"); tester.getOrCreate(rogueKey).setBuilder(new SkyFunction() { @Override @@ -2562,8 +2501,8 @@ } }); result = eval(/*keepGoing=*/false, ImmutableList.of(errorKey, rogueKey)); - assertThat(result.hasError()).isTrue(); - assertThat(result.getError(errorKey).getRootCauseOfException()).isEqualTo(errorKey); + assertThatEvaluationResult(result).hasErrorMapThat().hasSize(1); + assertThatEvaluationResult(result).hasErrorEntryForKeyThat(errorKey); assertThat(result.errorMap()).doesNotContainKey(rogueKey); } @@ -2660,7 +2599,7 @@ try { return env.getValueOrThrow(childKey, SomeErrorException.class); } catch (SomeErrorException e) { - throw new GenericFunctionException(e, childKey); + throw new GenericFunctionException(e); } } else { return env.getValue(childKey); @@ -2674,9 +2613,8 @@ }); tester.getOrCreate(childKey).setHasError(/*hasError=*/true); EvaluationResult<StringValue> result = eval(keepGoing, ImmutableList.of(grandparentKey)); - assertThat(result.hasError()).isTrue(); assertThat(errorPropagated.get()).isTrue(); - assertThat(result.getError().getRootCauseOfException()).isEqualTo(grandparentKey); + assertThatEvaluationResult(result).hasSingletonErrorThat(grandparentKey); } @Test