diff --git a/WHATSNEW b/WHATSNEW index 25a4ef0315..4cfe9393ea 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -41,6 +41,13 @@ Other changes: Fixed bugs: ----------- + * 's legacy-xml listener reported a skipped container as + a single skip and left the tests it holds out of the report + entirely. Every test of a skipped container is now reported as + skipped, and skipped tests count towards the total the way the + junit task counted ignored tests. + Bugzilla Report 69683 + * When running with "microsoft" Java, Ant used to hardcode the "$JAVA_HOME/Packages" directory in the runtime Paths for certain tasks. The JDK shipped by Microsoft no longer contains that diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatter.java b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatter.java index f72822d383..d0087d73fb 100644 --- a/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatter.java +++ b/src/main/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatter.java @@ -35,6 +35,7 @@ import java.io.OutputStream; import java.io.Reader; import java.util.Date; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Optional; import java.util.Properties; @@ -92,7 +93,39 @@ public void dynamicTestRegistered(final TestIdentifier testIdentifier) { @Override public void executionSkipped(final TestIdentifier testIdentifier, final String reason) { final long currentTime = System.currentTimeMillis(); + // JUnit reports a skipped container only once, but the report format deals in tests, + // so each test the container holds has to be reported as skipped individually + // (https://bz.apache.org/bugzilla/show_bug.cgi?id=69683) + final Set skippedTests = new LinkedHashSet<>(); + if (testIdentifier.isTest()) { + skippedTests.add(testIdentifier); + } + if (testIdentifier.isContainer()) { + for (final TestIdentifier descendant : this.testPlan.getDescendants(testIdentifier)) { + if (descendant.isTest()) { + skippedTests.add(descendant); + } + } + } + if (skippedTests.isEmpty()) { + // a container which holds no tests - as before, this contributes to the skipped + // count but has no testcase element of its own, since the format has no way to + // represent a container + recordSkipped(testIdentifier, reason, currentTime, false); + return; + } + for (final TestIdentifier skippedTest : skippedTests) { + recordSkipped(skippedTest, reason, currentTime, true); + } + } + + private void recordSkipped(final TestIdentifier testIdentifier, final String reason, + final long currentTime, final boolean countTowardsTotal) { this.numTestsSkipped.incrementAndGet(); + if (countTowardsTotal) { + // the legacy report counts skipped tests towards the total, like the junit task did + this.numTestsRun.incrementAndGet(); + } this.skipped.put(testIdentifier, Optional.ofNullable(reason)); // a skipped test is considered started and ended now final Stats stats = new Stats(testIdentifier, currentTime); diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatterTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatterTest.java index 3735de8389..d94314976f 100644 --- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatterTest.java +++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/junitlauncher/LegacyXmlResultFormatterTest.java @@ -20,6 +20,14 @@ import org.apache.tools.ant.Project; import org.junit.Test; import org.junit.platform.engine.ConfigurationParameters; +import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.TestSource; +import org.junit.platform.engine.UniqueId; +import org.junit.platform.engine.support.descriptor.AbstractTestDescriptor; +import org.junit.platform.engine.support.descriptor.ClassSource; +import org.junit.platform.engine.support.descriptor.EngineDescriptor; +import org.junit.platform.engine.support.descriptor.MethodSource; +import org.junit.platform.launcher.TestIdentifier; import org.junit.platform.launcher.TestPlan; import java.io.ByteArrayOutputStream; @@ -32,6 +40,7 @@ import java.util.function.Function; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertThat; public class LegacyXmlResultFormatterTest { @@ -39,6 +48,8 @@ public class LegacyXmlResultFormatterTest { private static final String KEY = "key"; private static final String ORIG = "<\u0000&>foo"; private static final String ENCODED = "<&#0;&>foo"; + private static final String CLASS_NAME = "org.example.ASkippedTest"; + private static final String SKIP_REASON = "the whole class is disabled"; private final LegacyXmlResultFormatter f = new LegacyXmlResultFormatter(); @@ -57,7 +68,116 @@ public void encodesSysOutProperly() throws Exception { assertThat(result, containsString(ENCODED)); } + /** + * Skipping a container skips every test it holds, and each of those has to show up in the + * report - a consumer of the legacy format only understands skipped tests. + * + * @see Bugzilla 69683 + */ + @Test + public void reportsEveryTestOfASkippedContainerAsSkipped() throws Exception { + final EngineDescriptor engine = new EngineDescriptor(UniqueId.forEngine("dummy"), "dummy"); + final Container testClass = new Container(engine.getUniqueId().append("class", CLASS_NAME), + CLASS_NAME, ClassSource.from(CLASS_NAME)); + engine.addChild(testClass); + testClass.addChild(new Leaf(testClass.getUniqueId().append("method", "first"), "first", + MethodSource.from(CLASS_NAME, "first"))); + testClass.addChild(new Leaf(testClass.getUniqueId().append("method", "second"), "second", + MethodSource.from(CLASS_NAME, "second"))); + + final TestPlan plan = startTest(engine); + f.executionSkipped(TestIdentifier.from(testClass), SKIP_REASON); + final String result = finishTest(plan); + + assertThat(result, containsString("tests=\"2\"")); + assertThat(result, containsString("skipped=\"2\"")); + assertThat(result, containsString("name=\"first\"")); + assertThat(result, containsString("name=\"second\"")); + assertThat(result, containsString("Bugzilla 69683 + */ + @Test + public void countsASkippedTestTowardsTheTotal() throws Exception { + final EngineDescriptor engine = new EngineDescriptor(UniqueId.forEngine("dummy"), "dummy"); + final Container testClass = new Container(engine.getUniqueId().append("class", CLASS_NAME), + CLASS_NAME, ClassSource.from(CLASS_NAME)); + engine.addChild(testClass); + final Leaf test = new Leaf(testClass.getUniqueId().append("method", "only"), "only", + MethodSource.from(CLASS_NAME, "only")); + testClass.addChild(test); + + final TestPlan plan = startTest(engine); + f.executionSkipped(TestIdentifier.from(test), SKIP_REASON); + final String result = finishTest(plan); + + assertThat(result, containsString("tests=\"1\"")); + assertThat(result, containsString("skipped=\"1\"")); + } + + /** + * A container holding no tests has nothing to report a testcase for, so it keeps + * contributing to the skipped count alone, as it did before. + * + * @see Bugzilla 69683 + */ + @Test + public void countsAnEmptySkippedContainerWithoutReportingATestcase() throws Exception { + final EngineDescriptor engine = new EngineDescriptor(UniqueId.forEngine("dummy"), "dummy"); + final Container empty = new Container(engine.getUniqueId().append("class", CLASS_NAME), + CLASS_NAME, ClassSource.from(CLASS_NAME)); + engine.addChild(empty); + + final TestPlan plan = startTest(engine); + f.executionSkipped(TestIdentifier.from(empty), SKIP_REASON); + final String result = finishTest(plan); + + assertThat(result, containsString("tests=\"0\"")); + assertThat(result, containsString("skipped=\"1\"")); + assertThat(result, not(containsString(" getProject() { return Optional.empty(); } }); - final ConfigurationParameters dummyParams = new ConfigurationParameters() { + } + + private ConfigurationParameters dummyParams() { + return new ConfigurationParameters() { @Override public Optional get(String key) { return Optional.empty(); @@ -99,9 +222,6 @@ public Set keySet() { return Collections.emptySet(); } }; - final TestPlan testPlan = TestPlan.from(Collections.emptySet(), dummyParams); - f.testPlanExecutionStarted(testPlan); - return testPlan; } private String finishTest(final TestPlan testPlan) throws IOException {