From c9f9b0b84c4863b5b19220679e610efa17e972f0 Mon Sep 17 00:00:00 2001
From: Googler <twerth@google.com>
Date: Thu, 27 Oct 2022 00:36:04 -0700
Subject: [PATCH] Fix spitting out multiple "Build completed" messages.

Also make these a proper event, so that `--ui_event_filters` is applied
properly.

Fixes https://github.com/bazelbuild/bazel/issues/16085

RELNOTES[INC]:
This has the side effect of changing the message on unsuccessful builds from
```
FAILED: Build did NOT complete successfully (0 packages loaded)
```
to
```
ERROR: Build did NOT complete successfully
```
PiperOrigin-RevId: 484175656
Change-Id: I080ada6756734e5cdf236854101595949a57c083
---
 .../lib/runtime/SkymeldUiStateTracker.java    | 27 ++---------
 .../build/lib/runtime/UiEventHandler.java     | 12 +++--
 .../build/lib/runtime/UiStateTracker.java     | 23 +++++----
 .../runtime/SkymeldUiStateTrackerTest.java    |  2 +-
 .../UiEventHandlerStdOutAndStdErrTest.java    | 48 ++++++++++++++++---
 .../build/lib/runtime/UiStateTrackerTest.java | 20 ++------
 .../target_compatible_with_test.sh            | 30 ++++++------
 ...rget_compatible_with_test_external_repo.sh |  2 +-
 src/test/shell/integration/ui_test.sh         |  7 +--
 9 files changed, 89 insertions(+), 82 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTracker.java
index a61db61a2cb033..bc74b3f694bcab 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTracker.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTracker.java
@@ -18,6 +18,7 @@
 import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
 import com.google.devtools.build.lib.buildtool.buildevent.ExecutionProgressReceiverAvailableEvent;
 import com.google.devtools.build.lib.clock.Clock;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
 import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent;
 import com.google.devtools.build.lib.skyframe.LoadingPhaseStartedEvent;
@@ -26,7 +27,6 @@
 import com.google.devtools.build.lib.util.io.PositionAwareAnsiTerminalWriter;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.io.IOException;
