In terse test_summary, skip no-status tests

If there is a build failure, don't clobber the terse test summary
by naming all the (usually many) tests that were skipped due to
this failure.

Change-Id: I6daae3efb1594c2b1018f87a50cf63949a34535b
PiperOrigin-RevId: 166983264

4 files changed, 39 insertions, 4 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
index ecea21a1da..3be1d4eac6 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java
@@ -185,8 +185,8 @@ public class ExecutionOptions extends OptionsBase {
     help =
         "Specifies the desired format ot the test summary. Valid values are 'short' to print "
             + "information only about tests executed, 'terse', to print information only about "
-            + "unsuccessful tests, 'detailed' to print detailed information about failed "
-            + "test cases, and 'none' to omit the summary."
+            + "unsuccessful tests that were run, 'detailed' to print detailed information about "
+            + "failed test cases, and 'none' to omit the summary."
   )
   public TestSummaryFormat testSummary;
 
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
index 78a878657f..445326abb6 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java
@@ -118,7 +118,7 @@ public abstract class TestStrategy implements TestActionContext {
 
   public enum TestSummaryFormat {
     SHORT, // Print information only about tests.
-    TERSE, // Like "SHORT", but even shorter: Do not print PASSED tests.
+    TERSE, // Like "SHORT", but even shorter: Do not print PASSED and NO STATUS tests.
     DETAILED, // Print information only about failed test cases.
     NONE; // Do not print summary.
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
index ced2070938..fcb2010b28 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
@@ -122,7 +122,9 @@ public class TerminalTestResultNotifier implements TestResultNotifier {
   private void printShortSummary(Set<TestSummary> summaries, boolean showPassingTests) {
     boolean withConfig = duplicateLabels(summaries);
     for (TestSummary summary : summaries) {
-      if (summary.getStatus() != BlazeTestStatus.PASSED || showPassingTests) {
+      if ((summary.getStatus() != BlazeTestStatus.PASSED
+              && summary.getStatus() != BlazeTestStatus.NO_STATUS)
+          || showPassingTests) {
         TestSummaryPrinter.print(summary, printer, summaryOptions.verboseSummary, false,
             withConfig);
       }
diff --git a/src/test/shell/integration/test_test.sh b/src/test/shell/integration/test_test.sh
index 7483ca0081..5a35ababa7 100755
--- a/src/test/shell/integration/test_test.sh
+++ b/src/test/shell/integration/test_test.sh
@@ -68,4 +68,37 @@ EOF
   expect_log "^Executed 1 out of 1 test: 1 fails"
 }
 
+function test_build_fail_terse_summary() {
+    mkdir -p tests
+    cat > tests/BUILD <<'EOF'
+genrule(
+  name = "testsrc",
+  outs = ["test.sh"],
+  cmd = "false",
+)
+sh_test(
+  name = "failstobuild1",
+  srcs = ["test.sh"],
+)
+sh_test(
+  name = "failstobuild2",
+  srcs = ["test.sh"],
+)
+genrule(
+  name = "slowtestsrc",
+  outs = ["slowtest.sh"],
+  cmd = "sleep 20 && echo '#!/bin/sh' > $@ && echo 'true' >> $@ && chmod 755 $@",
+)
+sh_test(
+  name = "willbeskipped",
+  srcs = ["slowtest.sh"],
+)
+EOF
+    bazel test --test_summary=terse //tests/... &>$TEST_log \
+      && fail "expected failure" || :
+    expect_not_log 'NO STATUS'
+    expect_log 'testsrc'
+    expect_log 'were skipped'
+}
+
 run_suite "test tests"