Skip to content

Commit

Permalink
[Skymeld] Fix an issue where non-test targets were reported as tests …
Browse files Browse the repository at this point in the history
…when there's no target.

Assume a situation where we have a build without any target.
In regular blaze, we expect the output to be "Found 0 targets", while in Skymeld it was "Found 0 test targets".

This happened because regular blaze checks for the nullness of `analysisResult.getTargetsToTest()` to confirm that we shouldn't print "Found 0 test targets". Skymeld, on the other hand, uses the BuildResultListener which initializes targetsToTest as an empty set instead of null. This resulted in it printing "Found 0 test targets".

This CL fixes that by detaching the logic of whether or not we should report the tests from both BuildResultListener and AnalysisResult, and instead getting this info from the BuildRequest.

PiperOrigin-RevId: 487810344
Change-Id: I59e900699ab226219488ca22ae93fedbc3763d21
  • Loading branch information
joeleba authored and copybara-github committed Nov 11, 2022
1 parent d3a92e8 commit 25ab148
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,14 @@ static AnalysisAndExecutionResult execute(
executionSetupCallback,
buildConfigurationCreatedCallback);
}

BuildResultListener buildResultListener = env.getBuildResultListener();
AnalysisPhaseRunner.reportTargets(
env, buildResultListener.getAnalyzedTargets(), buildResultListener.getAnalyzedTests());
if (request.shouldRunTests()) {
AnalysisPhaseRunner.reportTargetsWithTests(
env, buildResultListener.getAnalyzedTargets(), buildResultListener.getAnalyzedTests());
} else {
AnalysisPhaseRunner.reportTargets(env, buildResultListener.getAnalyzedTargets());
}

} else {
env.getReporter().handle(Event.progress("Loading complete."));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.buildtool;

import com.google.common.base.Preconditions;
import com.google.common.base.Stopwatch;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -143,7 +144,14 @@ public static AnalysisResult execute(
module.afterAnalysis(env, request, buildOptions, analysisResult);
}

reportTargets(env, analysisResult.getTargetsToBuild(), analysisResult.getTargetsToTest());
if (request.shouldRunTests()) {
reportTargetsWithTests(
env,
analysisResult.getTargetsToBuild(),
Preconditions.checkNotNull(analysisResult.getTargetsToTest()));
} else {
reportTargets(env, analysisResult.getTargetsToBuild());
}

for (ConfiguredTarget target : analysisResult.getTargetsToSkip()) {
BuildConfigurationValue config =
Expand Down Expand Up @@ -323,39 +331,38 @@ private static void postTopLevelStatusEvents(
}
}

static void reportTargets(
static void reportTargetsWithTests(
CommandEnvironment env,
Collection<ConfiguredTarget> targetsToBuild,
Collection<ConfiguredTarget> targetsToTest) {
if (targetsToTest != null) {
int testCount = targetsToTest.size();
int targetCount = targetsToBuild.size() - testCount;
if (targetCount == 0) {
env.getReporter()
.handle(
Event.info(
"Found "
+ testCount
+ (testCount == 1 ? " test target..." : " test targets...")));
} else {
env.getReporter()
.handle(
Event.info(
"Found "
+ targetCount
+ (targetCount == 1 ? " target and " : " targets and ")
+ testCount
+ (testCount == 1 ? " test target..." : " test targets...")));
}
int testCount = targetsToTest.size();
int targetCount = targetsToBuild.size() - testCount;
if (targetCount == 0) {
env.getReporter()
.handle(
Event.info(
"Found "
+ testCount
+ (testCount == 1 ? " test target..." : " test targets...")));
} else {
int targetCount = targetsToBuild.size();
env.getReporter()
.handle(
Event.info(
"Found " + targetCount + (targetCount == 1 ? " target..." : " targets...")));
"Found "
+ targetCount
+ (targetCount == 1 ? " target and " : " targets and ")
+ testCount
+ (testCount == 1 ? " test target..." : " test targets...")));
}
}

static void reportTargets(CommandEnvironment env, Collection<ConfiguredTarget> targetsToBuild) {
int targetCount = targetsToBuild.size();
env.getReporter()
.handle(
Event.info("Found " + targetCount + (targetCount == 1 ? " target..." : " targets...")));
}

/**
* Turns target patterns from the command line into parsed equivalents for single targets.
*
Expand Down

0 comments on commit 25ab148

Please sign in to comment.