Make deterministic the error that is stored in TransitiveTraversalValue. When there are multiple errors, we don't want non-determinism.
PiperOrigin-RevId: 240208756
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java
index 0fa3e05..9ca9b5d 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunctionTest.java
@@ -31,6 +31,8 @@
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.ValueOrException2;
+import com.google.devtools.build.skyframe.ValueOrUntypedException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -98,6 +100,101 @@
assertThat(wasOptimizationUsed.get()).isTrue();
}
+ @Test
+ public void multipleErrorsForTransitiveTraversalFunction() throws Exception {
+ Label label = Label.parseAbsolute("//foo:foo", ImmutableMap.of());
+ Package pkg =
+ scratchPackage(
+ "workspace",
+ label.getPackageIdentifier(),
+ "sh_library(name = '" + label.getName() + "', deps = [':bar', ':baz'])");
+ TargetAndErrorIfAnyImpl targetAndErrorIfAny =
+ new TargetAndErrorIfAnyImpl(
+ /*packageLoadedSuccessfully=*/ true,
+ /*errorLoadingTarget=*/ null,
+ pkg.getTarget(label.getName()));
+ TransitiveTraversalFunction function =
+ new TransitiveTraversalFunction() {
+ @Override
+ LoadTargetResults loadTarget(Environment env, Label label) {
+ return targetAndErrorIfAny;
+ }
+ };
+ SkyKey dep1 = function.getKey(Label.parseAbsolute("//foo:bar", ImmutableMap.of()));
+ SkyKey dep2 = function.getKey(Label.parseAbsolute("//foo:baz", ImmutableMap.of()));
+ ImmutableMap<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>
+ returnedDeps =
+ ImmutableMap.of(dep1, makeException("bad bar"), dep2, makeException("bad baz"));
+ SkyFunction.Environment mockEnv = Mockito.mock(SkyFunction.Environment.class);
+ // Try two evaluations, with the environment reversing the order of the map it returns.
+ when(mockEnv.getValuesOrThrow(
+ Mockito.any(),
+ Mockito.eq(NoSuchPackageException.class),
+ Mockito.eq(NoSuchTargetException.class)))
+ .thenReturn(returnedDeps);
+ when(mockEnv.valuesMissing()).thenReturn(false);
+
+ assertThat(
+ ((TransitiveTraversalValue) function.compute(function.getKey(label), mockEnv))
+ .getErrorMessage())
+ .isEqualTo("bad bar");
+ ImmutableMap<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>
+ reversedDeps =
+ ImmutableMap.of(dep2, makeException("bad baz"), dep1, makeException("bad bar"));
+ when(mockEnv.getValuesOrThrow(
+ Mockito.any(),
+ Mockito.eq(NoSuchPackageException.class),
+ Mockito.eq(NoSuchTargetException.class)))
+ .thenReturn(reversedDeps);
+ assertThat(
+ ((TransitiveTraversalValue) function.compute(function.getKey(label), mockEnv))
+ .getErrorMessage())
+ .isEqualTo("bad bar");
+ }
+
+ @Test
+ public void selfErrorWins() throws Exception {
+ Label label = Label.parseAbsolute("//foo:foo", ImmutableMap.of());
+ Package pkg =
+ scratchPackage(
+ "workspace",
+ label.getPackageIdentifier(),
+ "sh_library(name = '" + label.getName() + "', deps = [':bar', ':baz'])");
+ TargetAndErrorIfAnyImpl targetAndErrorIfAny =
+ new TargetAndErrorIfAnyImpl(
+ /*packageLoadedSuccessfully=*/ true,
+ /*errorLoadingTarget=*/ new NoSuchTargetException("self error is long and last"),
+ pkg.getTarget(label.getName()));
+ TransitiveTraversalFunction function =
+ new TransitiveTraversalFunction() {
+ @Override
+ LoadTargetResults loadTarget(Environment env, Label label) {
+ return targetAndErrorIfAny;
+ }
+ };
+ SkyKey dep = function.getKey(Label.parseAbsolute("//foo:bar", ImmutableMap.of()));
+ SkyFunction.Environment mockEnv = Mockito.mock(SkyFunction.Environment.class);
+ when(mockEnv.getValuesOrThrow(
+ Mockito.any(),
+ Mockito.eq(NoSuchPackageException.class),
+ Mockito.eq(NoSuchTargetException.class)))
+ .thenReturn(ImmutableMap.of(dep, makeException("bad bar")));
+ when(mockEnv.valuesMissing()).thenReturn(false);
+
+ assertThat(
+ ((TransitiveTraversalValue) function.compute(function.getKey(label), mockEnv))
+ .getErrorMessage())
+ .isEqualTo("self error is long and last");
+ }
+
+ private static ValueOrException2<NoSuchPackageException, NoSuchTargetException> makeException(
+ String errorMessage) {
+ ValueOrUntypedException exn =
+ ValueOrUntypedException.ofExn(new NoSuchTargetException(errorMessage));
+ return ValueOrException2.fromUntypedException(
+ exn, NoSuchPackageException.class, NoSuchTargetException.class);
+ }
+
private Package scratchPackage(String workspaceName, PackageIdentifier packageId, String... lines)
throws Exception {
Path buildFile = scratch.file("" + packageId.getSourceRoot() + "/BUILD", lines);