Skip to content

Commit

Permalink
[6.4.0] Show test labels in summaries in display form (#19625)
Browse files Browse the repository at this point in the history
When running external tests with Bzlmod, this results in apparent labels
instead of canonical ones.

Closes #19593.

Commit
4c15434

PiperOrigin-RevId: 568312090
Change-Id: Ice4c48d9ae7e313b33ad41fe954ce57d5a1e2b12

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
bazel-io and fmeum authored Sep 26, 2023
1 parent 61403d0 commit 8d04038
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.analysis.test.TestResult;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutionOptions.TestOutputFormat;
import com.google.devtools.build.lib.exec.ExecutionOptions.TestSummaryFormat;
Expand Down Expand Up @@ -108,18 +109,21 @@ public static class TestSummaryOptions extends OptionsBase {
private final TestLogPathFormatter testLogPathFormatter;
private final OptionsParsingResult options;
private final TestSummaryOptions summaryOptions;
private final RepositoryMapping mainRepoMapping;

/**
* @param printer The terminal to print to
*/
public TerminalTestResultNotifier(
AnsiTerminalPrinter printer,
TestLogPathFormatter testLogPathFormatter,
OptionsParsingResult options) {
OptionsParsingResult options,
RepositoryMapping mainRepoMapping) {
this.printer = printer;
this.testLogPathFormatter = testLogPathFormatter;
this.options = options;
this.summaryOptions = options.getOptions(TestSummaryOptions.class);
this.mainRepoMapping = mainRepoMapping;
}

/**
Expand Down Expand Up @@ -172,7 +176,8 @@ private void printSummary(
testLogPathFormatter,
summaryOptions.verboseSummary,
showAllTestCases,
withConfig);
withConfig,
mainRepoMapping);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.exec.ExecutionOptions.TestOutputFormat;
import com.google.devtools.build.lib.exec.TestLogHelper;
import com.google.devtools.build.lib.util.LoggingUtil;
Expand Down Expand Up @@ -119,7 +120,14 @@ public static void print(
TestLogPathFormatter testLogPathFormatter,
boolean verboseSummary,
boolean showAllTestCases) {
print(summary, terminalPrinter, testLogPathFormatter, verboseSummary, showAllTestCases, false);
print(
summary,
terminalPrinter,
testLogPathFormatter,
verboseSummary,
showAllTestCases,
false,
RepositoryMapping.ALWAYS_FALLBACK);
}

/**
Expand All @@ -133,15 +141,16 @@ public static void print(
TestLogPathFormatter testLogPathFormatter,
boolean verboseSummary,
boolean showAllTestCases,
boolean withConfigurationName) {
boolean withConfigurationName,
RepositoryMapping mainRepoMapping) {
BlazeTestStatus status = summary.getStatus();
// Skip output for tests that failed to build.
if ((!verboseSummary && status == BlazeTestStatus.FAILED_TO_BUILD)
|| status == BlazeTestStatus.BLAZE_HALTED_BEFORE_TESTING) {
return;
}
String message = getCacheMessage(summary) + statusString(summary);
String targetName = summary.getLabel().toString();
String targetName = summary.getLabel().getDisplayForm(mainRepoMapping);
if (withConfigurationName) {
targetName += " (" + summary.getConfiguration().getMnemonic() + ")";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.buildtool.OutputDirectoryLinksUtils;
import com.google.devtools.build.lib.buildtool.PathPrettyPrinter;
import com.google.devtools.build.lib.buildtool.buildevent.TestingCompleteEvent;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutionOptions.TestOutputFormat;
Expand All @@ -45,7 +46,9 @@
import com.google.devtools.build.lib.server.FailureDetails.TestCommand.Code;
import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.RepositoryMappingValue;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.InterruptedFailureDetails;
import com.google.devtools.build.lib.util.io.AnsiTerminalPrinter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.common.options.OptionPriority.PriorityCategory;
Expand Down Expand Up @@ -127,6 +130,18 @@ private BlazeCommandResult doTest(
env.getReporter().handle(Event.error(e.getMessage()));
return BlazeCommandResult.failureDetail(e.getFailureDetail());
}
RepositoryMapping mainRepoMapping;
try {
mainRepoMapping = env.getSkyframeExecutor().getMainRepoMapping(env.getReporter());
} catch (InterruptedException e) {
String message = "test command interrupted";
env.getReporter().handle(Event.error(message));
return BlazeCommandResult.detailedExitCode(
InterruptedFailureDetails.detailedExitCode(message));
} catch (RepositoryMappingValue.RepositoryMappingResolutionException e) {
env.getReporter().handle(Event.error(e.getMessage()));
return BlazeCommandResult.detailedExitCode(e.getDetailedExitCode());
}

BuildRequest.Builder builder =
BuildRequest.builder()
Expand Down Expand Up @@ -190,14 +205,16 @@ private BlazeCommandResult doTest(
}

DetailedExitCode testResults =
analyzeTestResults(request, buildResult, testListener, options, env, printer);
analyzeTestResults(
request, buildResult, testListener, options, env, printer, mainRepoMapping);

if (testResults.isSuccess() && !buildResult.getSuccess()) {
// If all tests run successfully, test summary should include warning if
// there were build errors not associated with the test targets.
printer.printLn(AnsiTerminalPrinter.Mode.ERROR
+ "All tests passed but there were other errors during the build.\n"
+ AnsiTerminalPrinter.Mode.DEFAULT);
printer.printLn(
AnsiTerminalPrinter.Mode.ERROR
+ "All tests passed but there were other errors during the build.\n"
+ AnsiTerminalPrinter.Mode.DEFAULT);
}

DetailedExitCode detailedExitCode =
Expand All @@ -218,7 +235,8 @@ private static DetailedExitCode analyzeTestResults(
AggregatingTestListener listener,
OptionsParsingResult options,
CommandEnvironment env,
AnsiTerminalPrinter printer) {
AnsiTerminalPrinter printer,
RepositoryMapping mainRepoMapping) {
ImmutableSet<ConfiguredTargetKey> validatedTargets;
if (buildRequest.useValidationAspect()) {
validatedTargets =
Expand All @@ -230,10 +248,9 @@ private static DetailedExitCode analyzeTestResults(
validatedTargets = null;
}

TestResultNotifier notifier = new TerminalTestResultNotifier(
printer,
makeTestLogPathFormatter(options, env),
options);
TestResultNotifier notifier =
new TerminalTestResultNotifier(
printer, makeTestLogPathFormatter(options, env), options, mainRepoMapping);
return listener.differentialAnalyzeAndReport(
buildResult.getTestTargets(), buildResult.getSkippedTargets(), validatedTargets, notifier);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.exec.ExecutionOptions.TestSummaryFormat;
import com.google.devtools.build.lib.runtime.TerminalTestResultNotifier.TestSummaryOptions;
Expand Down Expand Up @@ -293,7 +294,10 @@ private void printFailedToBuildSummaries() throws LabelSyntaxException {

TerminalTestResultNotifier terminalTestResultNotifier =
new TerminalTestResultNotifier(
ansiTerminalPrinter, Path::getPathString, optionsParsingResult);
ansiTerminalPrinter,
Path::getPathString,
optionsParsingResult,
RepositoryMapping.ALWAYS_FALLBACK);
terminalTestResultNotifier.notify(builder.build(), 0);
}

Expand All @@ -318,7 +322,10 @@ private void printTestCaseSummary() throws LabelSyntaxException {

TerminalTestResultNotifier terminalTestResultNotifier =
new TerminalTestResultNotifier(
ansiTerminalPrinter, Path::getPathString, optionsParsingResult);
ansiTerminalPrinter,
Path::getPathString,
optionsParsingResult,
RepositoryMapping.ALWAYS_FALLBACK);
terminalTestResultNotifier.notify(ImmutableSet.of(testSummary), 1);
}

Expand Down

0 comments on commit 8d04038

Please sign in to comment.