From 2c934f04c4aa381ffd59b9f1774e25ee6605441d Mon Sep 17 00:00:00 2001 From: Nelson Li Date: Wed, 2 Aug 2023 06:53:49 +0000 Subject: [PATCH] Print Passed and Failed methods in detailed test summary Previously, Bazel only printed Failed methods in the detailed test summary when using the `--test_summary=detailed` option. With this change, both Passed and Failed methods are now printed, providing a more comprehensive view of the test results. Fix #18685 --- .../runtime/TerminalTestResultNotifier.java | 10 ++-- .../build/lib/runtime/TestSummary.java | 8 ++- .../build/lib/runtime/TestSummaryPrinter.java | 52 +++++++++---------- .../build/lib/runtime/TestSummaryTest.java | 34 ++++++++++++ 4 files changed, 72 insertions(+), 32 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 3219af999d78cd..37f37dd0d41f54 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 @@ -148,7 +148,7 @@ private void printSummary( Set summaries, boolean showAllTests, boolean showNoStatusTests, - boolean printFailedTestCases) { + boolean printTestCases) { boolean withConfig = duplicateLabels(summaries); int numFailedToBuildReported = 0; for (TestSummary summary : summaries) { @@ -171,7 +171,7 @@ private void printSummary( printer, testLogPathFormatter, summaryOptions.verboseSummary, - printFailedTestCases, + printTestCases, withConfig); } } @@ -245,7 +245,7 @@ public void notify(Set summaries, int numberOfExecutedTargets) { summaries, /* showAllTests= */ true, /* showNoStatusTests= */ true, - /* printFailedTestCases= */ true); + /* printTestCases= */ true); break; case SHORT: @@ -253,7 +253,7 @@ public void notify(Set summaries, int numberOfExecutedTargets) { summaries, /* showAllTests= */ true, /* showNoStatusTests= */ false, - /* printFailedTestCases= */ false); + /* printTestCases= */ false); break; case TERSE: @@ -261,7 +261,7 @@ public void notify(Set summaries, int numberOfExecutedTargets) { summaries, /* showAllTests= */ false, /* showNoStatusTests= */ false, - /* printFailedTestCases= */ false); + /* printTestCases= */ false); break; case TESTCASE: diff --git a/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java b/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java index 7f4c4ef03190b4..95a142257d5fc8 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/TestSummary.java @@ -227,6 +227,12 @@ private int traverseTestCases(TestCase testCase) { return 1; } + public Builder addPassedTestCases(List testCases) { + checkMutation(testCases); + summary.passedTestCases.addAll(testCases); + return this; + } + @CanIgnoreReturnValue public Builder addFailedTestCases(List testCases, FailedTestCasesStatus status) { checkMutation(status); @@ -402,7 +408,7 @@ private void makeSummaryImmutable() { private boolean ranRemotely; private boolean wasUnreportedWrongSize; private List failedTestCases = new ArrayList<>(); - private List passedTestCases = new ArrayList<>(); + private final List passedTestCases = new ArrayList<>(); private List passedLogs = new ArrayList<>(); private List failedLogs = new ArrayList<>(); private List warnings = new ArrayList<>(); 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 d64111d703cc2c..48898dd33e496d 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 @@ -118,13 +118,13 @@ public static void print( AnsiTerminalPrinter terminalPrinter, TestLogPathFormatter testLogPathFormatter, boolean verboseSummary, - boolean printFailedTestCases) { + boolean printTestCases) { print( summary, terminalPrinter, testLogPathFormatter, verboseSummary, - printFailedTestCases, + printTestCases, false); } @@ -137,7 +137,7 @@ public static void print( AnsiTerminalPrinter terminalPrinter, TestLogPathFormatter testLogPathFormatter, boolean verboseSummary, - boolean printFailedTestCases, + boolean printTestCases, boolean withConfigurationName) { BlazeTestStatus status = summary.getStatus(); // Skip output for tests that failed to build. @@ -159,33 +159,34 @@ public static void print( + (verboseSummary ? getAttemptSummary(summary) + getTimeSummary(summary) : "") + "\n"); - for (TestCase testCase : summary.getPassedTestCases()) { - TestSummaryPrinter.printTestCase(terminalPrinter, testCase); - } + if (printTestCases) { + for (TestCase testCase : summary.getPassedTestCases()) { + TestSummaryPrinter.printTestCase(terminalPrinter, testCase); + } - if (printFailedTestCases && summary.getStatus() == BlazeTestStatus.FAILED) { - if (summary.getFailedTestCasesStatus() == FailedTestCasesStatus.NOT_AVAILABLE) { - terminalPrinter.print( - Mode.WARNING + " (individual test case information not available) " - + Mode.DEFAULT + "\n"); - } else { - for (TestCase testCase : summary.getFailedTestCases()) { - if (testCase.getStatus() != TestCase.Status.PASSED) { - TestSummaryPrinter.printTestCase(terminalPrinter, testCase); + if (summary.getStatus() == BlazeTestStatus.FAILED) { + if (summary.getFailedTestCasesStatus() == FailedTestCasesStatus.NOT_AVAILABLE) { + terminalPrinter.print( + Mode.WARNING + " (individual test case information not available) " + + Mode.DEFAULT + "\n"); + } else { + for (TestCase testCase : summary.getFailedTestCases()) { + if (testCase.getStatus() != TestCase.Status.PASSED) { + TestSummaryPrinter.printTestCase(terminalPrinter, testCase); + } } - } - if (summary.getFailedTestCasesStatus() != FailedTestCasesStatus.FULL) { - terminalPrinter.print( - Mode.WARNING - + " (some shards did not report details, list of failed test" - + " cases incomplete)\n" - + Mode.DEFAULT); + if (summary.getFailedTestCasesStatus() != FailedTestCasesStatus.FULL) { + terminalPrinter.print( + Mode.WARNING + + " (some shards did not report details, list of failed test" + + " cases incomplete)\n" + + Mode.DEFAULT); + } } } } - - if (!printFailedTestCases) { + else { for (String warning : summary.getWarnings()) { terminalPrinter.print(" " + AnsiTerminalPrinter.Mode.WARNING + "WARNING: " + AnsiTerminalPrinter.Mode.DEFAULT + warning + "\n"); @@ -211,8 +212,7 @@ public static void print( } /** - * Prints the result of an individual test case. It is assumed not to have - * passed, since passed test cases are not reported. + * Prints the result of an individual test case. */ static void printTestCase( AnsiTerminalPrinter terminalPrinter, TestCase testCase) { diff --git a/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java b/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java index 21cfea69fa9611..02586a8a5f0c2b 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/TestSummaryTest.java @@ -445,6 +445,31 @@ public void testTestCaseNamesShownWhenNeeded() throws Exception { verify(printerFailed).print(find("FAILED.*orange *\\(1\\.5")); } + @Test + public void testShowTestCaseNames() throws Exception { + TestCase detailPassed = + newDetail("strawberry", TestCase.Status.PASSED, 1000L); + TestCase detailFailed = + newDetail("orange", TestCase.Status.FAILED, 1500L); + + TestSummary summaryPassed = createPassedTestSummary( + BlazeTestStatus.PASSED, Arrays.asList(detailPassed)); + + TestSummary summaryFailed = createTestSummaryWithDetails( + BlazeTestStatus.FAILED, Arrays.asList(detailPassed, detailFailed)); + assertThat(summaryFailed.getStatus()).isEqualTo(BlazeTestStatus.FAILED); + + AnsiTerminalPrinter printerPassed = Mockito.mock(AnsiTerminalPrinter.class); + TestSummaryPrinter.print(summaryPassed, printerPassed, Path::getPathString, true, true); + verify(printerPassed).print(contains("//package:name")); + verify(printerPassed).print(find("PASSED.*strawberry *\\(1\\.0")); + + AnsiTerminalPrinter printerFailed = Mockito.mock(AnsiTerminalPrinter.class); + TestSummaryPrinter.print(summaryFailed, printerFailed, Path::getPathString, true, true); + verify(printerFailed).print(contains("//package:name")); + verify(printerFailed).print(find("FAILED.*orange *\\(1\\.5")); + } + @Test public void testTestCaseNamesOrdered() throws Exception { TestCase[] details = { @@ -642,6 +667,15 @@ private ConfiguredTarget stubTarget() throws Exception { return target(PATH, TARGET_NAME); } + private TestSummary createPassedTestSummary(BlazeTestStatus status, + List details) { + TestSummary summary = getTemplateBuilder() + .setStatus(status) + .addPassedTestCases(details) + .build(); + return summary; + } + private TestSummary createTestSummaryWithDetails(BlazeTestStatus status, List details) { TestSummary summary = getTemplateBuilder()