Remove remaining calls to `getOrderedValuesAndExceptions` and tests.
After this, all that remains is unused implementations.
PiperOrigin-RevId: 485928424
Change-Id: Idfa158fc1dfed3d13f1b23b55d23867b5db5b956
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/LocalDiffAwarenessIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/LocalDiffAwarenessIntegrationTest.java
index 0cc0c50..e1ba1f8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/LocalDiffAwarenessIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/LocalDiffAwarenessIntegrationTest.java
@@ -224,8 +224,8 @@
buildTarget("//hello:BUILD");
// New package path on first build triggers full-graph work.
calledGetValues.set(0);
- // getOrderedValuesAndExceptions() called during output file checking (although if an output
- // service is able to report modified files in practice there is no iteration).
+ // getValuesAndExceptions() called during output file checking (although if an output service is
+ // able to report modified files in practice there is no iteration).
buildTarget("//hello:BUILD");
assertThat(calledGetValues.getAndSet(0)).isEqualTo(1);
diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphTraversingHelperTest.java b/src/test/java/com/google/devtools/build/skyframe/GraphTraversingHelperTest.java
index 5d60333..73247aa 100644
--- a/src/test/java/com/google/devtools/build/skyframe/GraphTraversingHelperTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/GraphTraversingHelperTest.java
@@ -20,6 +20,7 @@
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.bugreport.BugReporter;
import org.junit.Test;
@@ -29,11 +30,12 @@
/** Tests for {@link GraphTraversingHelper}. */
@RunWith(JUnit4.class)
public final class GraphTraversingHelperTest {
- SkyFunction.Environment mockEnv = mock(SkyFunction.Environment.class);
- SkyKey keyA = mock(SkyKey.class, "keyA");
- SkyKey keyB = mock(SkyKey.class, "keyB");
- SomeErrorException exn = new SomeErrorException("");
- SkyValue value = mock(SkyValue.class);
+
+ private final SkyFunction.Environment mockEnv = mock(SkyFunction.Environment.class);
+ private final SkyKey keyA = mock(SkyKey.class, "keyA");
+ private final SkyKey keyB = mock(SkyKey.class, "keyB");
+ private final SomeErrorException exn = new SomeErrorException("");
+ private final SkyValue value = mock(SkyValue.class);
private static final class SomeOtherErrorException extends Exception {
private SomeOtherErrorException() {}
@@ -43,7 +45,7 @@
public void declareDependenciesAndCheckIfValuesMissing_valuesMissingBeforeCompute()
throws Exception {
when(mockEnv.valuesMissing()).thenReturn(true);
- when(mockEnv.getOrderedValuesAndExceptions(ImmutableSet.of(keyA))).thenReturn(null);
+ when(mockEnv.getValuesAndExceptions(ImmutableSet.of(keyA))).thenReturn(null);
boolean valuesMissing =
GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissing(
mockEnv, ImmutableSet.of(keyA), SomeOtherErrorException.class);
@@ -54,10 +56,10 @@
public void declareDependenciesAndCheckIfValuesMissing_valuesMissingAfterCompute()
throws Exception {
BugReporter mockReporter = mock(BugReporter.class);
- SkyframeIterableResult result =
- new SimpleSkyframeIterableResult(
- () -> {}, ImmutableSet.of(ValueOrUntypedException.ofExn(exn)).iterator());
- when(mockEnv.getOrderedValuesAndExceptions(ImmutableSet.of(keyA))).thenReturn(result);
+ SkyframeLookupResult result =
+ new SimpleSkyframeLookupResult(
+ () -> {}, ImmutableMap.of(keyA, ValueOrUntypedException.ofExn(exn))::get);
+ when(mockEnv.getValuesAndExceptions(ImmutableSet.of(keyA))).thenReturn(result);
boolean valuesMissing =
GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissing(
mockEnv,
@@ -74,14 +76,16 @@
@Test
public void declareDependenciesAndCheckIfValuesMissing_notValuesMissingAfterCompute()
throws Exception {
- SkyframeIterableResult result =
- new SimpleSkyframeIterableResult(
+ SkyframeLookupResult result =
+ new SimpleSkyframeLookupResult(
() -> {},
- ImmutableList.of(
+ ImmutableMap.of(
+ keyA,
ValueOrUntypedException.ofExn(exn),
+ keyB,
ValueOrUntypedException.ofValueUntyped(value))
- .iterator());
- when(mockEnv.getOrderedValuesAndExceptions(ImmutableList.of(keyA, keyB))).thenReturn(result);
+ ::get);
+ when(mockEnv.getValuesAndExceptions(ImmutableList.of(keyA, keyB))).thenReturn(result);
boolean valuesMissing =
GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissing(
mockEnv, ImmutableList.of(keyA, keyB), SomeErrorException.class);
@@ -91,12 +95,16 @@
@Test
public void declareDependenciesAndCheckIfValuesMissing_nullAfterError_hasCorrectKeyInBugReport()
throws Exception {
- SkyframeIterableResult result =
- new SimpleSkyframeIterableResult(
+ SkyframeLookupResult result =
+ new SimpleSkyframeLookupResult(
() -> {},
- ImmutableList.of(ValueOrUntypedException.ofExn(exn), ValueOrUntypedException.ofNull())
- .iterator());
- when(mockEnv.getOrderedValuesAndExceptions(ImmutableList.of(keyA, keyB))).thenReturn(result);
+ ImmutableMap.of(
+ keyA,
+ ValueOrUntypedException.ofExn(exn),
+ keyB,
+ ValueOrUntypedException.ofNull())
+ ::get);
+ when(mockEnv.getValuesAndExceptions(ImmutableList.of(keyA, keyB))).thenReturn(result);
BugReporter mockReporter = mock(BugReporter.class);
boolean valuesMissing =
@@ -113,7 +121,7 @@
public void declareDependenciesAndCheckIfValuesMissingMaybeWithExceptions_beforeCompute()
throws Exception {
when(mockEnv.valuesMissing()).thenReturn(true);
- when(mockEnv.getOrderedValuesAndExceptions(ImmutableSet.of(keyB))).thenReturn(null);
+ when(mockEnv.getValuesAndExceptions(ImmutableSet.of(keyB))).thenReturn(null);
assertThat(
GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissingMaybeWithExceptions(
@@ -124,10 +132,10 @@
@Test
public void declareDependenciesAndCheckIfValuesMissingMaybeWithExceptions_valuesMissing()
throws Exception {
- when(mockEnv.getOrderedValuesAndExceptions(ImmutableSet.of(keyA)))
+ when(mockEnv.getValuesAndExceptions(ImmutableSet.of(keyA)))
.thenReturn(
- new SimpleSkyframeIterableResult(
- () -> {}, ImmutableSet.of(ValueOrUntypedException.ofExn(exn)).iterator()));
+ new SimpleSkyframeLookupResult(
+ () -> {}, ImmutableMap.of(keyA, ValueOrUntypedException.ofExn(exn))::get));
assertThat(
GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissingMaybeWithExceptions(
@@ -138,11 +146,11 @@
@Test
public void declareDependenciesAndCheckIfValuesMissingMaybeWithExceptions_notValuesMissing()
throws Exception {
- when(mockEnv.getOrderedValuesAndExceptions(ImmutableSet.of(keyB)))
+ when(mockEnv.getValuesAndExceptions(ImmutableSet.of(keyB)))
.thenReturn(
- new SimpleSkyframeIterableResult(
+ new SimpleSkyframeLookupResult(
() -> {},
- ImmutableSet.of(ValueOrUntypedException.ofValueUntyped(value)).iterator()));
+ ImmutableMap.of(keyB, ValueOrUntypedException.ofValueUntyped(value))::get));
assertThat(
GraphTraversingHelper.declareDependenciesAndCheckIfValuesMissingMaybeWithExceptions(
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 0fc04c3..7d9e5a0 100644
--- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java
@@ -1132,7 +1132,7 @@
@Nullable
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
- env.getOrderedValuesAndExceptions(ImmutableList.of(leafKey, bKey));
+ env.getValuesAndExceptions(ImmutableList.of(leafKey, bKey));
return null;
}
});
@@ -1243,8 +1243,8 @@
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
// The order here is important -- 2 before 1.
- SkyframeIterableResult result =
- env.getOrderedValuesAndExceptions(ImmutableList.of(cycleKey2, cycleKey1));
+ SkyframeLookupResult result =
+ env.getValuesAndExceptions(ImmutableList.of(cycleKey2, cycleKey1));
Preconditions.checkState(env.valuesMissing(), result);
return null;
}
@@ -1584,7 +1584,7 @@
// Order depKey first - makeGraphDeterministic() only makes the batch maps returned
// by the graph deterministic, not the order of temporary direct deps. This makes
// the order of deps match (alphabetized by the string representation).
- env.getOrderedValuesAndExceptions(ImmutableList.of(depKey, topKey));
+ env.getValuesAndExceptions(ImmutableList.of(depKey, topKey));
assertThat(env.valuesMissing()).isTrue();
return null;
}
@@ -2117,7 +2117,7 @@
.getOrCreate(topKey)
.setBuilder(
(skyKey, env) -> {
- env.getOrderedValuesAndExceptions(ImmutableList.of(errorKey, midKey, mid2Key));
+ env.getValuesAndExceptions(ImmutableList.of(errorKey, midKey, mid2Key));
if (env.valuesMissing()) {
return null;
}
@@ -2155,9 +2155,9 @@
.setBuilder(
(skyKey, env) -> {
SkyKeyValue val =
- ((SkyKeyValue)
- env.getOrderedValuesAndExceptions(ImmutableList.of(groupDepA, groupDepB))
- .next());
+ (SkyKeyValue)
+ env.getValuesAndExceptions(ImmutableList.of(groupDepA, groupDepB))
+ .get(groupDepA);
if (env.valuesMissing()) {
return null;
}
@@ -2290,7 +2290,7 @@
topRequestedDepOrRestartedBuild.countDown();
}
// top's builder just requests both deps in a group.
- env.getOrderedValuesAndExceptions(ImmutableList.of(firstKey, slowAddingDep));
+ env.getValuesAndExceptions(ImmutableList.of(firstKey, slowAddingDep));
return env.valuesMissing() ? null : new StringValue("top");
});
// First build : just prime the graph.
@@ -2523,7 +2523,7 @@
if (dep1 == null) {
return null;
}
- env.getOrderedValuesAndExceptions(
+ env.getValuesAndExceptions(
ImmutableList.of(newlyRequestedDoneDep, newlyRequestedNotDoneDep));
if (numFunctionCalls.get() < 4) {
return Restart.SELF;
@@ -2637,7 +2637,7 @@
// Request the second group. In the first build it's [leaf2, leaf4].
// In the second build it's [leaf2, leaf5]
- env.getOrderedValuesAndExceptions(
+ env.getValuesAndExceptions(
ImmutableList.of(
leaves.get(2),
GraphTester.toSkyKey(((StringValue) values.get(leaves.get(2))).getValue())));
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 96558b5..c15414b 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -1127,7 +1127,7 @@
@Nullable
@Override
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
- env.getOrderedValuesAndExceptions(ImmutableList.of(cycleKey, catastropheKey));
+ env.getValuesAndExceptions(ImmutableList.of(cycleKey, catastropheKey));
Preconditions.checkState(env.valuesMissing());
return null;
}
@@ -2187,14 +2187,14 @@
.getOrCreate(topKey)
.setBuilder(
(skyKey, env) -> {
- env.getOrderedValuesAndExceptions(leaves);
+ env.getValuesAndExceptions(leaves);
if (twoCalls && env.valuesMissing()) {
return null;
}
SkyKey first = sameFirst ? leaves.get(0) : leaf4;
SkyKey second = sameFirst ? leaf4 : leaves.get(2);
ImmutableList<SkyKey> secondRequest = ImmutableList.of(first, second);
- env.getOrderedValuesAndExceptions(secondRequest);
+ env.getValuesAndExceptions(secondRequest);
if (env.valuesMissing()) {
return null;
}
@@ -2235,7 +2235,7 @@
} catch (SomeErrorException e) {
// Recover from the child error.
}
- env.getOrderedValuesAndExceptions(deps);
+ env.getValuesAndExceptions(deps);
if (env.valuesMissing()) {
return null;
}
@@ -2354,112 +2354,6 @@
}
@Test
- public void getOrderedValuesAndExceptions() throws Exception {
- graph = new InMemoryGraphImpl();
- SkyKey otherKey = GraphTester.toSkyKey("other");
- tester.set(otherKey, new StringValue("other"));
- SkyKey anotherKey = GraphTester.toSkyKey("another");
- tester.set(anotherKey, new StringValue("another"));
- SkyKey errorExpectedKey = GraphTester.toSkyKey("errorExpected");
- tester.getOrCreate(errorExpectedKey).setHasError(true);
- SkyKey topKey = GraphTester.toSkyKey("top");
- Exception topException = new SomeErrorException("top exception");
- AtomicInteger numComputes = new AtomicInteger(0);
- tester
- .getOrCreate(topKey)
- .setBuilder(
- new SkyFunction() {
- @Nullable
- @Override
- public SkyValue compute(SkyKey skyKey, Environment env)
- throws SkyFunctionException, InterruptedException {
- SkyframeIterableResult skyframeIterableResult =
- env.getOrderedValuesAndExceptions(
- ImmutableList.of(otherKey, anotherKey, errorExpectedKey));
- if (numComputes.incrementAndGet() == 1) {
- assertThat(env.valuesMissing()).isTrue();
- int numElements = 0;
- while (skyframeIterableResult.hasNext()) {
- numElements++;
- try {
- assertThat(skyframeIterableResult.nextOrThrow(SomeErrorException.class))
- .isNull();
- } catch (SomeErrorException e) {
- throw new AssertionError("should not have thrown", e);
- }
- }
- assertThat(numElements).isEqualTo(3);
- return null;
- } else {
- assertThat(numComputes.get()).isEqualTo(2);
- SkyValue value1 = skyframeIterableResult.next();
- assertThat(value1).isNotNull();
- assertThat(env.valuesMissing()).isFalse();
- try {
- SkyValue value2 = skyframeIterableResult.nextOrThrow(SomeErrorException.class);
- assertThat(value2).isNotNull();
- assertThat(env.valuesMissing()).isFalse();
- } catch (SomeErrorException e) {
- throw new AssertionError("Should not have thrown", e);
- }
- try {
- skyframeIterableResult.nextOrThrow(SomeErrorException.class);
- throw new AssertionError("Should throw");
- } catch (SomeErrorException e) {
- assertThat(env.valuesMissing()).isFalse();
- }
- throw new SkyFunctionException(topException, Transience.PERSISTENT) {};
- }
- }
- });
- EvaluationResult<StringValue> result = eval(/*keepGoing=*/ true, ImmutableList.of(topKey));
- assertThatEvaluationResult(result).hasError();
- assertThatEvaluationResult(result)
- .hasErrorEntryForKeyThat(topKey)
- .hasExceptionThat()
- .isSameInstanceAs(topException);
- assertThat(numComputes.get()).isEqualTo(2);
- }
-
- @Test
- public void getOrderedValuesAndExceptionsWithErrors() throws Exception {
- graph = new InMemoryGraphImpl();
- final SkyKey childKey = GraphTester.toSkyKey("error");
- final SomeErrorException childExn = new SomeErrorException("child error");
- tester
- .getOrCreate(childKey)
- .setBuilder(
- (skyKey, env) -> {
- throw new GenericFunctionException(childExn, Transience.PERSISTENT);
- });
- SkyKey parentKey = GraphTester.toSkyKey("parent");
- final AtomicInteger numComputes = new AtomicInteger(0);
- tester
- .getOrCreate(parentKey)
- .setBuilder(
- (skyKey, env) -> {
- try {
- SkyValue value =
- env.getOrderedValuesAndExceptions(ImmutableList.of(childKey))
- .nextOrThrow(SomeOtherErrorException.class);
- assertThat(value).isNull();
- } catch (SomeOtherErrorException e) {
- throw new AssertionError("Should not have thrown", e);
- }
- numComputes.incrementAndGet();
- assertThat(env.valuesMissing()).isTrue();
- return null;
- });
- EvaluationResult<StringValue> result = eval(/*keepGoing=*/ true, ImmutableList.of(parentKey));
- assertThatEvaluationResult(result).hasError();
- assertThatEvaluationResult(result)
- .hasErrorEntryForKeyThat(parentKey)
- .hasExceptionThat()
- .isSameInstanceAs(childExn);
- assertThat(numComputes.get()).isEqualTo(2);
- }
-
- @Test
public void declareDependenciesAndCheckIfValuesMissing() throws Exception {
graph = new InMemoryGraphImpl();
final SkyKey childKey = GraphTester.toSkyKey("error");
