From e22d22a530fe1d8fe524a052f7b52b60c8b74a6e Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Fri, 22 Sep 2023 13:36:58 +0200
Subject: [PATCH] Show test labels in summaries in display form
When running external tests with Bzlmod, this results in apparent
labels instead of canonical ones.
---
.../runtime/TerminalTestResultNotifier.java | 9 +++--
.../build/lib/runtime/TestSummaryPrinter.java | 15 ++++++--
.../lib/runtime/commands/TestCommand.java | 35 ++++++++++++++-----
.../TerminalTestResultNotifierTest.java | 11 ++++--
4 files changed, 54 insertions(+), 16 deletions(-)
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
index 4445624970cf42..1a67c025b5badf 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifier.java
@@ -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;
@@ -108,6 +109,7 @@ 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
@@ -115,11 +117,13 @@ public static class TestSummaryOptions extends OptionsBase {
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;
}
/**
@@ -172,7 +176,8 @@ private void printSummary(
testLogPathFormatter,
summaryOptions.verboseSummary,
showAllTestCases,
- withConfig);
+ withConfig,
+ mainRepoMapping);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java b/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
index 8ee42143bd0ef1..5f3efdabdcf61c 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/TestSummaryPrinter.java
@@ -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;
@@ -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);
}
/**
@@ -133,7 +141,8 @@ 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)
@@ -141,7 +150,7 @@ public static void print(
return;
}
String message = getCacheMessage(summary) + statusString(summary);
- String targetName = summary.getLabel().toString();
+ String targetName = summary.getLabel().getDisplayForm(mainRepoMapping);
if (withConfigurationName) {
targetName += " (" + summary.getConfiguration().getMnemonic() + ")";
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
index cf772847db75fe..c4b50132c83809 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java
@@ -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;
@@ -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;
@@ -189,15 +192,29 @@ private BlazeCommandResult doTest(
return BlazeCommandResult.detailedExitCode(detailedExitCode);
}
+ 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());
+ }
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 =
@@ -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 =
@@ -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);
}
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java b/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java
index 9fa43192457cf4..659ab2eef1aa40 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/TerminalTestResultNotifierTest.java
@@ -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;
@@ -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);
}
@@ -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);
}