commit 9ad28cd6001feb5dd8afda5878ebacfe25efe254
author Rumou Duan <rduan@google.com> Wed Oct 19 19:28:06 2016 +0000
committer Philipp Wollermann <philwo@google.com> Wed Oct 19 19:44:52 2016 +0000
tree 7fd858ca933ef88504a5d4ded21bde7e655fa531
parent ca9a425a4825b5fd9892ec55051e7ffb2025bea2

Proper action output checks for TreeArtifacts. Instead of crashing Bazel, we now handle failed TreeArtifact output checks gracefully.

--
MOS_MIGRATED_REVID=136627086

src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java

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");
     }
   }