Reporting skipped test cases
`<skipped>` is a valid and common test case status supported by JUnit XML. Reporting skipped test cases as "passed" hides potential broken test cases. This PR reports them when `--test_summary=detailed`.
Before the fix:
```
INFO: Found 1 target and 1 test target...
INFO: Elapsed time: 103.095s, Critical Path: 43.23s
INFO: 476 processes: 36 internal, 440 darwin-sandbox.
INFO: Build completed successfully, 476 total actions
//foo/collect:go_default_test PASSED in 0.6s
PASSED collect.TestCollect (0.0s)
PASSED collect.TestCollectBuildFailure (0.0s)
PASSED collect.TestFindComponentTestPaths (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_does_not_exist_in_directory_tree (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_does_not_exist_in_provided_directory (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_exists_in_directory_tree (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_exists_in_provided_directory (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_should_be_the_same (0.0s)
PASSED collect.TestFindComponentTestPaths/directory_above_service_root (0.0s)
PASSED collect.TestFindComponentTestPaths/in_component_tests_directory (0.0s)
PASSED collect.TestFindComponentTestPaths/multiple_component_tests_dirs (0.0s)
PASSED collect.TestFindPythonTestPath (0.0s)
PASSED collect.TestFindPythonTestPath/not_a_python_package (0.0s)
PASSED collect.TestFindPythonTestPath/python_tests_path_does_not_exist (0.0s)
PASSED collect.TestFindPythonTestPath/python_tests_path_exists (0.0s)
PASSED collect.TestRunComponentTests (0.0s)
PASSED collect.TestSetTest (0.0s)
PASSED collect.TestSetTest/component (0.0s)
PASSED collect.TestSetTest/fail (0.0s)
PASSED collect.TestSetTest/unit (0.0s)
PASSED collect.TestString (0.0s)
PASSED collect.TestString/should_be_component (0.0s)
PASSED collect.TestString/should_be_unit (0.0s)
Test cases: finished with 23 passing and 0 failing out of 23 test cases
Executed 1 out of 1 test: 1 test passes.
```
After the fix:
```
INFO: Found 1 target and 1 test target...
INFO: Elapsed time: 147.787s, Critical Path: 42.03s
INFO: 476 processes: 36 internal, 440 darwin-sandbox.
INFO: Build completed successfully, 476 total actions
//foo/collect:go_default_test PASSED in 0.7s
PASSED collect.TestCollectBuildFailure (0.0s)
PASSED collect.TestFindComponentTestPaths (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_does_not_exist_in_directory_tree (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_does_not_exist_in_provided_directory (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_exists_in_directory_tree (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_exists_in_provided_directory (0.0s)
PASSED collect.TestFindComponentTestPaths/component_tests_dir_should_be_the_same (0.0s)
PASSED collect.TestFindComponentTestPaths/directory_above_service_root (0.0s)
PASSED collect.TestFindComponentTestPaths/in_component_tests_directory (0.0s)
PASSED collect.TestFindComponentTestPaths/multiple_component_tests_dirs (0.0s)
PASSED collect.TestFindPythonTestPath (0.0s)
PASSED collect.TestFindPythonTestPath/not_a_python_package (0.0s)
PASSED collect.TestFindPythonTestPath/python_tests_path_does_not_exist (0.0s)
PASSED collect.TestFindPythonTestPath/python_tests_path_exists (0.0s)
PASSED collect.TestRunComponentTests (0.0s)
PASSED collect.TestSetTest (0.0s)
PASSED collect.TestSetTest/component (0.0s)
PASSED collect.TestSetTest/fail (0.0s)
PASSED collect.TestSetTest/unit (0.0s)
PASSED collect.TestString (0.0s)
PASSED collect.TestString/should_be_component (0.0s)
PASSED collect.TestString/should_be_unit (0.0s)
SKIPPED collect.TestCollect (0.0s)
Test cases: finished with 22 passing, 1 skipped and 0 failing out of 23 test cases
Executed 1 out of 1 test: 1 test passes.
```
Closes #23766.
PiperOrigin-RevId: 688477216
Change-Id: I4353d1dd4801a19ec56aae40d632d9f1061cfdbe
diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestXmlOutputParser.java b/src/main/java/com/google/devtools/build/lib/exec/TestXmlOutputParser.java
index fea69d2..54e84ee 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/TestXmlOutputParser.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/TestXmlOutputParser.java
@@ -216,6 +216,7 @@
throws XMLStreamException, TestXmlOutputParserException {
int failures = 0;
int errors = 0;
+ boolean skipped = false;
while (true) {
int event = parser.next();
@@ -243,6 +244,10 @@
errors += 1;
skipCompleteElement(parser);
break;
+ case "skipped":
+ skipped = true;
+ skipCompleteElement(parser);
+ break;
case "testdecorator":
builder.addChild(parseTestDecorator(parser));
break;
@@ -268,6 +273,8 @@
builder.setStatus(TestCase.Status.ERROR);
} else if (failures > 0) {
builder.setStatus(TestCase.Status.FAILED);
+ } else if (skipped) {
+ builder.setStatus(TestCase.Status.SKIPPED);
} else {
builder.setStatus(TestCase.Status.PASSED);
}
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 363f53b..fb0d468 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
@@ -58,6 +58,7 @@
int totalTestCases;
int totalFailedTestCases;
+ int totalSkippedTestCases;
int totalUnknownTestCases;
}
@@ -195,6 +196,7 @@
stats.totalTestCases += summary.getTotalTestCases();
stats.totalUnknownTestCases += summary.getUnkownTestCases();
stats.totalFailedTestCases += summary.getFailedTestCases().size();
+ stats.totalSkippedTestCases += summary.getSkippedTestCases().size();
}
stats.failedCount = summaries.size() - stats.passCount;
@@ -265,13 +267,20 @@
TestSummaryFormat testSummaryFormat = options.getOptions(ExecutionOptions.class).testSummary;
if (testSummaryFormat == DETAILED || testSummaryFormat == TESTCASE) {
int passCount =
- stats.totalTestCases - stats.totalFailedTestCases - stats.totalUnknownTestCases;
+ stats.totalTestCases
+ - stats.totalFailedTestCases
+ - stats.totalUnknownTestCases
+ - stats.totalSkippedTestCases;
String message =
String.format(
- "Test cases: finished with %s%d passing%s and %s%d failing%s out of %d test cases",
+ "Test cases: finished with %s%d passing%s, %s%d skipped%s and %s%d failing%s out of"
+ + " %d test cases",
passCount > 0 ? AnsiTerminalPrinter.Mode.INFO : "",
passCount,
AnsiTerminalPrinter.Mode.DEFAULT,
+ stats.totalSkippedTestCases > 0 ? AnsiTerminalPrinter.Mode.WARNING : "",
+ stats.totalSkippedTestCases,
+ AnsiTerminalPrinter.Mode.DEFAULT,
stats.totalFailedTestCases > 0 ? AnsiTerminalPrinter.Mode.ERROR : "",
stats.totalFailedTestCases,
AnsiTerminalPrinter.Mode.DEFAULT,
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 12d4ee1..80a1ffa 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,7 +41,6 @@
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;
@@ -216,12 +215,10 @@
// Don't count test cases that were not run.
return 0;
}
- if (testCase.getStatus() != TestCase.Status.PASSED) {
- this.summary.failedTestCases.add(testCase);
- }
-
- if (testCase.getStatus() == Status.PASSED) {
- this.summary.passedTestCases.add(testCase);
+ switch (testCase.getStatus()) {
+ case PASSED -> this.summary.passedTestCases.add(testCase);
+ case SKIPPED -> this.summary.skippedTestCases.add(testCase);
+ default -> this.summary.failedTestCases.add(testCase);
}
return 1;
@@ -409,6 +406,7 @@
private boolean wasUnreportedWrongSize;
private List<TestCase> failedTestCases = new ArrayList<>();
private final List<TestCase> passedTestCases = new ArrayList<>();
+ private final List<TestCase> skippedTestCases = new ArrayList<>();
private List<Path> passedLogs = new ArrayList<>();
private List<Path> failedLogs = new ArrayList<>();
private List<String> warnings = new ArrayList<>();
@@ -520,6 +518,10 @@
return failedTestCases;
}
+ public List<TestCase> getSkippedTestCases() {
+ return skippedTestCases;
+ }
+
public List<TestCase> getPassedTestCases() {
return passedTestCases;
}
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 5f3efda..eb9ecc4 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
@@ -25,7 +25,6 @@
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;
@@ -167,6 +166,9 @@
for (TestCase testCase : summary.getPassedTestCases()) {
TestSummaryPrinter.printTestCase(terminalPrinter, testCase);
}
+ for (TestCase testCase : summary.getSkippedTestCases()) {
+ TestSummaryPrinter.printTestCase(terminalPrinter, testCase);
+ }
if (summary.getStatus() == BlazeTestStatus.FAILED) {
if (summary.getFailedTestCasesStatus() == FailedTestCasesStatus.NOT_AVAILABLE) {
@@ -227,7 +229,12 @@
timeSummary = "";
}
- Mode mode = (testCase.getStatus() == Status.PASSED) ? Mode.INFO : Mode.ERROR;
+ Mode mode =
+ switch (testCase.getStatus()) {
+ case PASSED -> Mode.INFO;
+ case SKIPPED -> Mode.WARNING;
+ default -> Mode.ERROR;
+ };
terminalPrinter.print(
" "
+ mode
diff --git a/src/main/protobuf/test_status.proto b/src/main/protobuf/test_status.proto
index e77a19f..61cf22f 100644
--- a/src/main/protobuf/test_status.proto
+++ b/src/main/protobuf/test_status.proto
@@ -58,6 +58,7 @@
PASSED = 0;
FAILED = 1;
ERROR = 2;
+ SKIPPED = 3;
}
repeated TestCase child = 1;
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 b2ad9f0..fd1ffe4 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
@@ -56,6 +56,7 @@
private BlazeTestStatus targetStatus;
private int numFailedTestCases;
+ private int numSkippedTestCases;
private int numUnknownTestCases;
private int numTotalTestCases;
private TestSummaryFormat testSummaryFormat;
@@ -172,6 +173,27 @@
String printed = getPrintedMessage();
assertThat(printed).contains(info("10 passing"));
assertThat(printed).contains("0 failing");
+ assertThat(printed).contains("0 skipped");
+ assertThat(printed).contains("out of 10 test cases");
+ assertThat(printed).doesNotContain(SOME_TARGETS_ARE_MISSING_TEST_CASES_DISCLAIMER);
+ assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.ERROR.toString());
+ }
+
+ @Test
+ public void detailedOption_allPassButSomeSkipped() throws Exception {
+ testSummaryFormat = ExecutionOptions.TestSummaryFormat.DETAILED;
+ numFailedTestCases = 0;
+ numUnknownTestCases = 0;
+ numTotalTestCases = 10;
+ numSkippedTestCases = 2;
+ targetStatus = BlazeTestStatus.PASSED;
+
+ printTestCaseSummary();
+
+ String printed = getPrintedMessage();
+ assertThat(printed).contains(info("8 passing"));
+ assertThat(printed).contains("0 failing");
+ assertThat(printed).contains(warn("2 skipped"));
assertThat(printed).contains("out of 10 test cases");
assertThat(printed).doesNotContain(SOME_TARGETS_ARE_MISSING_TEST_CASES_DISCLAIMER);
assertThat(printed).doesNotContain(AnsiTerminalPrinter.Mode.ERROR.toString());
@@ -313,9 +335,12 @@
when(testSummary.getUnkownTestCases()).thenReturn(numUnknownTestCases);
TestCase failedTestCase = TestCase.newBuilder().setStatus(Status.FAILED).build();
List<TestCase> failedTestCases = Collections.nCopies(numFailedTestCases, failedTestCase);
+ TestCase skippedTestCase = TestCase.newBuilder().setStatus(Status.SKIPPED).build();
+ List<TestCase> skippedTestCases = Collections.nCopies(numSkippedTestCases, skippedTestCase);
Label labelA = Label.parseCanonical("//foo/bar:baz");
when(testSummary.getFailedTestCases()).thenReturn(failedTestCases);
+ when(testSummary.getSkippedTestCases()).thenReturn(skippedTestCases);
when(testSummary.getStatus()).thenReturn(targetStatus);
when(testSummary.getLabel()).thenReturn(labelA);
@@ -342,6 +367,10 @@
return AnsiTerminalPrinter.Mode.INFO + message + AnsiTerminalPrinter.Mode.DEFAULT;
}
+ private static String warn(String message) {
+ return AnsiTerminalPrinter.Mode.WARNING + message + AnsiTerminalPrinter.Mode.DEFAULT;
+ }
+
private static String error(String message) {
return AnsiTerminalPrinter.Mode.ERROR + message + AnsiTerminalPrinter.Mode.DEFAULT;
}
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 a82242a..253d21f 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
@@ -551,6 +551,7 @@
.addChild(newDetail("apple", TestCase.Status.FAILED, 1000L))
.addChild(newDetail("banana", TestCase.Status.PASSED, 1000L))
.addChild(newDetail("cherry", TestCase.Status.ERROR, 1000L))
+ .addChild(newDetail("sugarcane", Status.SKIPPED, 0))
.build();
TestSummary summary =
@@ -562,6 +563,7 @@
verify(printer).print(find("FAILED.*apple"));
verify(printer).print(find("PASSED.*banana"));
verify(printer).print(find("ERROR.*cherry"));
+ verify(printer).print(find("SKIPPED.*sugarcane"));
}
@Test
@@ -571,6 +573,7 @@
.setName("tests")
.setRunDurationMillis(5000L)
.addChild(newDetail("apple", TestCase.Status.PASSED, 1000L))
+ .addChild(newDetail("banana", Status.SKIPPED, 0))
.build();
TestSummary summary =
@@ -580,6 +583,7 @@
TestSummaryPrinter.print(summary, printer, Path::getPathString, true, true);
verify(printer).print(contains("//package:name"));
verify(printer).print(find("PASSED.*apple"));
+ verify(printer).print(find("SKIPPED.*banana"));
}
@Test
@@ -591,12 +595,13 @@
.addChild(newDetail("apple", TestCase.Status.FAILED, 1000L))
.addChild(newDetail("banana", TestCase.Status.PASSED, 1000L))
.addChild(newDetail("cherry", TestCase.Status.ERROR, 1000L))
+ .addChild(newDetail("sugarcane", Status.SKIPPED, 0))
.build();
TestSummary summary =
getTemplateBuilder().collectTestCases(rootCase).setStatus(BlazeTestStatus.FAILED).build();
- assertThat(summary.getTotalTestCases()).isEqualTo(3);
+ assertThat(summary.getTotalTestCases()).isEqualTo(4);
}
@Test
