Don't report targets as broken when they were actually skipped because of no[test_]keep_going.

--
MOS_MIGRATED_REVID=115506435

5 files changed, 52 insertions, 10 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
index e57b6795df..7ae2da9f69 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
@@ -35,6 +35,7 @@ public final class BuildResult {
 
   private Throwable crash = null;
   private boolean catastrophe = false;
+  private boolean stopOnFirstFailure;
   private ExitCode exitCondition = ExitCode.BLAZE_INTERNAL_ERROR;
 
   private BuildConfigurationCollection configurations;
@@ -115,6 +116,21 @@ public final class BuildResult {
   }
 
   /**
+   * Whether some targets were skipped because of {@code setStopOnFirstFailure}.
+   */
+  public boolean skippedTargetsBecauseOfEarlierFailure() {
+    return stopOnFirstFailure && !getSuccess();
+  }
+
+  /**
+   * Indicates that remaining targets should be skipped once a target breaks/fails.
+   * This will be set when --nokeep_going or --notest_keep_going is set.
+   */
+  public void setStopOnFirstFailure(boolean stopOnFirstFailure) {
+    this.stopOnFirstFailure = stopOnFirstFailure;
+  }
+
+  /**
    * Gets the Blaze crash Throwable. Null if Blaze did not crash.
    */
   public Throwable getUnhandledThrowable() {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index 4113fb3d6a..2b9043cf6e 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -329,6 +329,7 @@ public final class BuildTool {
   public BuildResult processRequest(BuildRequest request, TargetValidator validator) {
     BuildResult result = new BuildResult(request.getStartTime());
     env.getEventBus().register(result);
+    maybeSetStopOnFirstFailure(request, result);
     Throwable catastrophe = null;
     ExitCode exitCode = ExitCode.BLAZE_INTERNAL_ERROR;
     try {
@@ -373,6 +374,16 @@ public final class BuildTool {
     return result;
   }
 
+  private void maybeSetStopOnFirstFailure(BuildRequest request, BuildResult result) {
+    if (shouldStopOnFailure(request)) {
+      result.setStopOnFirstFailure(true);
+    }
+  }
+
+  private boolean shouldStopOnFailure(BuildRequest request) {
+    return !(request.getViewOptions().keepGoing && request.getExecutionOptions().testKeepGoing);
+  }
+
   @VisibleForTesting
   protected final LoadingResult runLoadingPhase(final BuildRequest request,
                                                 final TargetValidator validator)
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
index 134d9fa9d1..3677546fec 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.analysis.AnalysisFailureEvent;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.LabelAndConfiguration;
 import com.google.devtools.build.lib.analysis.TargetCompleteEvent;
+import com.google.devtools.build.lib.buildtool.BuildResult;
 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.TestFilteringCompleteEvent;
@@ -58,7 +59,7 @@ public class AggregatingTestListener {
   private final EventBus eventBus;
   private final EventHandlerPreconditions preconditionHelper;
   private volatile boolean blazeHalted = false;
-
+  private volatile boolean skippedTestsBecauseOfEarlierFailure;
 
   // summaryLock guards concurrent access to these two collections, which should be kept
   // synchronized with each other.
@@ -162,7 +163,10 @@ public class AggregatingTestListener {
         // Not a test target; nothing to do.
         return;
       }
-      finalSummary = analyzer.markUnbuilt(summary, blazeHalted).build();
+      finalSummary =
+          analyzer
+              .markUnbuilt(summary, blazeHalted, skippedTestsBecauseOfEarlierFailure)
+              .build();
 
       // These are never going to run; removing them marks the target complete.
       remainingRuns.removeAll(label);
@@ -185,10 +189,13 @@ public class AggregatingTestListener {
 
   @Subscribe
   public void buildCompleteEvent(BuildCompleteEvent event) {
-    if (event.getResult().wasCatastrophe()) {
+    BuildResult result = event.getResult();
+    if (result.wasCatastrophe()) {
       blazeHalted = true;
+    } else if (result.skippedTargetsBecauseOfEarlierFailure()) {
+      skippedTestsBecauseOfEarlierFailure = true;
     }
-    buildComplete(event.getResult().getActualTargets(), event.getResult().getSuccessfulTargets());
+    buildComplete(result.getActualTargets(), result.getSuccessfulTargets());
   }
 
   @Subscribe
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildPhase.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildPhase.java
index 1d95985891..7dedc91d68 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildPhase.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildPhase.java
@@ -28,7 +28,9 @@ public enum BuildPhase {
   NOT_ANALYZED("not-analyzed", false),
   EXECUTION("build-failed", false),
   BLAZE_HALTED("blaze-halted", false),
-  COMPLETE("built", true);
+  COMPLETE("built", true),
+  // We skip a target when a previous target has failed to build with --nokeep_going.
+  BUILD_SKIPPED("build-skipped", false);
 
   private final String msg;
   private final boolean success;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java
index 6d94e021ba..a5b038a68f 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestResultAnalyzer.java
@@ -279,11 +279,17 @@ public class TestResultAnalyzer {
     return summaryBuilder.setStatus(status);
   }
 
-  TestSummary.Builder markUnbuilt(TestSummary.Builder summary, boolean blazeHalted) {
-    BlazeTestStatus runStatus = blazeHalted ? BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING
-        : (executionOptions.testCheckUpToDate
-            ? BlazeTestStatus.NO_STATUS
-            : BlazeTestStatus.FAILED_TO_BUILD);
+  TestSummary.Builder markUnbuilt(
+      TestSummary.Builder summary, boolean blazeHalted, boolean stopOnFirstFailure) {
+    // stopOnFirstFailure = true means that at least on of the options keep_going and
+    // test_keep_going is set to false.
+    // Consequently, we mark all unbuilt targets with NO_STATUS instead of FAILED_TO_BUILD in 
+    // order to indicate that Blaze has skipped these targets.
+    BlazeTestStatus runStatus =
+        blazeHalted 
+            ? BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING : (
+                executionOptions.testCheckUpToDate || stopOnFirstFailure
+                    ? BlazeTestStatus.NO_STATUS : BlazeTestStatus.FAILED_TO_BUILD);
 
     return summary.setStatus(runStatus);
   }