Skip to content

Commit

Permalink
Show test labels in summaries in display form
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.

PiperOrigin-RevId: 568312090
Change-Id: Ice4c48d9ae7e313b33ad41fe954ce57d5a1e2b12
  • Loading branch information
fmeum authored and copybara-github committed Sep 25, 2023
1 parent 3309ee0 commit 4c15434
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 4c15434

Please sign in to comment.