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 Jul 28, 2023
1 parent 7a262a6 commit 00f0abc
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public void notify(Set<TestSummary> summaries, int numberOfExecutedTargets) {
case DETAILED:
printSummary(
summaries,
/* showAllTests= */ false,
/* showAllTests= */ true,
/* showNoStatusTests= */ true,
/* printFailedTestCases= */ true);
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -218,6 +219,11 @@ private int traverseTestCases(TestCase testCase) {
if (testCase.getStatus() != TestCase.Status.PASSED) {
this.summary.failedTestCases.add(testCase);
}

if (testCase.getStatus() == Status.PASSED) {
this.summary.passedTestCases.add(testCase);
}

return 1;
}

Expand Down Expand Up @@ -396,6 +402,7 @@ private void makeSummaryImmutable() {
private boolean ranRemotely;
private boolean wasUnreportedWrongSize;
private List<TestCase> failedTestCases = new ArrayList<>();
private 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 Expand Up @@ -507,6 +514,8 @@ public List<TestCase> getFailedTestCases() {
return failedTestCases;
}

public List<TestCase> getPassedTestCases() { return passedTestCases;}

public List<Path> getCoverageFiles() {
return coverageFiles;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -158,6 +159,10 @@ public static void print(
+ (verboseSummary ? getAttemptSummary(summary) + getTimeSummary(summary) : "")
+ "\n");

for (TestCase testCase : summary.getPassedTestCases()) {
TestSummaryPrinter.printTestCase(terminalPrinter, testCase);
}

if (printFailedTestCases && summary.getStatus() == BlazeTestStatus.FAILED) {
if (summary.getFailedTestCasesStatus() == FailedTestCasesStatus.NOT_AVAILABLE) {
terminalPrinter.print(
Expand Down Expand Up @@ -220,9 +225,10 @@ static void printTestCase(
timeSummary = "";
}

Mode mode = (testCase.getStatus() == Status.PASSED) ? Mode.INFO : Mode.ERROR;
terminalPrinter.print(
" "
+ Mode.ERROR
+ mode
+ Strings.padEnd(testCase.getStatus().toString(), 8, ' ')
+ Mode.DEFAULT
+ testCase.getClassName()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,25 @@ public void testCachedResultsFirstInSort() throws Exception {

@Test
public void testCollectingFailedDetails() throws Exception {
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();

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("ERROR.*cherry"));
}

@Test
public void testCollectingAllDetails() throws Exception {
TestCase rootCase = TestCase.newBuilder()
.setName("tests")
.setRunDurationMillis(5000L)
Expand All @@ -515,6 +534,7 @@ public void testCollectingFailedDetails() throws Exception {
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"));
}

Expand Down
15 changes: 14 additions & 1 deletion src/test/shell/bazel/bazel_test_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ EOF
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
Expand All @@ -662,6 +662,19 @@ function test_detailed_test_summary() {
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() {
Expand Down

0 comments on commit 00f0abc

Please sign in to comment.