Skip to content

Commit

Permalink
Add tests that would have caught duplicate "FAILED" messages.
Browse files Browse the repository at this point in the history
This whole part of the code probably needs to be reworked, but putting tests in first.

PiperOrigin-RevId: 344276537
  • Loading branch information
janakdr authored and copybara-github committed Nov 25, 2020
1 parent b32349f commit 8eae6b3
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)));
Expand Down Expand Up @@ -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);
}
Expand Down
18 changes: 18 additions & 0 deletions src/test/shell/integration/ui_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

0 comments on commit 8eae6b3

Please sign in to comment.