From 8eae6b34ec6e1e83010b17094d1225f045606eb2 Mon Sep 17 00:00:00 2001
From: janakr <janakr@google.com>
Date: Wed, 25 Nov 2020 10:26:31 -0800
Subject: [PATCH] Add tests that would have caught duplicate "FAILED" messages.

This whole part of the code probably needs to be reworked, but putting tests in first.

PiperOrigin-RevId: 344276537
---
 .../UiEventHandlerStdOutAndStdErrTest.java    | 26 +++++++++++++++++--
 src/test/shell/integration/ui_test.sh         | 18 +++++++++++++
 2 files changed, 42 insertions(+), 2 deletions(-)

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 6ac27f1861cc6b..0edcbdc7c77947 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
@@ -64,6 +64,12 @@ enum TestedOutput {
 
   @Before
   public void createUiEventHandler() {
+    UiOptions uiOptions = new UiOptions();
+    uiOptions.eventFilters = ImmutableList.of();
+    createUiEventHandler(uiOptions);
+  }
+
+  private void createUiEventHandler(UiOptions uiOptions) {
     output = new FlushCollectingOutputStream();
 
     OutErr outErr = null;
@@ -78,8 +84,6 @@ public void createUiEventHandler() {
         break;
     }
 
-    UiOptions uiOptions = new UiOptions();
-    uiOptions.eventFilters = ImmutableList.of();
     uiEventHandler =
         new UiEventHandler(outErr, uiOptions, new ManualClock(), /*workspacePathFragment=*/ null);
     uiEventHandler.buildStarted(new BuildStartingEvent(/*env=*/ null, mock(BuildRequest.class)));
@@ -154,6 +158,24 @@ public void handleOutputEvent_concatenatesBufferBeforeFlushingOnNewline() {
     output.assertFlushed("hello there!\n");
   }
 
+  // This test only exercises progress bar code when testing stderr output, since we don't make
+  // any assertions on stderr (where the progress bar is written) when testing stdout.
+  @Test
+  public void noChangeOnUnflushedWrite() {
+    UiOptions uiOptions = new UiOptions();
+    uiOptions.showProgress = true;
+    uiOptions.useCursesEnum = UiOptions.UseCurses.YES;
+    uiOptions.eventFilters = ImmutableList.of();
+    createUiEventHandler(uiOptions);
+    if (testedOutput == TestedOutput.STDERR) {
+      assertThat(output.flushed).hasSize(1);
+      output.flushed.clear();
+    }
+    // Unterminated strings are saved in memory and not pushed out at all.
+    assertThat(output.flushed).isEmpty();
+    assertThat(output.writtenSinceFlush).isEmpty();
+  }
+
   private Event output(String message) {
     return Event.of(eventKind, message);
   }
diff --git a/src/test/shell/integration/ui_test.sh b/src/test/shell/integration/ui_test.sh
index 55b5920aafbbd8..74afd1561ab7d0 100755
--- a/src/test/shell/integration/ui_test.sh
+++ b/src/test/shell/integration/ui_test.sh
@@ -632,4 +632,22 @@ EOF
   bazel info server_pid >& "$TEST_log" || fail "Couldn't use server"
 }
 
+function test_progress_bar_after_stderr() {
+  mkdir -p foo
+  cat > foo/BUILD <<'EOF'
+genrule(name = 'fail', outs = ['fail.out'], cmd = 'false')
+sh_test(name = 'foo', data = [':fail'], srcs = ['foo.sh'])
+EOF
+  touch foo/foo.sh
+  chmod +x foo/foo.sh
+  # 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
+}
+
 run_suite "Integration tests for ${PRODUCT_NAME}'s UI"