diff options
author | Florian Weikert <fwe@google.com> | 2016-03-11 16:27:13 +0000 |
---|---|---|
committer | David Chen <dzc@google.com> | 2016-03-11 21:29:30 +0000 |
commit | 5aa538e4bc4c6ba8a30dc72afb1bc4d37e0c28f1 (patch) | |
tree | ffd7d6174482b4f17a73b75d5d7f8d3f50f4274e /src/main/java/com/google/devtools/build/lib | |
parent | 26eb57c208a1540635ca2089258c48af3998b635 (diff) |
Fixed a problem with wrong test statuses when using notest_keep_going (especially with sharding).
--
MOS_MIGRATED_REVID=116975152
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
3 files changed, 24 insertions, 17 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 7ae2da9f69..42bf33d22f 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 @@ -118,8 +118,8 @@ public final class BuildResult { /** * Whether some targets were skipped because of {@code setStopOnFirstFailure}. */ - public boolean skippedTargetsBecauseOfEarlierFailure() { - return stopOnFirstFailure && !getSuccess(); + public boolean getStopOnFirstFailure() { + return stopOnFirstFailure; } /** 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 424ce41b3a..94e0a7947c 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 @@ -59,7 +59,6 @@ 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. @@ -165,7 +164,7 @@ public class AggregatingTestListener { } finalSummary = analyzer - .markUnbuilt(summary, blazeHalted, skippedTestsBecauseOfEarlierFailure) + .markUnbuilt(summary, blazeHalted) .build(); // These are never going to run; removing them marks the target complete. @@ -192,8 +191,6 @@ public class AggregatingTestListener { BuildResult result = event.getResult(); if (result.wasCatastrophe()) { blazeHalted = true; - } else if (result.skippedTargetsBecauseOfEarlierFailure()) { - skippedTestsBecauseOfEarlierFailure = true; } buildComplete(result.getActualTargets(), result.getSuccessfulTargets()); } 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 a5b038a68f..8206527d4f 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 @@ -15,9 +15,11 @@ package com.google.devtools.build.lib.runtime; import com.google.common.collect.Sets; import com.google.common.eventbus.EventBus; +import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.exec.ExecutionOptions; @@ -48,6 +50,10 @@ public class TestResultAnalyzer { private final ExecutionOptions executionOptions; private final EventBus eventBus; + // Store information about potential failures in the presence of --nokeep_going or + // --notest_keep_going. + private boolean skipTargetsOnFailure; + /** * @param summaryOptions Parsed test summarization options. * @param executionOptions Parsed build/test execution options. @@ -61,6 +67,12 @@ public class TestResultAnalyzer { this.summaryOptions = summaryOptions; this.executionOptions = executionOptions; this.eventBus = eventBus; + eventBus.register(this); + } + + @Subscribe + public void doneBuild(BuildCompleteEvent event) { + skipTargetsOnFailure = event.getResult().getStopOnFirstFailure(); } /** @@ -272,24 +284,22 @@ public class TestResultAnalyzer { // tests with no status and post it here. TestSummary summary = summaryBuilder.peek(); BlazeTestStatus status = summary.getStatus(); - if (status != BlazeTestStatus.NO_STATUS) { + if (skipTargetsOnFailure) { + status = BlazeTestStatus.NO_STATUS; + } else if (status != BlazeTestStatus.NO_STATUS) { status = aggregateStatus(status, BlazeTestStatus.INCOMPLETE); } return summaryBuilder.setStatus(status); } - 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. + TestSummary.Builder markUnbuilt(TestSummary.Builder summary, boolean blazeHalted) { BlazeTestStatus runStatus = - blazeHalted - ? BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING : ( - executionOptions.testCheckUpToDate || stopOnFirstFailure - ? BlazeTestStatus.NO_STATUS : BlazeTestStatus.FAILED_TO_BUILD); + blazeHalted + ? BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING + : (executionOptions.testCheckUpToDate || skipTargetsOnFailure + ? BlazeTestStatus.NO_STATUS + : BlazeTestStatus.FAILED_TO_BUILD); return summary.setStatus(runStatus); } |