-import java.time.Instant;
 
 /** Tracks the state of Skymeld builds and determines what to display at each state in the UI. */
 final class SkymeldUiStateTracker extends UiStateTracker {
@@ -219,30 +219,9 @@ synchronized void progressReceiverAvailable(ExecutionProgressReceiverAvailableEv
   }
 
   @Override
-  void buildComplete(BuildCompleteEvent event) {
+  Event buildComplete(BuildCompleteEvent event) {
     buildStatus = BuildStatus.BUILD_COMPLETED;
-    buildCompleteAt = Instant.ofEpochMilli(clock.currentTimeMillis());
-
-    if (event.getResult().getSuccess()) {
-      int actionsCompleted = this.actionsCompleted.get();
-      if (failedTests == 0) {
-        additionalMessage =
-            "Build completed successfully, "
-                + actionsCompleted
-                + pluralize(" total action", actionsCompleted);
-      } else {
-        additionalMessage =
-            "Build completed, "
-                + failedTests
-                + pluralize(" test", failedTests)
-                + " FAILED, "
-                + actionsCompleted
-                + pluralize(" total action", actionsCompleted);
-      }
-    } else {
-      ok = false;
-      additionalMessage = "Build did NOT complete successfully";
-    }
+    return super.buildComplete(event);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
index 21e625280a273f..379ae364969415 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/UiEventHandler.java
@@ -582,15 +582,17 @@ public void buildComplete(BuildCompleteEvent event) {
     // it as an event and add a timestamp, if events are supposed to have a timestamp.
     boolean done = false;
     synchronized (this) {
-      stateTracker.buildComplete(event);
+      handleInternal(stateTracker.buildComplete(event));
       ignoreRefreshLimitOnce();
-      refresh();
 
       // After a build has completed, only stop updating the UI if there is no more activities.
       if (!stateTracker.hasActivities()) {
         buildRunning = false;
         done = true;
       }
+
+      // Only refresh after we have determined whether we need to keep the progress bar up.
+      refresh();
     }
     if (done) {
       stopUpdateThread();
@@ -1001,8 +1003,10 @@ private synchronized void addProgressBar() throws IOException {
           TIMESTAMP_FORMAT.format(
               Instant.ofEpochMilli(clock.currentTimeMillis()).atZone(ZoneId.systemDefault()));
     }
-    stateTracker.writeProgressBar(terminalWriter, /*shortVersion=*/ !cursorControl, timestamp);
-    terminalWriter.newline();
+    if (stateTracker.hasActivities()) {
+      stateTracker.writeProgressBar(terminalWriter, /*shortVersion=*/ !cursorControl, timestamp);
+      terminalWriter.newline();
+    }
     numLinesProgressBar = countingTerminalWriter.getWrittenLines();
     if (progressInTermTitle) {
       LoggingTerminalWriter stringWriter = new LoggingTerminalWriter(true);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java
index 7cad0534a7c088..c38646b2085cd2 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java
@@ -42,6 +42,7 @@
 import com.google.devtools.build.lib.buildtool.buildevent.TestFilteringCompleteEvent;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseCompleteEvent;
 import com.google.devtools.build.lib.skyframe.ConfigurationPhaseStartedEvent;
@@ -87,7 +88,7 @@ class UiStateTracker {
 
   private int sampleSize = 3;
 
-  private String status;
+  protected String status;
   protected String additionalMessage;
 
   protected final Clock clock;
@@ -454,31 +455,31 @@ synchronized void progressReceiverAvailable(ExecutionProgressReceiverAvailableEv
     executionProgressReceiver = event.getExecutionProgressReceiver();
   }
 
-  void buildComplete(BuildCompleteEvent event) {
+  Event buildComplete(BuildCompleteEvent event) {
     buildComplete = true;
     buildCompleteAt = Instant.ofEpochMilli(clock.currentTimeMillis());
 
+    status = null;
+    additionalMessage = "";
     if (event.getResult().getSuccess()) {
-      status = "INFO";
       int actionsCompleted = this.actionsCompleted.get();
       if (failedTests == 0) {
-        additionalMessage =
+        return Event.info(
             "Build completed successfully, "
                 + actionsCompleted
-                + pluralize(" total action", actionsCompleted);
+                + pluralize(" total action", actionsCompleted));
       } else {
-        additionalMessage =
+        return Event.info(
             "Build completed, "
                 + failedTests
                 + pluralize(" test", failedTests)
                 + " FAILED, "
                 + actionsCompleted
-                + pluralize(" total action", actionsCompleted);
+                + pluralize(" total action", actionsCompleted));
       }
     } else {
       ok = false;
-      status = "FAILED";
-      additionalMessage = "Build did NOT complete successfully";
+      return Event.error("Build did NOT complete successfully");
     }
   }
 
@@ -1324,7 +1325,9 @@ synchronized void writeProgressBar(
       return;
     }
 
-    writeExecutionProgress(terminalWriter, shortVersion);
+    if (!buildComplete) {
+      writeExecutionProgress(terminalWriter, shortVersion);
+    }
 
     if (!shortVersion) {
       reportOnDownloads(terminalWriter);
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTrackerTest.java
index f91c05854143a3..727928919cbc84 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTrackerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/SkymeldUiStateTrackerTest.java
@@ -149,7 +149,7 @@ public void buildCompleted_stateChanges() {
     buildResult.setDetailedExitCode(DetailedExitCode.success());
     clock.advanceMillis(SECONDS.toMillis(1));
     buildResult.setStopTime(clock.currentTimeMillis());
-    uiStateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    var unused = uiStateTracker.buildComplete(new BuildCompleteEvent(buildResult));
 
     assertThat(uiStateTracker.buildStatus).isEqualTo(BuildStatus.BUILD_COMPLETED);
   }
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java b/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java
index da52662a897c69..ac4d7157e4fba3 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/UiEventHandlerStdOutAndStdErrTest.java
@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.testutil.ManualClock;
+import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.testing.junit.testparameterinjector.TestParameter;
 import com.google.testing.junit.testparameterinjector.TestParameterInjector;
@@ -45,6 +46,8 @@ public final class UiEventHandlerStdOutAndStdErrTest {
 
   private static final BuildCompleteEvent BUILD_COMPLETE_EVENT =
       new BuildCompleteEvent(new BuildResult(/*startTimeMillis=*/ 0));
+  private static final String BUILD_DID_NOT_COMPLETE_MESSAGE =
+      "\033[31m\033[1mERROR: \033[0mBuild did NOT complete successfully" + System.lineSeparator();
 
   @TestParameter private TestedOutput testedOutput;
 
@@ -91,9 +94,14 @@ private void createUiEventHandler(UiOptions uiOptions) {
   }
 
   @Test
-  public void buildComplete_outputsNothing() {
+  public void buildComplete_outputsBuildFailedOnStderr() {
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
-    output.assertFlushed();
+
+    if (testedOutput == TestedOutput.STDOUT) {
+      output.assertFlushed();
+    } else {
+      output.assertFlushed(BUILD_DID_NOT_COMPLETE_MESSAGE);
+    }
   }
 
   @Test
@@ -101,15 +109,39 @@ public void buildComplete_flushesBufferedMessage() {
     uiEventHandler.handle(output("hello"));
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
 
-    output.assertFlushed("hello");
+    if (testedOutput == TestedOutput.STDOUT) {
+      output.assertFlushed("hello");
+    } else {
+      output.assertFlushed("hello", System.lineSeparator() + BUILD_DID_NOT_COMPLETE_MESSAGE);
+    }
+  }
+
+  @Test
+  public void buildComplete_successfulBuild() {
+    uiEventHandler.handle(output(""));
+    var buildSuccessResult = new BuildResult(/*startTimeMillis=*/ 0);
+    buildSuccessResult.setDetailedExitCode(DetailedExitCode.success());
+    uiEventHandler.buildComplete(new BuildCompleteEvent(buildSuccessResult));
+
+    if (testedOutput == TestedOutput.STDOUT) {
+      output.assertFlushed();
+    } else {
+      output.assertFlushed(
+          "\033[32mINFO: \033[0mBuild completed successfully, 0 total actions"
+              + System.lineSeparator());
+    }
   }
 
   @Test
-  public void buildComplete_emptyBuffer_outputsNothing() {
+  public void buildComplete_emptyBuffer_outputsBuildFailedOnStderr() {
     uiEventHandler.handle(output(""));
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
 
-    output.assertFlushed();
+    if (testedOutput == TestedOutput.STDOUT) {
+      output.assertFlushed();
+    } else {
+      output.assertFlushed(BUILD_DID_NOT_COMPLETE_MESSAGE);
+    }
   }
 
   @Test
@@ -124,7 +156,11 @@ public void handleOutputEvent_concatenatesInBuffer() {
     uiEventHandler.handle(output("there"));
     uiEventHandler.buildComplete(BUILD_COMPLETE_EVENT);
 
-    output.assertFlushed("hello there");
+    if (testedOutput == TestedOutput.STDOUT) {
+      output.assertFlushed("hello there");
+    } else {
+      output.assertFlushed("hello there", System.lineSeparator() + BUILD_DID_NOT_COMPLETE_MESSAGE);
+    }
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java
index d2dbeb77b9e0cf..16237d01a28e26 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/UiStateTrackerTest.java
@@ -1360,7 +1360,7 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
         new AnnounceBuildEventTransportsEvent(ImmutableList.of(transport1, transport2)));
     stateTracker.buildEventTransportsAnnounced(
         new AnnounceBuildEventTransportsEvent(ImmutableList.of(transport3)));
-    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
 
     LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(true);
 
@@ -1371,8 +1371,6 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, containsString("BuildEventTransport1"));
     assertThat(output, containsString("BuildEventTransport2"));
     assertThat(output, containsString("BuildEventTransport3"));
-    assertThat(output, containsString("success"));
-    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport1));
@@ -1383,8 +1381,6 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, not(containsString("BuildEventTransport1")));
     assertThat(output, containsString("BuildEventTransport2"));
     assertThat(output, containsString("BuildEventTransport3"));
-    assertThat(output, containsString("success"));
-    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport3));
@@ -1395,8 +1391,6 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, not(containsString("BuildEventTransport1")));
     assertThat(output, containsString("BuildEventTransport2"));
     assertThat(output, not(containsString("BuildEventTransport3")));
-    assertThat(output, containsString("success"));
-    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport2));
@@ -1407,8 +1401,6 @@ public void testMultipleBuildEventProtocolTransports() throws Exception {
     assertThat(output, not(containsString("BuildEventTransport1")));
     assertThat(output, not(containsString("BuildEventTransport2")));
     assertThat(output, not(containsString("BuildEventTransport3")));
-    assertThat(output, containsString("success"));
-    assertThat(output, containsString("complete"));
     assertThat(output.split("\\n")).hasLength(1);
   }
 
@@ -1426,7 +1418,7 @@ public void testBuildEventTransportsOnNarrowTerminal() throws IOException {
     stateTracker.buildStarted();
     stateTracker.buildEventTransportsAnnounced(
         new AnnounceBuildEventTransportsEvent(ImmutableList.of(transport1, transport2)));
-    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.writeProgressBar(terminalWriter);
     String output = terminalWriter.getTranscript();
@@ -1434,8 +1426,6 @@ public void testBuildEventTransportsOnNarrowTerminal() throws IOException {
     assertThat(output, containsString("1s"));
     assertThat(output, containsString("A".repeat(30) + "..."));
     assertThat(output, containsString("BuildEventTransport"));
-    assertThat(output, containsString("success"));
-    assertThat(output, containsString("complete"));
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(1));
     stateTracker.buildEventTransportClosed(new BuildEventTransportClosedEvent(transport2));
@@ -1446,8 +1436,6 @@ public void testBuildEventTransportsOnNarrowTerminal() throws IOException {
     assertThat(output, containsString("2s"));
     assertThat(output, containsString("A".repeat(30) + "..."));
     assertThat(output, not(containsString("BuildEventTransport")));
-    assertThat(output, containsString("success"));
-    assertThat(output, containsString("complete"));
     assertThat(output.split("\\n")).hasLength(2);
   }
 
@@ -1526,7 +1514,7 @@ public void waitingRemoteCacheMessage_afterBuildComplete_visible() throws IOExce
     BuildResult buildResult = new BuildResult(clock.currentTimeMillis());
     buildResult.setDetailedExitCode(DetailedExitCode.success());
     buildResult.setStopTime(clock.currentTimeMillis());
-    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
     clock.advanceMillis(Duration.ofSeconds(2).toMillis());
     LoggingTerminalWriter terminalWriter = new LoggingTerminalWriter(/*discardHighlight=*/ true);
 
@@ -1545,7 +1533,7 @@ public void waitingRemoteCacheMessage_multipleUploadEvents_countCorrectly() thro
     BuildResult buildResult = new BuildResult(clock.currentTimeMillis());
     buildResult.setDetailedExitCode(DetailedExitCode.success());
     buildResult.setStopTime(clock.currentTimeMillis());
-    stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
+    var unused = stateTracker.buildComplete(new BuildCompleteEvent(buildResult));
     stateTracker.actionUploadStarted(ActionUploadStartedEvent.create(action, "b"));
     stateTracker.actionUploadStarted(ActionUploadStartedEvent.create(action, "c"));
     stateTracker.actionUploadFinished(ActionUploadFinishedEvent.create(action, "a"));
diff --git a/src/test/shell/integration/target_compatible_with_test.sh b/src/test/shell/integration/target_compatible_with_test.sh
index 5d37ccf56d80fb..93da0d05e75265 100755
--- a/src/test/shell/integration/target_compatible_with_test.sh
+++ b/src/test/shell/integration/target_compatible_with_test.sh
@@ -451,7 +451,7 @@ function test_failure_on_incompatible_top_level_target() {
       && fail "Bazel passed unexpectedly."
 
     expect_log 'ERROR: Target //target_skipping:pass_on_foo1_bar2 is incompatible and cannot be built'
-    expect_log '^FAILED: Build did NOT complete successfully'
+    expect_log '^ERROR: Build did NOT complete successfully'
 
     # Now look at the build event log.
     mv "${TEST_log}".build.json "${TEST_log}"
@@ -472,7 +472,7 @@ function test_failure_on_incompatible_top_level_target() {
 
   expect_log '^//target_skipping:pass_on_foo1  *  PASSED in'
   expect_log '^ERROR: command succeeded, but not all targets were analyzed'
-  expect_log '^FAILED: Build did NOT complete successfully'
+  expect_log '^ERROR: Build did NOT complete successfully'
 }
 
 # Crudely validates that the build event protocol contains useful information
@@ -531,7 +531,7 @@ EOF
     --platforms=@//target_skipping:foo2_bar1_platform \
     //target_skipping:sh_foo2 &> "${TEST_log}" && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target //target_skipping:sh_foo2 is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 
   bazel build \
     --show_result=10 \
@@ -539,7 +539,7 @@ EOF
     --platforms=@//target_skipping:foo2_bar1_platform \
     //target_skipping:foo_test &> "${TEST_log}" && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target //target_skipping:foo_test is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 }
 
 # Validate that targets are skipped when the implementation is in Starlark
@@ -592,7 +592,7 @@ EOF
     --platforms=@//target_skipping:foo3_platform \
     //target_skipping:hello_world_bin &> "${TEST_log}" && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target //target_skipping:hello_world_bin is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 }
 
 # Validates that rules with custom providers are skipped when incompatible.
@@ -726,7 +726,7 @@ EOF
   expect_log '^Dependency chain:$'
   expect_log '^    //target_skipping:generate_with_tool (.*)$'
   expect_log "^    //target_skipping:generator_tool (.*)   <-- target platform (//target_skipping:foo2_bar1_platform) didn't satisfy constraint //target_skipping:foo1"
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 
   # Validate the test.
   bazel test \
@@ -741,7 +741,7 @@ EOF
   expect_log '^    //target_skipping:generated_test (.*)$'
   expect_log '^    //target_skipping:generate_with_tool (.*)$'
   expect_log "^    //target_skipping:generator_tool (.*)   <-- target platform (//target_skipping:foo2_bar1_platform) didn't satisfy constraint //target_skipping:foo1"
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 }
 
 # Validates the same thing as test_cc_test, but with multiple violated
@@ -789,7 +789,7 @@ EOF
   expect_log '^    //target_skipping:generated_test (.*)$'
   expect_log '^    //target_skipping:generate_with_tool (.*)$'
   expect_log "^    //target_skipping:generator_tool (.*)   <-- target platform (//target_skipping:foo2_bar1_platform) didn't satisfy constraints \[//target_skipping:bar2, //target_skipping:foo1\]"
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 }
 
 # Validates that we can express targets being compatible with A _or_ B.
@@ -834,7 +834,7 @@ EOF
     && fail "Bazel passed unexpectedly."
 
   expect_log 'ERROR: Target //target_skipping:pass_on_foo1_or_foo2_but_not_on_foo3 is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 }
 
 # Validates that we can express targets being compatible with everything _but_
@@ -866,7 +866,7 @@ EOF
     //target_skipping:pass_on_everything_but_foo1_and_foo2  &> "${TEST_log}" \
     && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target //target_skipping:pass_on_everything_but_foo1_and_foo2 is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 
   # Try with :foo2. This should fail.
   bazel test \
@@ -876,7 +876,7 @@ EOF
     //target_skipping:pass_on_everything_but_foo1_and_foo2 &> "${TEST_log}" \
     && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target //target_skipping:pass_on_everything_but_foo1_and_foo2 is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 
   # Now with :foo3. This should pass.
   bazel test \
@@ -933,7 +933,7 @@ EOF
 
   expect_log 'ERROR: Target //target_skipping:pass_on_foo3_and_bar2 is incompatible and cannot be built, but was explicitly requested'
   expect_log "^    //target_skipping:pass_on_foo3_and_bar2 (.*)   <-- target platform (//target_skipping:foo1_bar1_platform) didn't satisfy constraint //target_skipping:not_compatible$"
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 }
 
 function test_incompatible_with_aliased_constraint() {
@@ -984,7 +984,7 @@ EOF
     //target_skipping:also_some_foo3_target  &> "${TEST_log}" \
     && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target //target_skipping:also_some_foo3_target is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 }
 
 # Validate that an incompatible target with a toolchain not available for the
@@ -1120,7 +1120,7 @@ EOF
     --platforms=@//target_skipping:foo2_bar1_platform \
     //target_skipping:host_tool &> "${TEST_log}" && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target //target_skipping:host_tool is incompatible and cannot be built, but was explicitly requested'
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
 
   # Run with :foo1 in the host platform, but with :foo2 in the target platform.
   # This should work fine because we're not asking for any constraints to be
@@ -1511,7 +1511,7 @@ EOF
   expect_log_once '^    //target_skipping:aliased_other_basic_target '
   expect_log_once '^    //target_skipping:other_basic_target '
   expect_log_once "    //target_skipping:basic_foo3_target .*  <-- target platform (//target_skipping:foo1_bar1_platform) didn't satisfy constraint //target_skipping:foo3$"
-  expect_log 'FAILED: Build did NOT complete successfully'
+  expect_log 'ERROR: Build did NOT complete successfully'
   expect_not_log "${debug_message1}"
   expect_not_log "${debug_message2}"
   expect_not_log "${debug_message3}"
diff --git a/src/test/shell/integration/target_compatible_with_test_external_repo.sh b/src/test/shell/integration/target_compatible_with_test_external_repo.sh
index 94464946518245..0f1238f1a5b50a 100755
--- a/src/test/shell/integration/target_compatible_with_test_external_repo.sh
+++ b/src/test/shell/integration/target_compatible_with_test_external_repo.sh
@@ -216,7 +216,7 @@ EOF
     --build_event_text_file="${TEST_log}".build.json \
     @test_repo//:bin &> "${TEST_log}" && fail "Bazel passed unexpectedly."
   expect_log 'ERROR: Target @test_repo//:bin is incompatible and cannot be built'
-  expect_log '^FAILED: Build did NOT complete successfully'
+  expect_log '^ERROR: Build did NOT complete successfully'
   # Now look at the build event log.
   mv "${TEST_log}".build.json "${TEST_log}"
   expect_log '^    name: "PARSING_FAILURE"$'
diff --git a/src/test/shell/integration/ui_test.sh b/src/test/shell/integration/ui_test.sh
index 62ead39e60772c..f22be83bfd644d 100755
--- a/src/test/shell/integration/ui_test.sh
+++ b/src/test/shell/integration/ui_test.sh
@@ -688,11 +688,8 @@ EOF
   # Build event file needed so UI considers build to continue after failure.
   ! bazel test --build_event_json_file=bep.json --curses=yes --color=yes \
       //foo:foo &> "$TEST_log" || fail "Expected failure"
-  # Expect to see a failure message with an "erase line" control code prepended.
-  expect_log $'\e'"\[K"$'\e'"\[31m"$'\e'"\[1mFAILED:"$'\e'"\[0m Build did NOT complete successfully"
-  # We should not see a build failure message without an "erase line" to start.
-  # TODO(janakr): Fix the excessive printing of this failure message.
-  expect_log_n "^"$'\e'"\[31m"$'\e'"\[1mFAILED:"$'\e'"\[0m Build did NOT complete successfully" 4
+  # Expect to see exactly one failure message.
+  expect_log_n '\[31m\[1mERROR: \[0mBuild did NOT complete successfully' 1
 }
 
 function test_bazel_run_error_visible() {