diff options
Diffstat (limited to 'src/test/java/com/google/devtools/build')
22 files changed, 613 insertions, 473 deletions
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java index 53009de1e9..8f9e44311b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationTest.java @@ -221,13 +221,11 @@ public class BuildConfigurationTest extends ConfigurationTestCase { @Override public Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions) - throws InvalidConfigurationException { + throws InvalidConfigurationException, InterruptedException { for (Class<? extends Fragment> fragmentType : dependsOn) { env.getFragment(buildOptions, fragmentType); } - return new Fragment() { - - }; + return new Fragment() {}; } }; } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java index eca845975c..58eab302a4 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ActionTester.java @@ -31,14 +31,15 @@ public class ActionTester { /** * Returns a new action instance. The parameter {@code i} is used to vary the parameters used to - * create the action. Implementations should do something like this: - * <code><pre> + * create the action. Implementations should do something like this: <code> + * <pre> * return new MyAction(owner, inputs, outputs, configuration, * (i & 1) == 0 ? a1 : a2, * (i & 2) == 0 ? b1 : b2, * (i & 4) == 0 ? c1 : c2); * (i & 16) == 0 ? d1 : d2); - * </pre></code> + * </pre> + * </code> * * <p>The wrap-around (in this case at 32) is intentional and is checked for by the testing * method. @@ -50,7 +51,7 @@ public class ActionTester { * <p>Furthermore, when called with identical parameters, this method should return different * instances (i.e. according to {@code ==}), but they should have the same key. */ - Action generate(int i); + Action generate(int i) throws InterruptedException; } /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java index 304d964fdf..d046a234d0 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java @@ -52,7 +52,6 @@ import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; - import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -165,18 +164,19 @@ public final class AnalysisTestUtil { } @Override - public Artifact getStableWorkspaceStatusArtifact() { + public Artifact getStableWorkspaceStatusArtifact() throws InterruptedException { return original.getStableWorkspaceStatusArtifact(); } @Override - public Artifact getVolatileWorkspaceStatusArtifact() { + public Artifact getVolatileWorkspaceStatusArtifact() throws InterruptedException { return original.getVolatileWorkspaceStatusArtifact(); } @Override - public ImmutableList<Artifact> getBuildInfo(RuleContext ruleContext, BuildInfoKey key, - BuildConfiguration config) { + public ImmutableList<Artifact> getBuildInfo( + RuleContext ruleContext, BuildInfoKey key, BuildConfiguration config) + throws InterruptedException { return original.getBuildInfo(ruleContext, key, config); } diff --git a/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java b/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java index 8f6d389b98..7be04c4547 100644 --- a/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java +++ b/src/test/java/com/google/devtools/build/lib/concurrent/AbstractQueueVisitorTest.java @@ -26,21 +26,18 @@ import com.google.common.util.concurrent.Uninterruptibles; import com.google.devtools.build.lib.concurrent.ErrorClassifier.ErrorClassification; import com.google.devtools.build.lib.testutil.TestThread; import com.google.devtools.build.lib.testutil.TestUtils; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * Tests for AbstractQueueVisitor. @@ -350,20 +347,8 @@ public class AbstractQueueVisitorTest { ThreadPoolExecutor executor = new ThreadPoolExecutor(3, 3, 0, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>()); - final AtomicBoolean throwableSeen = new AtomicBoolean(false); - ErrorHandler errorHandler = new ErrorHandler() { - @Override - public void handle(Throwable t, ErrorClassification classification) { - if (t == THROWABLE) { - assertThat(classification).isEqualTo(ErrorClassification.CRITICAL); - throwableSeen.compareAndSet(false, true); - } else { - fail(); - } - } - }; - final AbstractQueueVisitor visitor = createQueueVisitorWithConstantErrorClassification( - executor, ErrorClassification.CRITICAL, errorHandler); + final AbstractQueueVisitor visitor = + createQueueVisitorWithConstantErrorClassification(executor, ErrorClassification.CRITICAL); final CountDownLatch latch1 = new CountDownLatch(1); final AtomicBoolean wasInterrupted = new AtomicBoolean(false); @@ -386,6 +371,7 @@ public class AbstractQueueVisitorTest { visitor.execute(r1); latch1.await(); visitor.execute(throwingRunnable()); + CountDownLatch exnLatch = visitor.getExceptionLatchForTestingOnly(); try { visitor.awaitQuiescence(/*interruptWorkers=*/ true); @@ -396,7 +382,7 @@ public class AbstractQueueVisitorTest { assertTrue(wasInterrupted.get()); assertTrue(executor.isShutdown()); - assertTrue(throwableSeen.get()); + assertTrue(exnLatch.await(0, TimeUnit.MILLISECONDS)); } @Test @@ -404,20 +390,9 @@ public class AbstractQueueVisitorTest { ThreadPoolExecutor executor = new ThreadPoolExecutor(2, 2, 0, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>()); final Error error = new Error("bad!"); - final AtomicBoolean criticalErrorSeen = new AtomicBoolean(false); - ErrorHandler errorHandler = new ErrorHandler() { - @Override - public void handle(Throwable t, ErrorClassification classification) { - if (t == error) { - assertThat(classification).isEqualTo(ErrorClassification.AS_CRITICAL_AS_POSSIBLE); - criticalErrorSeen.compareAndSet(false, true); - } else { - fail(); - } - } - }; - AbstractQueueVisitor visitor = createQueueVisitorWithConstantErrorClassification( - executor, ErrorClassification.NOT_CRITICAL, errorHandler); + AbstractQueueVisitor visitor = + createQueueVisitorWithConstantErrorClassification( + executor, ErrorClassification.NOT_CRITICAL); final CountDownLatch latch = new CountDownLatch(1); final AtomicBoolean sleepFinished = new AtomicBoolean(false); final AtomicBoolean sleepInterrupted = new AtomicBoolean(false); @@ -444,6 +419,7 @@ public class AbstractQueueVisitorTest { } } }; + CountDownLatch exnLatch = visitor.getExceptionLatchForTestingOnly(); visitor.execute(errorRunnable); visitor.execute(sleepRunnable); Error thrownError = null; @@ -457,7 +433,7 @@ public class AbstractQueueVisitorTest { assertTrue(sleepInterrupted.get()); assertFalse(sleepFinished.get()); assertEquals(error, thrownError); - assertTrue(criticalErrorSeen.get()); + assertTrue(exnLatch.await(0, TimeUnit.MILLISECONDS)); } private static class ClassifiedException extends RuntimeException { @@ -472,7 +448,6 @@ public class AbstractQueueVisitorTest { public void mostSevereErrorPropagated() throws Exception { ThreadPoolExecutor executor = new ThreadPoolExecutor(2, 2, 0, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>()); - final Set<Throwable> seenErrors = Sets.newConcurrentHashSet(); final ClassifiedException criticalException = new ClassifiedException(ErrorClassification.CRITICAL); final ClassifiedException criticalAndLogException = @@ -485,21 +460,13 @@ public class AbstractQueueVisitorTest { : ErrorClassification.NOT_CRITICAL; } }; - ErrorHandler errorHandler = new ErrorHandler() { - @Override - public void handle(Throwable t, ErrorClassification classification) { - assertThat(classification).isEqualTo(errorClassifier.classify(t)); - seenErrors.add(t); - } - }; AbstractQueueVisitor visitor = new AbstractQueueVisitor( - /*concurrent=*/ true, - executor, - /*shutdownOnCompletion=*/ true, - /*failFastOnException=*/ false, - errorClassifier, - errorHandler); + /*concurrent=*/ true, + executor, + /*shutdownOnCompletion=*/ true, + /*failFastOnException=*/ false, + errorClassifier); final CountDownLatch exnLatch = visitor.getExceptionLatchForTestingOnly(); Runnable criticalExceptionRunnable = new Runnable() { @Override @@ -532,7 +499,6 @@ public class AbstractQueueVisitorTest { exn = e; } assertEquals(criticalAndLogException, exn); - assertThat(seenErrors).containsExactly(criticalException, criticalAndLogException); } private static Runnable throwingRunnable() { @@ -636,8 +602,7 @@ public class AbstractQueueVisitorTest { } private static AbstractQueueVisitor createQueueVisitorWithConstantErrorClassification( - ThreadPoolExecutor executor, final ErrorClassification classification, - ErrorHandler errorHandler) { + ThreadPoolExecutor executor, final ErrorClassification classification) { return new AbstractQueueVisitor( /*concurrent=*/ true, executor, @@ -648,7 +613,6 @@ public class AbstractQueueVisitorTest { protected ErrorClassification classifyException(Exception e) { return classification; } - }, - errorHandler); + }); } } diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java index ef81f436ed..84a6a29c31 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java @@ -28,14 +28,12 @@ import com.google.devtools.build.lib.syntax.ParserInputSource; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.vfs.Path; - +import java.io.IOException; +import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import java.io.IOException; -import java.util.List; - /** * Tests for WorkspaceFactory. */ @@ -137,7 +135,7 @@ public class WorkspaceFactoryTest { this.exception = exception; } - public Package getPackage() { + public Package getPackage() throws InterruptedException { return builder.build(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index d16068eee6..b2bd91a8b8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -158,10 +158,10 @@ public class CppLinkActionTest extends BuildViewTestCase { new ActionCombinationFactory() { @Override - public Action generate(int i) { + public Action generate(int i) throws InterruptedException { CppLinkActionBuilder builder = - new CppLinkActionBuilder(ruleContext, (i & 2) == 0 - ? dynamicOutputFile : staticOutputFile) { + new CppLinkActionBuilder( + ruleContext, (i & 2) == 0 ? dynamicOutputFile : staticOutputFile) { @Override protected Artifact getInterfaceSoBuilder() { return interfaceSoBuilder; @@ -209,10 +209,10 @@ public class CppLinkActionTest extends BuildViewTestCase { new ActionCombinationFactory() { @Override - public Action generate(int i) { + public Action generate(int i) throws InterruptedException { CppLinkActionBuilder builder = - new CppLinkActionBuilder(ruleContext, (i & 2) == 0 - ? staticOutputFile : dynamicOutputFile) { + new CppLinkActionBuilder( + ruleContext, (i & 2) == 0 ? staticOutputFile : dynamicOutputFile) { @Override protected Artifact getInterfaceSoBuilder() { return interfaceSoBuilder; @@ -372,7 +372,8 @@ public class CppLinkActionTest extends BuildViewTestCase { } } - private void assertError(String expectedSubstring, CppLinkActionBuilder builder) { + private static void assertError(String expectedSubstring, CppLinkActionBuilder builder) + throws InterruptedException { try { builder.build(); fail(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java index a21c6f5826..869423e55b 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java @@ -30,13 +30,11 @@ import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.WalkableGraph; - +import java.io.IOException; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import java.io.IOException; - /** Tests for {@link com.google.devtools.build.lib.skyframe.PrepareDepsOfPatternsFunction}. */ @RunWith(JUnit4.class) public class PrepareDepsOfPatternsFunctionTest extends BuildViewTestCase { @@ -250,16 +248,18 @@ public class PrepareDepsOfPatternsFunctionTest extends BuildViewTestCase { scratch.file("bar/BUILD"); } - private void assertValidValue(WalkableGraph graph, SkyKey key) { + private static void assertValidValue(WalkableGraph graph, SkyKey key) + throws InterruptedException { assertValidValue(graph, key, /*expectTransitiveException=*/ false); } /** - * A node in the walkable graph may have both a value and an exception. This happens when one - * of a node's transitive dependencies throws an exception, but its parent recovers from it. + * A node in the walkable graph may have both a value and an exception. This happens when one of a + * node's transitive dependencies throws an exception, but its parent recovers from it. */ - private void assertValidValue( - WalkableGraph graph, SkyKey key, boolean expectTransitiveException) { + private static void assertValidValue( + WalkableGraph graph, SkyKey key, boolean expectTransitiveException) + throws InterruptedException { assertTrue(graph.exists(key)); assertNotNull(graph.getValue(key)); if (expectTransitiveException) { @@ -269,7 +269,8 @@ public class PrepareDepsOfPatternsFunctionTest extends BuildViewTestCase { } } - private Exception assertException(WalkableGraph graph, SkyKey key) { + private static Exception assertException(WalkableGraph graph, SkyKey key) + throws InterruptedException { assertTrue(graph.exists(key)); assertNull(graph.getValue(key)); Exception exception = graph.getException(key); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java index de989417da..7f08e04d95 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeAwareActionTest.java @@ -248,7 +248,8 @@ public class SkyframeAwareActionTest extends TimestampBuilderTestCase { } @Override - public void establishSkyframeDependencies(Environment env) throws ExceptionBase { + public void establishSkyframeDependencies(Environment env) + throws ExceptionBase, InterruptedException { // Establish some Skyframe dependency. A real action would then use this to compute and // cache data for the execute(...) method. env.getValue(actionDepKey); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java index 25ddfc3b3c..80650fdf84 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java @@ -48,9 +48,6 @@ import com.google.devtools.build.skyframe.DelegatingWalkableGraph; import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.WalkableGraph; - -import org.junit.Before; - import java.io.IOException; import java.util.ArrayList; import java.util.Collection; @@ -58,8 +55,8 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; - import javax.annotation.Nullable; +import org.junit.Before; abstract public class SkyframeLabelVisitorTestCase extends PackageLoadingTestCase { // Convenience constants, so test args are readable vs true/false @@ -179,7 +176,8 @@ abstract public class SkyframeLabelVisitorTestCase extends PackageLoadingTestCas * loaded targets. */ public static Set<Label> getVisitedLabels( - Iterable<Label> startingLabels, SkyframeExecutor skyframeExecutor) { + Iterable<Label> startingLabels, SkyframeExecutor skyframeExecutor) + throws InterruptedException { final WalkableGraph graph = new DelegatingWalkableGraph( ((InMemoryMemoizingEvaluator) skyframeExecutor.getEvaluatorForTesting()) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceASTFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceASTFunctionTest.java index 4d61274456..832cb1c5c7 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceASTFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceASTFunctionTest.java @@ -27,7 +27,8 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; - +import java.io.IOException; +import java.util.List; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -35,9 +36,6 @@ import org.junit.runners.JUnit4; import org.mockito.Matchers; import org.mockito.Mockito; -import java.io.IOException; -import java.util.List; - /** * Test for WorkspaceASTFunction. */ @@ -65,7 +63,7 @@ public class WorkspaceASTFunctionTest extends BuildViewTestCase { workspacePath.getParentDirectory(), new PathFragment(workspacePath.getBaseName())); } - private SkyFunction.Environment getEnv() { + private SkyFunction.Environment getEnv() throws InterruptedException { SkyFunction.Environment env = Mockito.mock(SkyFunction.Environment.class); Mockito.when(env.getValue(Matchers.argThat(new SkyKeyMatchers(SkyFunctions.FILE)))) .thenReturn(fakeWorkspaceFileValue); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java index 612debd861..6a5955721a 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java @@ -36,7 +36,7 @@ import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; - +import java.io.IOException; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.junit.Before; @@ -48,8 +48,6 @@ import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import java.io.IOException; - /** * Test for {@link WorkspaceFileFunction}. */ @@ -148,7 +146,7 @@ public class WorkspaceFileFunctionTest extends BuildViewTestCase { public void describeTo(Description description) {} } - private SkyFunction.Environment getEnv() { + private SkyFunction.Environment getEnv() throws InterruptedException { SkyFunction.Environment env = Mockito.mock(SkyFunction.Environment.class); Mockito.when(env.getValue(Matchers.argThat(new SkyKeyMatchers(SkyFunctions.FILE)))) .thenReturn(fakeWorkspaceFileValue); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java b/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java index 34218e67c8..34fb706b09 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/util/SkyframeExecutorTestUtils.java @@ -34,10 +34,8 @@ import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.MemoizingEvaluator; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; - import java.util.Collection; import java.util.List; - import javax.annotation.Nullable; /** @@ -57,11 +55,11 @@ public class SkyframeExecutorTestUtils { } /** - * Returns an existing error info, or {@code null} if the given key is not currently in the - * graph. + * Returns an existing error info, or {@code null} if the given key is not currently in the graph. */ @Nullable - public static ErrorInfo getExistingError(SkyframeExecutor skyframeExecutor, SkyKey key) { + public static ErrorInfo getExistingError(SkyframeExecutor skyframeExecutor, SkyKey key) + throws InterruptedException { return skyframeExecutor.getEvaluatorForTesting().getExistingErrorForTesting(key); } @@ -165,10 +163,11 @@ public class SkyframeExecutorTestUtils { * Returns the error info for an existing target value, or {@code null} if there is not an * appropriate target value key in the graph. * - * This helper is provided so legacy tests don't need to know about details of skyframe keys. + * <p>This helper is provided so legacy tests don't need to know about details of skyframe keys. */ @Nullable - public static ErrorInfo getExistingFailedTarget(SkyframeExecutor skyframeExecutor, Label label) { + public static ErrorInfo getExistingFailedTarget(SkyframeExecutor skyframeExecutor, Label label) + throws InterruptedException { SkyKey key = TargetMarkerValue.key(label); return getExistingError(skyframeExecutor, key); } diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java index f2a1560f54..b27d2ef6e7 100644 --- a/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java +++ b/src/test/java/com/google/devtools/build/skyframe/DeterministicHelper.java @@ -20,7 +20,6 @@ import java.util.Map; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; - import javax.annotation.Nullable; /** @@ -77,7 +76,8 @@ public class DeterministicHelper extends NotifyingHelper { return entry == null ? null : new DeterministicValueEntry(key, entry); } - private static Map<SkyKey, NodeEntry> makeDeterministic(Map<SkyKey, NodeEntry> map) { + private static Map<SkyKey, ? extends NodeEntry> makeDeterministic( + Map<SkyKey, ? extends NodeEntry> map) { Map<SkyKey, NodeEntry> result = new TreeMap<>(ALPHABETICAL_SKYKEY_COMPARATOR); result.putAll(map); return result; @@ -89,10 +89,9 @@ public class DeterministicHelper extends NotifyingHelper { } @Override - public Map<SkyKey, NodeEntry> getBatch( - @Nullable SkyKey requestor, - Reason reason, - Iterable<SkyKey> keys) { + public Map<SkyKey, ? extends NodeEntry> getBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) + throws InterruptedException { return makeDeterministic(super.getBatch(requestor, reason, keys)); } } @@ -112,16 +111,16 @@ public class DeterministicHelper extends NotifyingHelper { } @Override - public Map<SkyKey, NodeEntry> createIfAbsentBatch( - @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) { + public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) + throws InterruptedException { return makeDeterministic(super.createIfAbsentBatch(requestor, reason, keys)); } @Override - public Map<SkyKey, NodeEntry> getBatch( - @Nullable SkyKey requestor, - Reason reason, - Iterable<SkyKey> keys) { + public Map<SkyKey, ? extends NodeEntry> getBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) + throws InterruptedException { return makeDeterministic(super.getBatch(requestor, reason, keys)); } } diff --git a/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java b/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java index a0233838ec..79904d0bff 100644 --- a/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java +++ b/src/test/java/com/google/devtools/build/skyframe/DeterministicInMemoryGraph.java @@ -14,6 +14,7 @@ package com.google.devtools.build.skyframe; import java.util.Map; +import javax.annotation.Nullable; /** * {@link DeterministicHelper.DeterministicProcessableGraph} that implements the {@link @@ -27,6 +28,36 @@ class DeterministicInMemoryGraph extends DeterministicHelper.DeterministicProces } @Override + public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) { + try { + return super.createIfAbsentBatch(requestor, reason, keys); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + + @Nullable + @Override + public NodeEntry get(@Nullable SkyKey requestor, Reason reason, SkyKey key) { + try { + return super.get(requestor, reason, key); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + + @Override + public Map<SkyKey, ? extends NodeEntry> getBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) { + try { + return super.getBatch(requestor, reason, keys); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + + @Override public Map<SkyKey, SkyValue> getValues() { return ((InMemoryGraph) delegate).getValues(); } @@ -36,8 +67,9 @@ class DeterministicInMemoryGraph extends DeterministicHelper.DeterministicProces return ((InMemoryGraph) delegate).getDoneValues(); } + @Override - public Map<SkyKey, NodeEntry> getAllValues() { + public Map<SkyKey, ? extends NodeEntry> getAllValues() { return ((InMemoryGraph) delegate).getAllValues(); } } 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 5c99281a61..c86d6cfd48 100644 --- a/src/test/java/com/google/devtools/build/skyframe/EvaluationResultSubject.java +++ b/src/test/java/com/google/devtools/build/skyframe/EvaluationResultSubject.java @@ -53,13 +53,13 @@ public class EvaluationResultSubject extends Subject<EvaluationResultSubject, Ev .named("Error entry for " + getDisplaySubject()); } - public IterableSubject hasDirectDepsInGraphThat(SkyKey parent) { + public IterableSubject hasDirectDepsInGraphThat(SkyKey parent) throws InterruptedException { return assertThat( getSubject().getWalkableGraph().getDirectDeps(ImmutableList.of(parent)).get(parent)) .named("Direct deps for " + parent + " in " + getDisplaySubject()); } - public IterableSubject hasReverseDepsInGraphThat(SkyKey child) { + public IterableSubject hasReverseDepsInGraphThat(SkyKey child) throws InterruptedException { return assertThat( getSubject().getWalkableGraph().getReverseDeps(ImmutableList.of(child)).get(child)) .named("Reverse deps for " + child + " in " + getDisplaySubject()); diff --git a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java index ca011cbb31..fa5a7dfaf7 100644 --- a/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/GraphConcurrencyTest.java @@ -31,9 +31,6 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.skyframe.GraphTester.StringValue; import com.google.devtools.build.skyframe.NodeEntry.DependencyState; import com.google.devtools.build.skyframe.QueryableGraph.Reason; -import org.junit.Before; -import org.junit.Test; - import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -43,6 +40,8 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import org.junit.Before; +import org.junit.Test; /** Base class for concurrency sanity tests on {@link EvaluableGraph} implementations. */ public abstract class GraphConcurrencyTest { @@ -76,12 +75,12 @@ public abstract class GraphConcurrencyTest { } @Test - public void createIfAbsentBatchSanity() { + public void createIfAbsentBatchSanity() throws InterruptedException { graph.createIfAbsentBatch(null, Reason.OTHER, ImmutableList.of(key("cat"), key("dog"))); } @Test - public void createIfAbsentConcurrentWithGet() { + public void createIfAbsentConcurrentWithGet() throws InterruptedException { int numIters = 50; final SkyKey key = key("key"); for (int i = 0; i < numIters; i++) { @@ -91,7 +90,11 @@ public abstract class GraphConcurrencyTest { new Runnable() { @Override public void run() { - graph.get(null, Reason.OTHER, key); + try { + graph.get(null, Reason.OTHER, key); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } } })); t.start(); @@ -111,7 +114,11 @@ public abstract class GraphConcurrencyTest { public void run() { TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions( startThreads, "threads not started"); - graph.createIfAbsentBatch(null, Reason.OTHER, ImmutableList.of(key)); + try { + graph.createIfAbsentBatch(null, Reason.OTHER, ImmutableList.of(key)); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } } }; Runnable noCreateRunnable = @@ -120,7 +127,11 @@ public abstract class GraphConcurrencyTest { public void run() { TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions( startThreads, "threads not started"); - graph.get(null, Reason.OTHER, key); + try { + graph.get(null, Reason.OTHER, key); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } } }; List<Thread> threads = new ArrayList<>(2 * numThreads); @@ -236,13 +247,22 @@ public abstract class GraphConcurrencyTest { new Runnable() { public void run() { for (SkyKey key : keys) { - NodeEntry entry = graph.get(null, Reason.OTHER, key); + NodeEntry entry = null; + try { + entry = graph.get(null, Reason.OTHER, key); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } if (entry == null) { nodeCreated.add(key); } } - Map<SkyKey, NodeEntry> entries = - graph.createIfAbsentBatch(null, Reason.OTHER, keys); + Map<SkyKey, ? extends NodeEntry> entries; + try { + entries = graph.createIfAbsentBatch(null, Reason.OTHER, keys); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } for (Integer keyNum : ImmutableList.of(keyNum1, keyNum2)) { SkyKey key = key("foo" + keyNum); NodeEntry entry = entries.get(key); @@ -251,12 +271,20 @@ public abstract class GraphConcurrencyTest { if (startEvaluation(entry).equals(DependencyState.NEEDS_SCHEDULING)) { assertTrue(valuesSet.add(key)); // Set to done. - entry.setValue(new StringValue("bar" + keyNum), startingVersion); + try { + entry.setValue(new StringValue("bar" + keyNum), startingVersion); + } catch (InterruptedException e) { + throw new IllegalStateException(key + ", " + entry, e); + } assertThat(entry.isDone()).isTrue(); } } // This shouldn't cause any problems from the other threads. - graph.createIfAbsentBatch(null, Reason.OTHER, keys); + try { + graph.createIfAbsentBatch(null, Reason.OTHER, keys); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } } }; pool.execute(wrapper.wrap(r)); @@ -278,8 +306,7 @@ public abstract class GraphConcurrencyTest { /** * Initially calling {@link NodeEntry#setValue} and then making sure concurrent calls to {@link - * QueryableGraph#get} and {@link QueryableGraph#getBatchWithFieldHints} do not interfere with the - * node. + * QueryableGraph#get} and {@link QueryableGraph#getBatch} do not interfere with the node. */ @Test public void testDoneToDirty() throws Exception { @@ -291,7 +318,7 @@ public abstract class GraphConcurrencyTest { for (int i = 0; i < numKeys; i++) { keys.add(key("foo" + i)); } - Map<SkyKey, NodeEntry> entries = graph.createIfAbsentBatch(null, Reason.OTHER, keys); + Map<SkyKey, ? extends NodeEntry> entries = graph.createIfAbsentBatch(null, Reason.OTHER, keys); for (int i = 0; i < numKeys; i++) { NodeEntry entry = entries.get(key("foo" + i)); startEvaluation(entry); @@ -325,15 +352,28 @@ public abstract class GraphConcurrencyTest { } catch (InterruptedException e) { throw new AssertionError(e); } - NodeEntry entry = graph.get(null, Reason.OTHER, key("foo" + keyNum)); - entry.markDirty(true); + NodeEntry entry = null; + try { + entry = graph.get(null, Reason.OTHER, key("foo" + keyNum)); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + try { + entry.markDirty(true); + } catch (InterruptedException e) { + throw new IllegalStateException(keyNum + ", " + entry, e); + } // Make some changes, like adding a dep and rdep. entry.addReverseDepAndCheckIfDone(key("rdep")); entry.markRebuilding(); addTemporaryDirectDep(entry, key("dep")); entry.signalDep(); // Move node from dirty back to done. - entry.setValue(new StringValue("bar" + keyNum), getNextVersion(startingVersion)); + try { + entry.setValue(new StringValue("bar" + keyNum), getNextVersion(startingVersion)); + } catch (InterruptedException e) { + throw new IllegalStateException(keyNum + ", " + entry, e); + } } }; @@ -347,13 +387,18 @@ public abstract class GraphConcurrencyTest { } catch (InterruptedException e) { throw new AssertionError(e); } - NodeEntry entry = graph.get(null, Reason.OTHER, key("foo" + keyNum)); + NodeEntry entry = null; + try { + entry = graph.get(null, Reason.OTHER, key("foo" + keyNum)); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } assertNotNull(entry); // Requests for the value are made at the same time that the version increments from // the base. Check that there is no problem in requesting the version and that the // number is sane. - assertThat(entry.getVersion()).isAnyOf(startingVersion, - getNextVersion(startingVersion)); + assertThat(entry.getVersion()) + .isAnyOf(startingVersion, getNextVersion(startingVersion)); getCountDownLatch.countDown(); } }; @@ -380,7 +425,12 @@ public abstract class GraphConcurrencyTest { } catch (InterruptedException e) { throw new AssertionError(e); } - Map<SkyKey, NodeEntry> batchMap = graph.getBatch(null, Reason.OTHER, batch); + Map<SkyKey, ? extends NodeEntry> batchMap = null; + try { + batchMap = graph.getBatch(null, Reason.OTHER, batch); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } getBatchCountDownLatch.countDown(); assertThat(batchMap).hasSize(batch.size()); for (NodeEntry entry : batchMap.values()) { @@ -411,7 +461,7 @@ public abstract class GraphConcurrencyTest { } } - private DependencyState startEvaluation(NodeEntry entry) { + private static DependencyState startEvaluation(NodeEntry entry) { return entry.addReverseDepAndCheckIfDone(null); } 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 23c1fca413..00681c8d3b 100644 --- a/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryNodeEntryTest.java @@ -32,16 +32,13 @@ import com.google.devtools.build.lib.util.GroupedList.GroupedListHelper; import com.google.devtools.build.skyframe.NodeEntry.DependencyState; import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.util.ArrayList; import java.util.List; import java.util.Set; - import javax.annotation.Nullable; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * Tests for {@link InMemoryNodeEntry}. @@ -69,7 +66,7 @@ public class InMemoryNodeEntryTest { } @Test - public void signalEntry() { + public void signalEntry() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep1 = key("dep1"); @@ -95,7 +92,7 @@ public class InMemoryNodeEntryTest { } @Test - public void reverseDeps() { + public void reverseDeps() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); SkyKey mother = key("mother"); SkyKey father = key("father"); @@ -111,7 +108,7 @@ public class InMemoryNodeEntryTest { } @Test - public void errorValue() { + public void errorValue() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( @@ -125,7 +122,7 @@ public class InMemoryNodeEntryTest { } @Test - public void errorAndValue() { + public void errorAndValue() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. ReifiedSkyFunctionException exception = new ReifiedSkyFunctionException( @@ -138,7 +135,7 @@ public class InMemoryNodeEntryTest { } @Test - public void crashOnNullErrorAndValue() { + public void crashOnNullErrorAndValue() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. try { @@ -162,7 +159,7 @@ public class InMemoryNodeEntryTest { } @Test - public void crashOnDifferentValue() { + public void crashOnDifferentValue() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L); @@ -176,7 +173,7 @@ public class InMemoryNodeEntryTest { } @Test - public void dirtyLifecycle() { + public void dirtyLifecycle() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -208,7 +205,7 @@ public class InMemoryNodeEntryTest { } @Test - public void changedLifecycle() { + public void changedLifecycle() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -237,7 +234,7 @@ public class InMemoryNodeEntryTest { } @Test - public void markDirtyThenChanged() { + public void markDirtyThenChanged() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. addTemporaryDirectDep(entry, key("dep")); @@ -261,7 +258,7 @@ public class InMemoryNodeEntryTest { @Test - public void markChangedThenDirty() { + public void markChangedThenDirty() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. addTemporaryDirectDep(entry, key("dep")); @@ -284,7 +281,7 @@ public class InMemoryNodeEntryTest { } @Test - public void crashOnTwiceMarkedChanged() { + public void crashOnTwiceMarkedChanged() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L); @@ -300,7 +297,7 @@ public class InMemoryNodeEntryTest { } @Test - public void crashOnTwiceMarkedDirty() { + public void crashOnTwiceMarkedDirty() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. addTemporaryDirectDep(entry, key("dep")); @@ -316,7 +313,7 @@ public class InMemoryNodeEntryTest { } @Test - public void crashOnAddReverseDepTwice() { + public void crashOnAddReverseDepTwice() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); SkyKey parent = key("parent"); assertEquals(DependencyState.NEEDS_SCHEDULING, entry.addReverseDepAndCheckIfDone(parent)); @@ -331,7 +328,7 @@ public class InMemoryNodeEntryTest { } @Test - public void crashOnAddReverseDepTwiceAfterDone() { + public void crashOnAddReverseDepTwiceAfterDone() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L); @@ -348,7 +345,7 @@ public class InMemoryNodeEntryTest { } @Test - public void crashOnAddReverseDepBeforeAfterDone() { + public void crashOnAddReverseDepBeforeAfterDone() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); SkyKey parent = key("parent"); assertEquals(DependencyState.NEEDS_SCHEDULING, entry.addReverseDepAndCheckIfDone(parent)); @@ -364,7 +361,7 @@ public class InMemoryNodeEntryTest { } @Test - public void pruneBeforeBuild() { + public void pruneBeforeBuild() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); SkyKey dep = key("dep"); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. @@ -412,7 +409,7 @@ public class InMemoryNodeEntryTest { } @Test - public void pruneAfterBuild() { + public void pruneAfterBuild() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -435,7 +432,7 @@ public class InMemoryNodeEntryTest { } @Test - public void noPruneWhenDetailsChange() { + public void noPruneWhenDetailsChange() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -471,7 +468,7 @@ public class InMemoryNodeEntryTest { } @Test - public void pruneWhenDepGroupReordered() { + public void pruneWhenDepGroupReordered() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -511,7 +508,7 @@ public class InMemoryNodeEntryTest { } @Test - public void errorInfoCannotBePruned() { + public void errorInfoCannotBePruned() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -538,7 +535,7 @@ public class InMemoryNodeEntryTest { } @Test - public void getDependencyGroup() { + public void getDependencyGroup() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -562,7 +559,7 @@ public class InMemoryNodeEntryTest { } @Test - public void maintainDependencyGroupAfterRemoval() { + public void maintainDependencyGroupAfterRemoval() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -592,7 +589,7 @@ public class InMemoryNodeEntryTest { } @Test - public void pruneWhenDepsChange() { + public void pruneWhenDepsChange() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. SkyKey dep = key("dep"); @@ -616,7 +613,7 @@ public class InMemoryNodeEntryTest { } @Test - public void checkDepsOneByOne() { + public void checkDepsOneByOne() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(null); // Start evaluation. List<SkyKey> deps = new ArrayList<>(); @@ -643,7 +640,7 @@ public class InMemoryNodeEntryTest { } @Test - public void signalOnlyNewParents() { + public void signalOnlyNewParents() throws InterruptedException { NodeEntry entry = new InMemoryNodeEntry(); entry.addReverseDepAndCheckIfDone(key("parent")); setValue(entry, new SkyValue() {}, /*errorInfo=*/null, /*graphVersion=*/0L); @@ -657,7 +654,7 @@ public class InMemoryNodeEntryTest { } @Test - public void testClone() { + public void testClone() throws InterruptedException { InMemoryNodeEntry entry = new InMemoryNodeEntry(); IntVersion version = IntVersion.of(0); IntegerValue originalValue = new IntegerValue(42); @@ -701,7 +698,7 @@ public class InMemoryNodeEntryTest { } @Test - public void getGroupedDirectDeps() { + public void getGroupedDirectDeps() throws InterruptedException { InMemoryNodeEntry entry = new InMemoryNodeEntry(); ImmutableList<ImmutableSet<SkyKey>> groupedDirectDeps = ImmutableList.of( ImmutableSet.of(key("1A")), @@ -726,8 +723,9 @@ public class InMemoryNodeEntryTest { } } - private static Set<SkyKey> setValue(NodeEntry entry, SkyValue value, - @Nullable ErrorInfo errorInfo, long graphVersion) { + private static Set<SkyKey> setValue( + NodeEntry entry, SkyValue value, @Nullable ErrorInfo errorInfo, long graphVersion) + throws InterruptedException { return entry.setValue( ValueWithMetadata.normal(value, errorInfo, NO_EVENTS), IntVersion.of(graphVersion)); } 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 7b55d6f37c..0afcb62c5c 100644 --- a/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/MemoizingEvaluatorTest.java @@ -33,6 +33,7 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.base.Predicates; +import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -987,7 +988,7 @@ public class MemoizingEvaluatorTest { new SkyFunction() { @Nullable @Override - public SkyValue compute(SkyKey skyKey, Environment env) { + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { env.getValues(ImmutableList.of(leafKey, bKey)); return null; } @@ -1229,17 +1230,20 @@ public class MemoizingEvaluatorTest { tester.set(dep2, new StringValue("dep2")); // otherTop should request the deps one at a time, so that it can be in the CHECK_DEPENDENCIES // state even after one dep is re-evaluated. - tester.getOrCreate(otherTop).setBuilder(new NoExtractorFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) { - env.getValue(dep1); - if (env.valuesMissing()) { - return null; - } - env.getValue(dep2); - return env.valuesMissing() ? null : new StringValue("otherTop"); - } - }); + tester + .getOrCreate(otherTop) + .setBuilder( + new NoExtractorFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { + env.getValue(dep1); + if (env.valuesMissing()) { + return null; + } + env.getValue(dep2); + return env.valuesMissing() ? null : new StringValue("otherTop"); + } + }); // Prime the graph with otherTop, so we can dirty it next build. assertEquals(new StringValue("otherTop"), tester.evalAndGet(/*keepGoing=*/false, otherTop)); // Mark dep1 changed, so otherTop will be dirty and request re-evaluation of dep1. @@ -1787,23 +1791,28 @@ public class MemoizingEvaluatorTest { tester.set(groupDepA, new StringValue("depC")); tester.set(groupDepB, new StringValue("")); tester.getOrCreate(depC).setHasError(true); - tester.getOrCreate(topKey).setBuilder(new NoExtractorFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - StringValue val = ((StringValue) env.getValues( - ImmutableList.of(groupDepA, groupDepB)).get(groupDepA)); - if (env.valuesMissing()) { - return null; - } - String nextDep = val.getValue(); - try { - env.getValueOrThrow(GraphTester.toSkyKey(nextDep), SomeErrorException.class); - } catch (SomeErrorException e) { - throw new GenericFunctionException(e, Transience.PERSISTENT); - } - return env.valuesMissing() ? null : new StringValue("top"); - } - }); + tester + .getOrCreate(topKey) + .setBuilder( + new NoExtractorFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + StringValue val = + ((StringValue) + env.getValues(ImmutableList.of(groupDepA, groupDepB)).get(groupDepA)); + if (env.valuesMissing()) { + return null; + } + String nextDep = val.getValue(); + try { + env.getValueOrThrow(GraphTester.toSkyKey(nextDep), SomeErrorException.class); + } catch (SomeErrorException e) { + throw new GenericFunctionException(e, Transience.PERSISTENT); + } + return env.valuesMissing() ? null : new StringValue("top"); + } + }); EvaluationResult<StringValue> evaluationResult = tester.eval( /*keepGoing=*/true, groupDepA, depC); @@ -1892,20 +1901,25 @@ public class MemoizingEvaluatorTest { tester.set(firstKey, new StringValue("biding")); tester.set(slowAddingDep, new StringValue("dep")); final AtomicInteger numTopInvocations = new AtomicInteger(0); - tester.getOrCreate(top).setBuilder(new NoExtractorFunction() { - @Override - public SkyValue compute(SkyKey key, SkyFunction.Environment env) { - numTopInvocations.incrementAndGet(); - if (delayTopSignaling.get()) { - // The reporter will be given firstKey's warning to emit when it is requested as a dep - // below, if firstKey is already built, so we release the reporter's latch beforehand. - topRestartedBuild.countDown(); - } - // top's builder just requests both deps in a group. - env.getValuesOrThrow(ImmutableList.of(firstKey, slowAddingDep), SomeErrorException.class); - return env.valuesMissing() ? null : new StringValue("top"); - } - }); + tester + .getOrCreate(top) + .setBuilder( + new NoExtractorFunction() { + @Override + public SkyValue compute(SkyKey key, SkyFunction.Environment env) + throws InterruptedException { + numTopInvocations.incrementAndGet(); + if (delayTopSignaling.get()) { + // The reporter will be given firstKey's warning to emit when it is requested as a dep + // below, if firstKey is already built, so we release the reporter's latch beforehand. + topRestartedBuild.countDown(); + } + // top's builder just requests both deps in a group. + env.getValuesOrThrow( + ImmutableList.of(firstKey, slowAddingDep), SomeErrorException.class); + return env.valuesMissing() ? null : new StringValue("top"); + } + }); reporter = new DelegatingEventHandler(reporter) { @Override @@ -2581,7 +2595,7 @@ public class MemoizingEvaluatorTest { .setBuilder( new SkyFunction() { @Override - public SkyValue compute(SkyKey skyKey, Environment env) { + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { topEvaluated.set(true); return env.getValue(leaf) == null ? null : fixedTopValue; } @@ -2636,7 +2650,7 @@ public class MemoizingEvaluatorTest { .setBuilder( new SkyFunction() { @Override - public SkyValue compute(SkyKey skyKey, Environment env) { + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { topEvaluated.set(true); return env.getValue(other) == null || env.getValue(leaf) == null @@ -2682,27 +2696,29 @@ public class MemoizingEvaluatorTest { public void changedChildChangesDepOfParent() throws Exception { initializeTester(); final SkyKey buildFile = GraphTester.toSkyKey("buildFile"); - ValueComputer authorDrink = new ValueComputer() { - @Override - public SkyValue compute(Map<SkyKey, SkyValue> deps, SkyFunction.Environment env) { - String author = ((StringValue) deps.get(buildFile)).getValue(); - StringValue beverage; - switch (author) { - case "hemingway": - beverage = (StringValue) env.getValue(GraphTester.toSkyKey("absinthe")); - break; - case "joyce": - beverage = (StringValue) env.getValue(GraphTester.toSkyKey("whiskey")); - break; - default: - throw new IllegalStateException(author); - } - if (beverage == null) { - return null; - } - return new StringValue(author + " drank " + beverage.getValue()); - } - }; + ValueComputer authorDrink = + new ValueComputer() { + @Override + public SkyValue compute(Map<SkyKey, SkyValue> deps, SkyFunction.Environment env) + throws InterruptedException { + String author = ((StringValue) deps.get(buildFile)).getValue(); + StringValue beverage; + switch (author) { + case "hemingway": + beverage = (StringValue) env.getValue(GraphTester.toSkyKey("absinthe")); + break; + case "joyce": + beverage = (StringValue) env.getValue(GraphTester.toSkyKey("whiskey")); + break; + default: + throw new IllegalStateException(author); + } + if (beverage == null) { + return null; + } + return new StringValue(author + " drank " + beverage.getValue()); + } + }; tester.set(buildFile, new StringValue("hemingway")); SkyKey absinthe = GraphTester.toSkyKey("absinthe"); @@ -3267,7 +3283,8 @@ public class MemoizingEvaluatorTest { .setBuilder( new SkyFunction() { @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { otherStarted.countDown(); int invocations = numOtherInvocations.incrementAndGet(); // And given that otherErrorKey waits for errorKey's error to be committed before @@ -3865,6 +3882,15 @@ public class MemoizingEvaluatorTest { */ @Test public void shutDownBuildOnCachedError_Verified() throws Exception { + // TrackingInvalidationReceiver does unnecessary examination of node values. + initializeTester( + new TrackingInvalidationReceiver() { + @Override + public void evaluated( + SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) { + evaluated.add(skyKey); + } + }); // errorKey will be invalidated due to its dependence on invalidatedKey, but later revalidated // since invalidatedKey re-evaluates to the same value on a subsequent build. SkyKey errorKey = GraphTester.toSkyKey("error"); @@ -3991,6 +4017,15 @@ public class MemoizingEvaluatorTest { */ @Test public void cachedErrorCausesRestart() throws Exception { + // TrackingInvalidationReceiver does unnecessary examination of node values. + initializeTester( + new TrackingInvalidationReceiver() { + @Override + public void evaluated( + SkyKey skyKey, Supplier<SkyValue> skyValueSupplier, EvaluationState state) { + evaluated.add(skyKey); + } + }); final SkyKey errorKey = GraphTester.toSkyKey("error"); SkyKey invalidatedKey = GraphTester.toSkyKey("invalidated"); final SkyKey topKey = GraphTester.toSkyKey("top"); @@ -4074,22 +4109,23 @@ public class MemoizingEvaluatorTest { SkyKey parent2Key = GraphTester.toSkyKey("parent2"); final SkyKey errorKey = GraphTester.toSkyKey("error"); final SkyKey otherKey = GraphTester.toSkyKey("other"); - SkyFunction parentBuilder = new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) { - env.getValue(errorKey); - env.getValue(otherKey); - if (env.valuesMissing()) { - return null; - } - return new StringValue("parent"); - } + SkyFunction parentBuilder = + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { + env.getValue(errorKey); + env.getValue(otherKey); + if (env.valuesMissing()) { + return null; + } + return new StringValue("parent"); + } - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }; + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }; tester.getOrCreate(parent1Key).setBuilder(parentBuilder); tester.getOrCreate(parent2Key).setBuilder(parentBuilder); tester.getOrCreate(errorKey).setConstantValue(new StringValue("no error yet")); @@ -4400,12 +4436,12 @@ public class MemoizingEvaluatorTest { } @Nullable - public SkyValue getExistingValue(SkyKey key) { + public SkyValue getExistingValue(SkyKey key) throws InterruptedException { return driver.getExistingValueForTesting(key); } @Nullable - public SkyValue getExistingValue(String key) { + public SkyValue getExistingValue(String key) throws InterruptedException { return getExistingValue(toSkyKey(key)); } } diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java index ad9a591eb8..5adbe45eb6 100644 --- a/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java +++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingHelper.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.util.GroupedList; import java.util.Map; import java.util.Set; - import javax.annotation.Nullable; /** @@ -87,10 +86,9 @@ public class NotifyingHelper { } @Override - public Map<SkyKey, NodeEntry> getBatch( - @Nullable SkyKey requestor, - Reason reason, - Iterable<SkyKey> keys) { + public Map<SkyKey, ? extends NodeEntry> getBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) + throws InterruptedException { return Maps.transformEntries( delegate.getBatch(requestor, reason, keys), notifyingHelper.wrapEntry); @@ -98,7 +96,8 @@ public class NotifyingHelper { @Nullable @Override - public NodeEntry get(@Nullable SkyKey requestor, Reason reason, SkyKey key) { + public NodeEntry get(@Nullable SkyKey requestor, Reason reason, SkyKey key) + throws InterruptedException { return notifyingHelper.wrapEntry(key, delegate.get(requestor, reason, key)); } } @@ -122,8 +121,9 @@ public class NotifyingHelper { } @Override - public Map<SkyKey, NodeEntry> createIfAbsentBatch( - @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) { + public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) + throws InterruptedException { for (SkyKey key : keys) { notifyingHelper.graphListener.accept(key, EventType.CREATE_IF_ABSENT, Order.BEFORE, null); } @@ -245,7 +245,7 @@ public class NotifyingHelper { } @Override - public Set<SkyKey> setValue(SkyValue value, Version version) { + public Set<SkyKey> setValue(SkyValue value, Version version) throws InterruptedException { graphListener.accept(myKey, EventType.SET_VALUE, Order.BEFORE, value); Set<SkyKey> result = super.setValue(value, version); graphListener.accept(myKey, EventType.SET_VALUE, Order.AFTER, value); @@ -253,7 +253,7 @@ public class NotifyingHelper { } @Override - public MarkedDirtyResult markDirty(boolean isChanged) { + public MarkedDirtyResult markDirty(boolean isChanged) throws InterruptedException { graphListener.accept(myKey, EventType.MARK_DIRTY, Order.BEFORE, isChanged); MarkedDirtyResult result = super.markDirty(isChanged); graphListener.accept(myKey, EventType.MARK_DIRTY, Order.AFTER, isChanged); @@ -261,7 +261,7 @@ public class NotifyingHelper { } @Override - public Set<SkyKey> markClean() { + public Set<SkyKey> markClean() throws InterruptedException { graphListener.accept(myKey, EventType.MARK_CLEAN, Order.BEFORE, this); Set<SkyKey> result = super.markClean(); graphListener.accept(myKey, EventType.MARK_CLEAN, Order.AFTER, this); @@ -287,7 +287,7 @@ public class NotifyingHelper { } @Override - public SkyValue getValueMaybeWithMetadata() { + public SkyValue getValueMaybeWithMetadata() throws InterruptedException { graphListener.accept(myKey, EventType.GET_VALUE_WITH_METADATA, Order.BEFORE, this); return super.getValueMaybeWithMetadata(); } diff --git a/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java b/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java index 752dac7001..31242dac47 100644 --- a/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java +++ b/src/test/java/com/google/devtools/build/skyframe/NotifyingInMemoryGraph.java @@ -14,6 +14,7 @@ package com.google.devtools.build.skyframe; import java.util.Map; +import javax.annotation.Nullable; /** {@link NotifyingHelper} that additionally implements the {@link InMemoryGraph} interface. */ class NotifyingInMemoryGraph extends NotifyingHelper.NotifyingProcessableGraph @@ -23,6 +24,36 @@ class NotifyingInMemoryGraph extends NotifyingHelper.NotifyingProcessableGraph } @Override + public Map<SkyKey, ? extends NodeEntry> createIfAbsentBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) { + try { + return super.createIfAbsentBatch(requestor, reason, keys); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + + @Nullable + @Override + public NodeEntry get(@Nullable SkyKey requestor, Reason reason, SkyKey key) { + try { + return super.get(requestor, reason, key); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + + @Override + public Map<SkyKey, ? extends NodeEntry> getBatch( + @Nullable SkyKey requestor, Reason reason, Iterable<SkyKey> keys) { + try { + return super.getBatch(requestor, reason, keys); + } catch (InterruptedException e) { + throw new IllegalStateException(e); + } + } + + @Override public Map<SkyKey, SkyValue> getValues() { return ((InMemoryGraph) delegate).getValues(); } @@ -33,7 +64,7 @@ class NotifyingInMemoryGraph extends NotifyingHelper.NotifyingProcessableGraph } @Override - public Map<SkyKey, NodeEntry> getAllValues() { + public Map<SkyKey, ? extends NodeEntry> getAllValues() { return ((InMemoryGraph) delegate).getAllValues(); } } 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 9c87572818..c8464b4291 100644 --- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java @@ -48,14 +48,6 @@ import com.google.devtools.build.skyframe.NotifyingHelper.Listener; import com.google.devtools.build.skyframe.NotifyingHelper.Order; import com.google.devtools.build.skyframe.ParallelEvaluator.EventFilter; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; - -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -67,8 +59,13 @@ import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; - import javax.annotation.Nullable; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * Tests for {@link ParallelEvaluator}. @@ -202,36 +199,38 @@ public class ParallelEvaluatorTest { */ @Test public void interruptedEvaluatorThread() throws Exception { - runInterruptionTest(new SkyFunctionFactory() { - @Override - public SkyFunction create(final Semaphore threadStarted, final String[] errorMessage) { - return new SkyFunction() { - // No need to synchronize access to this field; we always request just one more - // dependency, so it's only one SkyFunction running at any time. - private int valueIdCounter = 0; - + runInterruptionTest( + new SkyFunctionFactory() { @Override - public SkyValue compute(SkyKey key, Environment env) { - // Signal the waiting test thread that the Evaluator thread has really started. - threadStarted.release(); + public SkyFunction create(final Semaphore threadStarted, final String[] errorMessage) { + return new SkyFunction() { + // No need to synchronize access to this field; we always request just one more + // dependency, so it's only one SkyFunction running at any time. + private int valueIdCounter = 0; - // Keep the evaluator busy until the test's thread gets scheduled and can - // interrupt the Evaluator's thread. - env.getValue(GraphTester.toSkyKey("a" + valueIdCounter++)); + @Override + public SkyValue compute(SkyKey key, Environment env) throws InterruptedException { + // Signal the waiting test thread that the Evaluator thread has really started. + threadStarted.release(); - // This method never throws InterruptedException, therefore it's the responsibility - // of the Evaluator to detect the interrupt and avoid calling subsequent SkyFunctions. - return null; - } + // Keep the evaluator busy until the test's thread gets scheduled and can + // interrupt the Evaluator's thread. + env.getValue(GraphTester.toSkyKey("a" + valueIdCounter++)); - @Nullable - @Override - public String extractTag(SkyKey skyKey) { - return null; + // This method never throws InterruptedException, therefore it's the responsibility + // of the Evaluator to detect the interrupt and avoid calling subsequent + // SkyFunctions. + return null; + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }; } - }; - } - }); + }); } private void runPartialResultOnInterruption(boolean buildFastFirst) throws Exception { @@ -1263,7 +1262,8 @@ public class ParallelEvaluatorTest { new SkyFunction() { @Nullable @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { Map<SkyKey, ValueOrException<SomeErrorException>> values = env.getValuesOrThrow( ImmutableList.of(errorKey, otherKey), SomeErrorException.class); @@ -1488,7 +1488,7 @@ public class ParallelEvaluatorTest { class ParentFunction implements SkyFunction { @Override - public SkyValue compute(SkyKey skyKey, Environment env) { + public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException { SkyValue dep = env.getValue(SkyKey.create(childType, "billy the kid")); if (dep == null) { return null; @@ -1579,27 +1579,32 @@ public class ParallelEvaluatorTest { SkyKey topKey = GraphTester.toSkyKey("top"); final SkyKey parentKey = GraphTester.toSkyKey("parent"); tester.getOrCreate(parentKey).addDependency(errorKey).setComputedValue(CONCATENATE); - tester.getOrCreate(topKey).setBuilder(new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws GenericFunctionException { - try { - if (env.getValueOrThrow(parentKey, SomeErrorException.class) == null) { - return null; - } - } catch (SomeErrorException e) { - assertEquals(e.toString(), exception, e); - } - if (keepGoing) { - return topValue; - } else { - throw new GenericFunctionException(topException, Transience.PERSISTENT); - } - } - @Override - public String extractTag(SkyKey skyKey) { - throw new UnsupportedOperationException(); - } - }); + tester + .getOrCreate(topKey) + .setBuilder( + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws GenericFunctionException, InterruptedException { + try { + if (env.getValueOrThrow(parentKey, SomeErrorException.class) == null) { + return null; + } + } catch (SomeErrorException e) { + assertEquals(e.toString(), exception, e); + } + if (keepGoing) { + return topValue; + } else { + throw new GenericFunctionException(topException, Transience.PERSISTENT); + } + } + + @Override + public String extractTag(SkyKey skyKey) { + throw new UnsupportedOperationException(); + } + }); tester.getOrCreate(topKey).addErrorDependency(errorKey, new StringValue("recovered")) .setComputedValue(CONCATENATE); EvaluationResult<StringValue> result = eval(keepGoing, ImmutableList.of(topKey)); @@ -1742,29 +1747,33 @@ public class ParallelEvaluatorTest { tester.set(dep, new StringValue("child" + i)); } final SomeErrorException parentExn = new SomeErrorException("parent error"); - tester.getOrCreate(parentKey).setBuilder(new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - try { - SkyValue value = env.getValueOrThrow(errorDep, SomeErrorException.class); - if (value == null) { - return null; - } - } catch (SomeErrorException e) { - // Recover from the child error. - } - env.getValues(deps); - if (env.valuesMissing()) { - return null; - } - throw new GenericFunctionException(parentExn, Transience.PERSISTENT); - } + tester + .getOrCreate(parentKey) + .setBuilder( + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + try { + SkyValue value = env.getValueOrThrow(errorDep, SomeErrorException.class); + if (value == null) { + return null; + } + } catch (SomeErrorException e) { + // Recover from the child error. + } + env.getValues(deps); + if (env.valuesMissing()) { + return null; + } + throw new GenericFunctionException(parentExn, Transience.PERSISTENT); + } - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }); + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }); EvaluationResult<StringValue> evaluationResult = eval(keepGoing, ImmutableList.of(parentKey)); assertTrue(evaluationResult.hasError()); assertEquals(keepGoing ? parentExn : childExn, evaluationResult.getError().getException()); @@ -1891,46 +1900,55 @@ public class ParallelEvaluatorTest { // The parent should be built exactly twice: once during normal evaluation and once // during error bubbling. final AtomicInteger numParentInvocations = new AtomicInteger(0); - tester.getOrCreate(parentKey).setBuilder(new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - int invocations = numParentInvocations.incrementAndGet(); - if (handleChildError) { - try { - SkyValue value = env.getValueOrThrow(childKey, SomeErrorException.class); - // On the first invocation, either the child error should already be cached and not - // propagated, or it should be computed freshly and not propagated. On the second build - // (error bubbling), the child error should be propagated. - assertTrue("bogus non-null value " + value, value == null); - assertEquals("parent incorrectly re-computed during normal evaluation", 1, invocations); - assertFalse("child error not propagated during error bubbling", - env.inErrorBubblingForTesting()); - return value; - } catch (SomeErrorException e) { - assertTrue("child error propagated during normal evaluation", - env.inErrorBubblingForTesting()); - assertEquals(2, invocations); - return null; - } - } else { - if (invocations == 1) { - assertFalse("parent's first computation should be during normal evaluation", - env.inErrorBubblingForTesting()); - return env.getValue(childKey); - } else { - assertEquals(2, invocations); - assertTrue("parent incorrectly re-computed during normal evaluation", - env.inErrorBubblingForTesting()); - return env.getValue(childKey); - } - } - } + tester + .getOrCreate(parentKey) + .setBuilder( + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + int invocations = numParentInvocations.incrementAndGet(); + if (handleChildError) { + try { + SkyValue value = env.getValueOrThrow(childKey, SomeErrorException.class); + // On the first invocation, either the child error should already be cached and + // not propagated, or it should be computed freshly and not propagated. On the + // second build (error bubbling), the child error should be propagated. + assertTrue("bogus non-null value " + value, value == null); + assertEquals( + "parent incorrectly re-computed during normal evaluation", 1, invocations); + assertFalse( + "child error not propagated during error bubbling", + env.inErrorBubblingForTesting()); + return value; + } catch (SomeErrorException e) { + assertTrue( + "child error propagated during normal evaluation", + env.inErrorBubblingForTesting()); + assertEquals(2, invocations); + return null; + } + } else { + if (invocations == 1) { + assertFalse( + "parent's first computation should be during normal evaluation", + env.inErrorBubblingForTesting()); + return env.getValue(childKey); + } else { + assertEquals(2, invocations); + assertTrue( + "parent incorrectly re-computed during normal evaluation", + env.inErrorBubblingForTesting()); + return env.getValue(childKey); + } + } + } - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }); + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }); if (childErrorCached) { // Ensure that the child is already in the graph. evalValueInError(childKey); @@ -1979,19 +1997,23 @@ public class ParallelEvaluatorTest { final SkyKey otherKey = GraphTester.toSkyKey("otherKey"); final AtomicInteger numOtherParentInvocations = new AtomicInteger(0); final AtomicInteger numErrorParentInvocations = new AtomicInteger(0); - tester.getOrCreate(otherParentKey).setBuilder(new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - int invocations = numOtherParentInvocations.incrementAndGet(); - assertEquals("otherParentKey should not be restarted", 1, invocations); - return env.getValue(otherKey); - } + tester + .getOrCreate(otherParentKey) + .setBuilder( + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + int invocations = numOtherParentInvocations.incrementAndGet(); + assertEquals("otherParentKey should not be restarted", 1, invocations); + return env.getValue(otherKey); + } - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }); + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }); tester .getOrCreate(otherKey) .setBuilder( @@ -2026,33 +2048,38 @@ public class ParallelEvaluatorTest { return null; } }); - tester.getOrCreate(errorParentKey).setBuilder(new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - int invocations = numErrorParentInvocations.incrementAndGet(); - try { - SkyValue value = env.getValueOrThrow(errorKey, SomeErrorException.class); - assertTrue("bogus non-null value " + value, value == null); - if (invocations == 1) { - return null; - } else { - assertFalse(env.inErrorBubblingForTesting()); - fail("RACE CONDITION: errorParentKey was restarted!"); - return null; - } - } catch (SomeErrorException e) { - assertTrue("child error propagated during normal evaluation", - env.inErrorBubblingForTesting()); - assertEquals(2, invocations); - return null; - } - } + tester + .getOrCreate(errorParentKey) + .setBuilder( + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + int invocations = numErrorParentInvocations.incrementAndGet(); + try { + SkyValue value = env.getValueOrThrow(errorKey, SomeErrorException.class); + assertTrue("bogus non-null value " + value, value == null); + if (invocations == 1) { + return null; + } else { + assertFalse(env.inErrorBubblingForTesting()); + fail("RACE CONDITION: errorParentKey was restarted!"); + return null; + } + } catch (SomeErrorException e) { + assertTrue( + "child error propagated during normal evaluation", + env.inErrorBubblingForTesting()); + assertEquals(2, invocations); + return null; + } + } - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }); + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }); graph = new NotifyingHelper.NotifyingProcessableGraph( new InMemoryGraphImpl(), @@ -2143,41 +2170,49 @@ public class ParallelEvaluatorTest { final SkyKey parentKey = GraphTester.toSkyKey("parent"); final SkyKey childKey = GraphTester.toSkyKey("child"); final AtomicBoolean errorPropagated = new AtomicBoolean(false); - tester.getOrCreate(grandparentKey).setBuilder(new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - try { - return env.getValueOrThrow(parentKey, SomeErrorException.class); - } catch (SomeErrorException e) { - errorPropagated.set(true); - throw new GenericFunctionException(e, Transience.PERSISTENT); - } - } + tester + .getOrCreate(grandparentKey) + .setBuilder( + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + try { + return env.getValueOrThrow(parentKey, SomeErrorException.class); + } catch (SomeErrorException e) { + errorPropagated.set(true); + throw new GenericFunctionException(e, Transience.PERSISTENT); + } + } - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }); - tester.getOrCreate(parentKey).setBuilder(new SkyFunction() { - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - if (explicitlyPropagateError) { - try { - return env.getValueOrThrow(childKey, SomeErrorException.class); - } catch (SomeErrorException e) { - throw new GenericFunctionException(e, childKey); - } - } else { - return env.getValue(childKey); - } - } + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }); + tester + .getOrCreate(parentKey) + .setBuilder( + new SkyFunction() { + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws SkyFunctionException, InterruptedException { + if (explicitlyPropagateError) { + try { + return env.getValueOrThrow(childKey, SomeErrorException.class); + } catch (SomeErrorException e) { + throw new GenericFunctionException(e, childKey); + } + } else { + return env.getValue(childKey); + } + } - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }); + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }); tester.getOrCreate(childKey).setHasError(/*hasError=*/true); EvaluationResult<StringValue> result = eval(keepGoing, ImmutableList.of(grandparentKey)); assertTrue(result.hasError()); diff --git a/src/test/java/com/google/devtools/build/skyframe/WalkableGraphUtils.java b/src/test/java/com/google/devtools/build/skyframe/WalkableGraphUtils.java index 6c6f0c7b2a..7095371508 100644 --- a/src/test/java/com/google/devtools/build/skyframe/WalkableGraphUtils.java +++ b/src/test/java/com/google/devtools/build/skyframe/WalkableGraphUtils.java @@ -19,11 +19,13 @@ import com.google.common.collect.Iterables; /** Utility methods for querying (r)deps of nodes from {@link WalkableGraph}s more concisely. */ public class WalkableGraphUtils { - public static Iterable<SkyKey> getDirectDeps(WalkableGraph graph, SkyKey key) { + public static Iterable<SkyKey> getDirectDeps(WalkableGraph graph, SkyKey key) + throws InterruptedException { return Iterables.getOnlyElement(graph.getDirectDeps(ImmutableList.of(key)).values()); } - public static Iterable<SkyKey> getReverseDeps(WalkableGraph graph, SkyKey key) { + public static Iterable<SkyKey> getReverseDeps(WalkableGraph graph, SkyKey key) + throws InterruptedException { return Iterables.getOnlyElement(graph.getReverseDeps(ImmutableList.of(key)).values()); } } |