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
Closes #19099.
RELNOTES: The `--test_summary=detailed` option now also prints passed test cases.
PiperOrigin-RevId: 553737487
Change-Id: I332b70d916394de7caed7a07667b46087724a6c1
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 add22ee..4445624 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
@@ -142,13 +142,13 @@
* @param summaries summaries of tests {@link TestSummary}
* @param showAllTests if true, print information about each test regardless of its status
* @param showNoStatusTests if true, print information about not executed tests (no status tests)
- * @param printFailedTestCases if true, print details about which test cases in a test failed
+ * @param showAllTestCases if true, print all test cases status and detailed information
*/
private void printSummary(
Set<TestSummary> summaries,
boolean showAllTests,
boolean showNoStatusTests,
- boolean printFailedTestCases) {
+ boolean showAllTestCases) {
boolean withConfig = duplicateLabels(summaries);
int numFailedToBuildReported = 0;
for (TestSummary summary : summaries) {
@@ -171,7 +171,7 @@
printer,
testLogPathFormatter,
summaryOptions.verboseSummary,
- printFailedTestCases,
+ showAllTestCases,
withConfig);
}
}
@@ -243,9 +243,9 @@
case DETAILED:
printSummary(
summaries,
- /* showAllTests= */ false,
+ /* showAllTests= */ true,
/* showNoStatusTests= */ true,
- /* printFailedTestCases= */ true);
+ /* showAllTestCases= */ true);
break;
case SHORT:
@@ -253,7 +253,7 @@
summaries,
/* showAllTests= */ true,
/* showNoStatusTests= */ false,
- /* printFailedTestCases= */ false);
+ /* showAllTestCases= */ false);
break;
case TERSE:
@@ -261,7 +261,7 @@
summaries,
/* showAllTests= */ false,
/* showNoStatusTests= */ false,
- /* printFailedTestCases= */ false);
+ /* showAllTestCases= */ 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 2ddd38e..baf1968 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
@@ -41,6 +41,7 @@
import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
import com.google.devtools.build.lib.view.test.TestStatus.FailedTestCasesStatus;
import com.google.devtools.build.lib.view.test.TestStatus.TestCase;
+import com.google.devtools.build.lib.view.test.TestStatus.TestCase.Status;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.protobuf.util.Durations;
import com.google.protobuf.util.Timestamps;
@@ -218,9 +219,20 @@
if (testCase.getStatus() != TestCase.Status.PASSED) {
this.summary.failedTestCases.add(testCase);
}
+
+ if (testCase.getStatus() == Status.PASSED) {
+ this.summary.passedTestCases.add(testCase);
+ }
+
return 1;
}
+ public Builder addPassedTestCases(List<TestCase> testCases) {
+ checkMutation(testCases);
+ summary.passedTestCases.addAll(testCases);
+ return this;
+ }
+
@CanIgnoreReturnValue
public Builder addFailedTestCases(List<TestCase> testCases, FailedTestCasesStatus status) {
checkMutation(status);
@@ -396,6 +408,7 @@
private boolean ranRemotely;
private boolean wasUnreportedWrongSize;
private List<TestCase> failedTestCases = new ArrayList<>();
+ private final List<TestCase> passedTestCases = new ArrayList<>();
private List<Path> passedLogs = new ArrayList<>();
private List<Path> failedLogs = new ArrayList<>();
private List<String> warnings = new ArrayList<>();
@@ -507,6 +520,10 @@
return failedTestCases;
}
+ public List<TestCase> getPassedTestCases() {
+ return passedTestCases;
+ }
+
public List<Path> getCoverageFiles() {
return coverageFiles;
}
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 eb76c11..8ee4214 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
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus;
import com.google.devtools.build.lib.view.test.TestStatus.FailedTestCasesStatus;
import com.google.devtools.build.lib.view.test.TestStatus.TestCase;
+import com.google.devtools.build.lib.view.test.TestStatus.TestCase.Status;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
@@ -117,18 +118,13 @@
AnsiTerminalPrinter terminalPrinter,
TestLogPathFormatter testLogPathFormatter,
boolean verboseSummary,
- boolean printFailedTestCases) {
- print(
- summary,
- terminalPrinter,
- testLogPathFormatter,
- verboseSummary,
- printFailedTestCases,
- false);
+ boolean showAllTestCases) {
+ print(summary, terminalPrinter, testLogPathFormatter, verboseSummary, showAllTestCases, false);
}
/**
* Prints summary status for a single test.
+ *
* @param terminalPrinter The printer to print to
*/
public static void print(
@@ -136,7 +132,7 @@
AnsiTerminalPrinter terminalPrinter,
TestLogPathFormatter testLogPathFormatter,
boolean verboseSummary,
- boolean printFailedTestCases,
+ boolean showAllTestCases,
boolean withConfigurationName) {
BlazeTestStatus status = summary.getStatus();
// Skip output for tests that failed to build.
@@ -158,29 +154,35 @@
+ (verboseSummary ? getAttemptSummary(summary) + getTimeSummary(summary) : "")
+ "\n");
- 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 (showAllTestCases) {
+ for (TestCase testCase : summary.getPassedTestCases()) {
+ TestSummaryPrinter.printTestCase(terminalPrinter, testCase);
+ }
- if (summary.getFailedTestCasesStatus() != FailedTestCasesStatus.FULL) {
+ if (summary.getStatus() == BlazeTestStatus.FAILED) {
+ if (summary.getFailedTestCasesStatus() == FailedTestCasesStatus.NOT_AVAILABLE) {
terminalPrinter.print(
Mode.WARNING
- + " (some shards did not report details, list of failed test"
- + " cases incomplete)\n"
- + Mode.DEFAULT);
+ + " (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 (!printFailedTestCases) {
+ } else {
for (String warning : summary.getWarnings()) {
terminalPrinter.print(" " + AnsiTerminalPrinter.Mode.WARNING + "WARNING: "
+ AnsiTerminalPrinter.Mode.DEFAULT + warning + "\n");
@@ -205,12 +207,8 @@
}
}
- /**
- * Prints the result of an individual test case. It is assumed not to have
- * passed, since passed test cases are not reported.
- */
- static void printTestCase(
- AnsiTerminalPrinter terminalPrinter, TestCase testCase) {
+ /** Prints the result of an individual test case. */
+ static void printTestCase(AnsiTerminalPrinter terminalPrinter, TestCase testCase) {
String timeSummary;
if (testCase.hasRunDurationMillis()) {
timeSummary = " ("
@@ -220,16 +218,17 @@
timeSummary = "";
}
+ Mode mode = (testCase.getStatus() == Status.PASSED) ? Mode.INFO : Mode.ERROR;
terminalPrinter.print(
" "
- + Mode.ERROR
- + Strings.padEnd(testCase.getStatus().toString(), 8, ' ')
- + Mode.DEFAULT
- + testCase.getClassName()
- + "."
- + testCase.getName()
- + timeSummary
- + "\n");
+ + mode
+ + Strings.padEnd(testCase.getStatus().toString(), 8, ' ')
+ + Mode.DEFAULT
+ + testCase.getClassName()
+ + "."
+ + testCase.getName()
+ + timeSummary
+ + "\n");
}
/**
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 2bb0307..a82242a 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
@@ -446,6 +446,30 @@
}
@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 = {
newDetail("apple", TestCase.Status.FAILED, 1000L),
@@ -500,13 +524,13 @@
@Test
public void testCollectingFailedDetails() throws Exception {
- TestCase rootCase = TestCase.newBuilder()
- .setName("tests")
- .setRunDurationMillis(5000L)
- .addChild(newDetail("apple", TestCase.Status.FAILED, 1000L))
- .addChild(newDetail("banana", TestCase.Status.PASSED, 1000L))
- .addChild(newDetail("cherry", TestCase.Status.ERROR, 1000L))
- .build();
+ TestCase rootCase =
+ TestCase.newBuilder()
+ .setName("tests")
+ .setRunDurationMillis(5000L)
+ .addChild(newDetail("apple", TestCase.Status.FAILED, 1000L))
+ .addChild(newDetail("cherry", TestCase.Status.ERROR, 1000L))
+ .build();
TestSummary summary =
getTemplateBuilder().collectTestCases(rootCase).setStatus(BlazeTestStatus.FAILED).build();
@@ -519,6 +543,46 @@
}
@Test
+ public void testCollectingAllDetails() throws Exception {
+ TestCase rootCase =
+ TestCase.newBuilder()
+ .setName("tests")
+ .setRunDurationMillis(5000L)
+ .addChild(newDetail("apple", TestCase.Status.FAILED, 1000L))
+ .addChild(newDetail("banana", TestCase.Status.PASSED, 1000L))
+ .addChild(newDetail("cherry", TestCase.Status.ERROR, 1000L))
+ .build();
+
+ TestSummary summary =
+ getTemplateBuilder().collectTestCases(rootCase).setStatus(BlazeTestStatus.FAILED).build();
+
+ AnsiTerminalPrinter printer = Mockito.mock(AnsiTerminalPrinter.class);
+ TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
+ verify(printer).print(contains("//package:name"));
+ verify(printer).print(find("FAILED.*apple"));
+ verify(printer).print(find("PASSED.*banana"));
+ verify(printer).print(find("ERROR.*cherry"));
+ }
+
+ @Test
+ public void testCollectingPassedDetails() throws Exception {
+ TestCase rootCase =
+ TestCase.newBuilder()
+ .setName("tests")
+ .setRunDurationMillis(5000L)
+ .addChild(newDetail("apple", TestCase.Status.PASSED, 1000L))
+ .build();
+
+ TestSummary summary =
+ getTemplateBuilder().collectTestCases(rootCase).setStatus(BlazeTestStatus.PASSED).build();
+
+ AnsiTerminalPrinter printer = Mockito.mock(AnsiTerminalPrinter.class);
+ TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
+ verify(printer).print(contains("//package:name"));
+ verify(printer).print(find("PASSED.*apple"));
+ }
+
+ @Test
public void countTotalTestCases() throws Exception {
TestCase rootCase =
TestCase.newBuilder()
@@ -605,6 +669,10 @@
return target(PATH, TARGET_NAME);
}
+ private TestSummary createPassedTestSummary(BlazeTestStatus status, List<TestCase> details) {
+ return getTemplateBuilder().setStatus(status).addPassedTestCases(details).build();
+ }
+
private TestSummary createTestSummaryWithDetails(BlazeTestStatus status,
List<TestCase> details) {
TestSummary summary = getTemplateBuilder()
diff --git a/src/test/shell/bazel/bazel_test_test.sh b/src/test/shell/bazel/bazel_test_test.sh
index b96507d..f65533c 100755
--- a/src/test/shell/bazel/bazel_test_test.sh
+++ b/src/test/shell/bazel/bazel_test_test.sh
@@ -649,7 +649,7 @@
expect_log "name=\"dir/fail\""
}
-function test_detailed_test_summary() {
+function test_detailed_test_summary_for_failed_test() {
copy_examples
create_workspace_with_default_repos WORKSPACE
setup_javatest_support
@@ -662,6 +662,19 @@
expect_log 'FAILED.*com\.example\.myproject\.Fail\.testFail'
}
+function test_detailed_test_summary_for_passed_test() {
+ copy_examples
+ create_workspace_with_default_repos WORKSPACE
+ setup_javatest_support
+
+ local java_native_tests=//examples/java-native/src/test/java/com/example/myproject
+
+ bazel test --test_summary=detailed "${java_native_tests}:hello" >& $TEST_log \
+ || fail "expected success"
+ expect_log 'PASSED.*com\.example\.myproject\.TestHello\.testNoArgument'
+ expect_log 'PASSED.*com\.example\.myproject\.TestHello\.testWithArgument'
+}
+
# This test uses "--ignore_all_rc_files" since outside .bazelrc files can pollute
# this environment. Just "--bazelrc=/dev/null" is not sufficient to fix.
function test_flaky_test() {
