Skip to content

Commit

Permalink
Print Passed and Failed methods in detailed test summary
Browse files Browse the repository at this point in the history
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 bazelbuild#18685
  • Loading branch information
Nelson Li committed Aug 2, 2023
1 parent 69f8772 commit 2c934f0
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ private void printSummary(
Set<TestSummary> summaries,
boolean showAllTests,
boolean showNoStatusTests,
boolean printFailedTestCases) {
boolean printTestCases) {
boolean withConfig = duplicateLabels(summaries);
int numFailedToBuildReported = 0;
for (TestSummary summary : summaries) {
Expand All @@ -171,7 +171,7 @@ private void printSummary(
printer,
testLogPathFormatter,
summaryOptions.verboseSummary,
printFailedTestCases,
printTestCases,
withConfig);
}
}
Expand Down Expand Up @@ -245,23 +245,23 @@ public void notify(Set<TestSummary> summaries, int numberOfExecutedTargets) {
summaries,
/* showAllTests= */ true,
/* showNoStatusTests= */ true,
/* printFailedTestCases= */ true);
/* printTestCases= */ true);
break;

case SHORT:
printSummary(
summaries,
/* showAllTests= */ true,
/* showNoStatusTests= */ false,
/* printFailedTestCases= */ false);
/* printTestCases= */ false);
break;

case TERSE:
printSummary(
summaries,
/* showAllTests= */ false,
/* showNoStatusTests= */ false,
/* printFailedTestCases= */ false);
/* printTestCases= */ false);
break;

case TESTCASE:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,12 @@ private int traverseTestCases(TestCase 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);
Expand Down Expand Up @@ -402,7 +408,7 @@ private void makeSummaryImmutable() {
private boolean ranRemotely;
private boolean wasUnreportedWrongSize;
private List<TestCase> failedTestCases = new ArrayList<>();
private List<TestCase> passedTestCases = 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<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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.
Expand All @@ -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");
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -642,6 +667,15 @@ private ConfiguredTarget stubTarget() throws Exception {
return target(PATH, TARGET_NAME);
}

private TestSummary createPassedTestSummary(BlazeTestStatus status,
List<TestCase> details) {
TestSummary summary = getTemplateBuilder()
.setStatus(status)
.addPassedTestCases(details)
.build();
return summary;
}

private TestSummary createTestSummaryWithDetails(BlazeTestStatus status,
List<TestCase> details) {
TestSummary summary = getTemplateBuilder()
Expand Down

0 comments on commit 2c934f0

Please sign in to comment.