From 8d0403866770e5a7b46909c2fa2c6a0721778d00 Mon Sep 17 00:00:00 2001 From: "bazel.build machine account" <15028808+bazel-io@users.noreply.github.com> Date: Tue, 26 Sep 2023 14:06:56 -0400 Subject: [PATCH] [6.4.0] Show test labels in summaries in display form (#19625) When running external tests with Bzlmod, this results in apparent labels instead of canonical ones. Closes #19593. Commit https://github.com/bazelbuild/bazel/commit/4c154346c76250d2c0287e202ee0dc3eb07a28c7 PiperOrigin-RevId: 568312090 Change-Id: Ice4c48d9ae7e313b33ad41fe954ce57d5a1e2b12 Co-authored-by: Fabian Meumertzheim --- .../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 43f006d9196458..e4a4c0debc7cc9 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; @@ -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() @@ -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 = @@ -218,7 +235,8 @@ private static DetailedExitCode analyzeTestResults( AggregatingTestListener listener, OptionsParsingResult options, CommandEnvironment env, - AnsiTerminalPrinter printer) { + AnsiTerminalPrinter printer, + RepositoryMapping mainRepoMapping) { ImmutableSet 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); }