Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.4.0] Show test labels in summaries in display form #19625

Merged
merged 2 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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