BEP: Correctly report test exit code.

When a test failure occurred the BEP would still report a build_finished event
with exit code SUCCESS, even though bazel's exit code was TESTS_FAILED. This is
because we would rely on the exit code reported by the BuildCompleteEvent. However,
this exit code contains only the build status without taking into account the test status.

In this change we introduce the TestingCompleteEvent that reports bazel's final
exit code in case of "bazel test".

RELNOTES: None.
PiperOrigin-RevId: 157445808

4 files changed, 102 insertions, 4 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestingCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestingCompleteEvent.java
new file mode 100644
index 0000000000..a150de5694
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestingCompleteEvent.java
@@ -0,0 +1,76 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.buildtool.buildevent;
+
+import static com.google.devtools.build.lib.util.Preconditions.checkNotNull;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.buildeventstream.BuildEvent;
+import com.google.devtools.build.lib.buildeventstream.BuildEventConverters;
+import com.google.devtools.build.lib.buildeventstream.BuildEventId;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
+import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent;
+import com.google.devtools.build.lib.util.ExitCode;
+import java.util.Collection;
+
+/**
+ * Event triggered after testing has completed.
+ *
+ * <p>This event is used by the BEP to construct the {@link BuildEventStreamProtos.BuildFinished}
+ * event when the test command is used.
+ */
+public class TestingCompleteEvent implements BuildEvent {
+
+  private final ExitCode exitCode;
+  private final long finishTimeMillis;
+
+  /**
+   * Creates a new {@link TestingCompleteEvent}.
+   *
+   * @param exitCode  the overall exit code of "bazel test".
+   * @param finishTimeMillis the finish time in milliseconds since the epoch.
+   */
+  public TestingCompleteEvent(ExitCode exitCode, long finishTimeMillis) {
+    this.exitCode = checkNotNull(exitCode);
+    this.finishTimeMillis = finishTimeMillis;
+  }
+
+  @Override
+  public BuildEventId getEventId() {
+    return BuildEventId.buildFinished();
+  }
+
+  @Override
+  public Collection<BuildEventId> getChildrenEvents() {
+    return ImmutableList.of();
+  }
+
+  @Override
+  public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventConverters converters) {
+    BuildEventStreamProtos.BuildFinished.ExitCode protoExitCode =
+        BuildEventStreamProtos.BuildFinished.ExitCode.newBuilder()
+            .setName(exitCode.name())
+            .setCode(exitCode.getNumericExitCode())
+            .build();
+
+    BuildEventStreamProtos.BuildFinished finished =
+        BuildEventStreamProtos.BuildFinished.newBuilder()
+            .setOverallSuccess(ExitCode.SUCCESS.equals(exitCode))
+            .setExitCode(protoExitCode)
+            .setFinishTimeMillis(finishTimeMillis)
+            .build();
+    return GenericBuildEvent.protoChaining(this).setFinished(finished).build();
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
index 1fb35f770f..9c2047b6bf 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
@@ -44,8 +44,11 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
 import com.google.devtools.build.lib.buildeventstream.BuildEventTransportClosedEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint;
 import com.google.devtools.build.lib.buildeventstream.ProgressEvent;
+import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent;
+import com.google.devtools.build.lib.buildtool.buildevent.BuildStartingEvent;
+import com.google.devtools.build.lib.buildtool.buildevent.TestingCompleteEvent;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetView;
 import com.google.devtools.build.lib.events.Event;
@@ -87,6 +90,9 @@ public class BuildEventStreamer implements EventHandler {
   private final CountingArtifactGroupNamer artifactGroupNamer = new CountingArtifactGroupNamer();
   private OutErrProvider outErrProvider;
   private AbortReason abortReason = AbortReason.UNKNOWN;
+  // Will be set to true if the build was invoked through "bazel test".
+  private boolean isTestCommand;
+
   private static final Logger log = Logger.getLogger(BuildEventStreamer.class.getName());
 
   /**
@@ -335,6 +341,17 @@ public class BuildEventStreamer implements EventHandler {
       return;
     }
 
+    if (isTestCommand && event instanceof BuildCompleteEvent) {
+      // In case of "bazel test" ignore the BuildCompleteEvent, as it will be followed by a
+      // TestingCompleteEvent that contains the correct exit code.
+      return;
+    }
+
+    if (event instanceof BuildStartingEvent) {
+      BuildRequest buildRequest = ((BuildStartingEvent) event).getRequest();
+      isTestCommand = "test".equals(buildRequest.getCommandName());
+    }
+
     if (event instanceof BuildEventWithConfiguration) {
       for (BuildConfiguration configuration :
           ((BuildEventWithConfiguration) event).getConfigurations()) {
@@ -357,7 +374,7 @@ public class BuildEventStreamer implements EventHandler {
       buildEvent(freedEvent);
     }
 
-    if (event instanceof BuildCompleteEvent) {
+    if (event instanceof BuildCompleteEvent || event instanceof TestingCompleteEvent) {
       buildComplete();
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
index 94b71d8df3..c1403703d4 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.buildtool.BuildResult;
 import com.google.devtools.build.lib.buildtool.BuildTool;
+import com.google.devtools.build.lib.buildtool.buildevent.TestingCompleteEvent;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.exec.ExecutionOptions;
 import com.google.devtools.build.lib.exec.TestStrategy;
@@ -142,9 +143,11 @@ public class TestCommand implements BlazeCommand {
           + AnsiTerminalPrinter.Mode.DEFAULT);
     }
 
-    return buildSuccess ?
-           (testSuccess ? ExitCode.SUCCESS : ExitCode.TESTS_FAILED)
-           : buildResult.getExitCondition();
+    ExitCode exitCode = buildSuccess
+        ? (testSuccess ? ExitCode.SUCCESS : ExitCode.TESTS_FAILED)
+        : buildResult.getExitCondition();
+    env.getEventBus().post(new TestingCompleteEvent(exitCode, buildResult.getStopTime()));
+    return exitCode;
   }
 
   /**
diff --git a/src/test/shell/integration/build_event_stream_test.sh b/src/test/shell/integration/build_event_stream_test.sh
index a4bbffc526..67d85019a5 100755
--- a/src/test/shell/integration/build_event_stream_test.sh
+++ b/src/test/shell/integration/build_event_stream_test.sh
@@ -210,6 +210,8 @@ function test_test_attempts() {
   expect_log 'flaky/.*test.xml'
   expect_log 'name:.*test.log'
   expect_log 'name:.*test.xml'
+  expect_log 'name:.*TESTS_FAILED'
+  expect_not_log 'name:.*SUCCESS'
 }
 
 function test_test_runtime() {