Proper action output checks for TreeArtifacts. Instead of crashing Bazel, we now handle failed TreeArtifact output checks gracefully.
--
MOS_MIGRATED_REVID=136627086
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java
index 87d26bb..a221496 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactActionTest.java
@@ -98,11 +98,6 @@
}
@Override
- public boolean artifactExists(Artifact artifact) {
- throw new UnsupportedOperationException(artifact.prettyPrint());
- }
-
- @Override
public boolean isRegularFile(Artifact artifact) {
throw new UnsupportedOperationException(artifact.prettyPrint());
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java
index d06a544..159c3c1 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ParallelBuilderTest.java
@@ -51,12 +51,6 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
@@ -70,6 +64,10 @@
import java.util.concurrent.Semaphore;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Logger;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
/**
* Test suite for ParallelBuilder.
@@ -311,7 +309,7 @@
buildArtifacts(foo);
fail("Expected to fail");
} catch (BuildFailedException e) {
- assertContainsEvent("not all outputs were created");
+ assertContainsEvent("not all outputs were created or valid");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
index b2359d9..d21c908 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import static com.google.common.base.Throwables.getRootCause;
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.actions.ActionInputHelper.treeFileArtifact;
import static org.junit.Assert.assertFalse;
@@ -22,9 +21,11 @@
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
import com.google.common.collect.Collections2;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.common.hash.Hashing;
@@ -47,6 +48,9 @@
import com.google.devtools.build.lib.actions.util.TestAction;
import com.google.devtools.build.lib.actions.util.TestAction.DummyAction;
import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventKind;
+import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.vfs.FileStatus;
@@ -88,7 +92,6 @@
TreeFileArtifact outTwoFileOne;
TreeFileArtifact outTwoFileTwo;
Button buttonTwo = new Button();
-
@Before
public void setUp() throws Exception {
in = createSourceArtifact("input");
@@ -380,6 +383,18 @@
@Test
public void testInvalidOutputRegistrations() throws Exception {
+ // Failure expected
+ StoredEventHandler storingEventHandler = new StoredEventHandler();
+ reporter.removeHandler(failFastHandler);
+ reporter.addHandler(storingEventHandler);
+
+ Predicate<Event> isErrorEvent = new Predicate<Event>() {
+ @Override
+ public boolean apply(Event event) {
+ return event.getKind().equals(EventKind.ERROR);
+ }
+ };
+
TreeArtifactTestAction failureOne = new TreeArtifactTestAction(
Runnables.doNothing(), outOneFileOne, outOneFileTwo) {
@Override
@@ -400,8 +415,13 @@
try {
buildArtifact(outOne);
fail(); // Should have thrown
- } catch (Exception e) {
- assertThat(getRootCause(e).getMessage()).contains("not present on disk");
+ } catch (BuildFailedException e) {
+ //not all outputs were created
+ List<Event> errors = ImmutableList.copyOf(
+ Iterables.filter(storingEventHandler.getEvents(), isErrorEvent));
+ assertThat(errors).hasSize(2);
+ assertThat(errors.get(0).getMessage()).contains("not present on disk");
+ assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
}
TreeArtifactTestAction failureTwo = new TreeArtifactTestAction(
@@ -423,11 +443,16 @@
};
registerAction(failureTwo);
+ storingEventHandler.clear();
try {
buildArtifact(outTwo);
fail(); // Should have thrown
- } catch (Exception e) {
- assertThat(getRootCause(e).getMessage()).contains("not present on disk");
+ } catch (BuildFailedException e) {
+ List<Event> errors = ImmutableList.copyOf(
+ Iterables.filter(storingEventHandler.getEvents(), isErrorEvent));
+ assertThat(errors).hasSize(2);
+ assertThat(errors.get(0).getMessage()).contains("not present on disk");
+ assertThat(errors.get(1).getMessage()).contains("not all outputs were created or valid");
}
}
@@ -590,7 +615,7 @@
buildArtifact(artifact2);
fail("Expected BuildFailedException");
} catch (BuildFailedException e) {
- assertThat(e.getMessage()).contains("not all outputs were created");
+ assertThat(e.getMessage()).contains("not all outputs were created or valid");
}
}
