From 63da2e7998720cf20d35a961e316c786f3f4ec87 Mon Sep 17 00:00:00 2001 From: Florian Weikert Date: Thu, 25 Feb 2016 00:22:05 +0000 Subject: Don't report targets as broken when they were actually skipped because of no[test_]keep_going. -- MOS_MIGRATED_REVID=115506435 --- .../google/devtools/build/lib/buildtool/BuildResult.java | 16 ++++++++++++++++ .../google/devtools/build/lib/buildtool/BuildTool.java | 11 +++++++++++ .../build/lib/runtime/AggregatingTestListener.java | 15 +++++++++++---- .../google/devtools/build/lib/runtime/BuildPhase.java | 4 +++- .../devtools/build/lib/runtime/TestResultAnalyzer.java | 16 +++++++++++----- 5 files changed, 52 insertions(+), 10 deletions(-) (limited to 'src/main/java/com/google/devtools') 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; @@ -114,6 +115,21 @@ public final class BuildResult { return catastrophe; } + /** + * 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. */ 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); } -- cgit v1.2.3