diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc index f9e994b0bd79..f3186db2b66a 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc @@ -47,7 +47,7 @@ JUnit repository on GitHub. ==== Bug Fixes -* ❓ +* Fix reporting of JUnit 3 test classes with `@Ignored` annotation ==== Deprecations and Breaking Changes diff --git a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/RunnerTestDescriptor.java b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/RunnerTestDescriptor.java index 0ad3c5f9b6a0..03ae08dd0a6c 100644 --- a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/RunnerTestDescriptor.java +++ b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/descriptor/RunnerTestDescriptor.java @@ -43,12 +43,14 @@ public class RunnerTestDescriptor extends VintageTestDescriptor { private final Set rejectedExclusions = new HashSet<>(); private Runner runner; + private final boolean ignored; private boolean wasFiltered; private List filters = new ArrayList<>(); - public RunnerTestDescriptor(UniqueId uniqueId, Class testClass, Runner runner) { + public RunnerTestDescriptor(UniqueId uniqueId, Class testClass, Runner runner, boolean ignored) { super(uniqueId, runner.getDescription(), testClass.getSimpleName(), ClassSource.from(testClass)); this.runner = runner; + this.ignored = ignored; } @Override @@ -155,6 +157,10 @@ private Runner getRunnerToReport() { return (runner instanceof RunnerDecorator) ? ((RunnerDecorator) runner).getDecoratedRunner() : runner; } + public boolean isIgnored() { + return ignored; + } + private static class ExcludeDescriptionFilter extends Filter { private final Description description; diff --git a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClassSelectorResolver.java b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClassSelectorResolver.java index a2d1e7d4c80c..14445f155b4e 100644 --- a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClassSelectorResolver.java +++ b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/ClassSelectorResolver.java @@ -26,7 +26,6 @@ import org.junit.platform.engine.discovery.UniqueIdSelector; import org.junit.platform.engine.support.discovery.SelectorResolver; import org.junit.runner.Runner; -import org.junit.runners.model.RunnerBuilder; import org.junit.vintage.engine.descriptor.RunnerTestDescriptor; /** @@ -34,7 +33,7 @@ */ class ClassSelectorResolver implements SelectorResolver { - private static final RunnerBuilder RUNNER_BUILDER = new DefensiveAllDefaultPossibilitiesBuilder(); + private static final DefensiveAllDefaultPossibilitiesBuilder RUNNER_BUILDER = new DefensiveAllDefaultPossibilitiesBuilder(); private final ClassFilter classFilter; @@ -76,7 +75,7 @@ private Resolution resolveTestClass(Class testClass, Context context) { private RunnerTestDescriptor createRunnerTestDescriptor(TestDescriptor parent, Class testClass, Runner runner) { UniqueId uniqueId = parent.getUniqueId().append(SEGMENT_TYPE_RUNNER, testClass.getName()); - return new RunnerTestDescriptor(uniqueId, testClass, runner); + return new RunnerTestDescriptor(uniqueId, testClass, runner, RUNNER_BUILDER.isIgnored(runner)); } } diff --git a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/DefensiveAllDefaultPossibilitiesBuilder.java b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/DefensiveAllDefaultPossibilitiesBuilder.java index 0da94f689fde..b35dc20077c4 100644 --- a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/DefensiveAllDefaultPossibilitiesBuilder.java +++ b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/DefensiveAllDefaultPossibilitiesBuilder.java @@ -64,6 +64,10 @@ public Runner runnerForClass(Class testClass) throws Throwable { return runner; } + boolean isIgnored(Runner runner) { + return runner instanceof IgnoredClassRunner || runner instanceof IgnoringRunnerDecorator; + } + /** * Instead of checking for the {@link Ignore} annotation and returning an * {@link IgnoredClassRunner} from {@link IgnoredBuilder}, we've let the @@ -72,7 +76,7 @@ public Runner runnerForClass(Class testClass) throws Throwable { * override its runtime behavior (i.e. skip execution) but return its * regular {@link org.junit.runner.Description}. */ - private Runner decorateIgnoredTestClass(Runner runner) { + private IgnoringRunnerDecorator decorateIgnoredTestClass(Runner runner) { if (runner instanceof Filterable) { return new FilterableIgnoringRunnerDecorator(runner); } diff --git a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/RunListenerAdapter.java b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/RunListenerAdapter.java index 9cefb8faee30..58144877d04a 100644 --- a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/RunListenerAdapter.java +++ b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/RunListenerAdapter.java @@ -20,6 +20,7 @@ import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.TestExecutionResult; import org.junit.platform.engine.UniqueId; +import org.junit.platform.engine.support.descriptor.ClassSource; import org.junit.runner.Description; import org.junit.runner.Result; import org.junit.runner.notification.Failure; @@ -49,7 +50,7 @@ class RunListenerAdapter extends RunListener { @Override public void testRunStarted(Description description) { - if (description.isSuite() && description.getAnnotation(Ignore.class) == null) { + if (description.isSuite() && !testRun.getRunnerTestDescriptor().isIgnored()) { fireExecutionStarted(testRun.getRunnerTestDescriptor(), EventType.REPORTED); } } @@ -65,7 +66,9 @@ public void testSuiteStarted(Description description) { @Override public void testIgnored(Description description) { - testIgnored(lookupOrRegisterNextTestDescriptor(description), determineReasonForIgnoredTest(description)); + TestDescriptor testDescriptor = lookupOrRegisterNextTestDescriptor(description); + String reason = determineReasonForIgnoredTest(testDescriptor, description).orElse(""); + testIgnored(testDescriptor, reason); } @Override @@ -176,9 +179,21 @@ private void testIgnored(TestDescriptor testDescriptor, String reason) { fireExecutionSkipped(testDescriptor, reason); } - private String determineReasonForIgnoredTest(Description description) { - Ignore ignoreAnnotation = description.getAnnotation(Ignore.class); - return Optional.ofNullable(ignoreAnnotation).map(Ignore::value).orElse(""); + private Optional determineReasonForIgnoredTest(TestDescriptor testDescriptor, Description description) { + Optional reason = getReason(description.getAnnotation(Ignore.class)); + if (reason.isPresent()) { + return reason; + } + // Workaround for some runners (e.g. JUnit38ClassRunner) don't include the @Ignore annotation + // in the description, so we read it from the test class directly + return testDescriptor.getSource() // + .filter(ClassSource.class::isInstance) // + .map(source -> ((ClassSource) source).getJavaClass()) // + .flatMap(testClass -> getReason(testClass.getAnnotation(Ignore.class))); + } + + private static Optional getReason(Ignore annotation) { + return Optional.ofNullable(annotation).map(Ignore::value); } private void dynamicTestRegistered(TestDescriptor testDescriptor) { diff --git a/junit-vintage-engine/src/test/java/org/junit/vintage/engine/VintageTestEngineExecutionTests.java b/junit-vintage-engine/src/test/java/org/junit/vintage/engine/VintageTestEngineExecutionTests.java index 66af815aff99..cd33b52e2269 100644 --- a/junit-vintage-engine/src/test/java/org/junit/vintage/engine/VintageTestEngineExecutionTests.java +++ b/junit-vintage-engine/src/test/java/org/junit/vintage/engine/VintageTestEngineExecutionTests.java @@ -10,6 +10,7 @@ package org.junit.vintage.engine; +import static java.util.function.Predicate.isEqual; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assumptions.assumeTrue; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; @@ -56,8 +57,10 @@ import org.junit.runner.notification.RunNotifier; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; +import org.junit.vintage.engine.samples.junit3.IgnoredJUnit3TestCase; import org.junit.vintage.engine.samples.junit3.JUnit3ParallelSuiteWithSubsuites; import org.junit.vintage.engine.samples.junit3.JUnit3SuiteWithSubsuites; +import org.junit.vintage.engine.samples.junit3.JUnit4SuiteWithIgnoredJUnit3TestCase; import org.junit.vintage.engine.samples.junit3.PlainJUnit3TestCaseWithSingleTestWhichFails; import org.junit.vintage.engine.samples.junit4.CompletelyDynamicTestCase; import org.junit.vintage.engine.samples.junit4.EmptyIgnoredTestCase; @@ -896,6 +899,27 @@ void executesRegularSpockFeatureMethod() { event(engine(), finishedSuccessfully())); } + @Test + void executesIgnoredJUnit3TestCase() { + var suiteClass = IgnoredJUnit3TestCase.class; + execute(suiteClass).allEvents().assertEventsMatchExactly( // + event(engine(), started()), // + event(container(suiteClass), skippedWithReason(isEqual("testing"))), // + event(engine(), finishedSuccessfully())); + } + + @Test + void executesJUnit4SuiteWithIgnoredJUnit3TestCase() { + var suiteClass = JUnit4SuiteWithIgnoredJUnit3TestCase.class; + var testClass = IgnoredJUnit3TestCase.class; + execute(suiteClass).allEvents().assertEventsMatchExactly( // + event(engine(), started()), // + event(container(suiteClass), started()), // + event(container(testClass), skippedWithReason(isEqual("testing"))), // + event(container(suiteClass), finishedSuccessfully()), // + event(engine(), finishedSuccessfully())); + } + private static EngineExecutionResults execute(Class testClass) { return execute(request(testClass)); } diff --git a/junit-vintage-engine/src/test/java/org/junit/vintage/engine/execution/TestRunTests.java b/junit-vintage-engine/src/test/java/org/junit/vintage/engine/execution/TestRunTests.java index 69272fd511db..918e32dffa96 100644 --- a/junit-vintage-engine/src/test/java/org/junit/vintage/engine/execution/TestRunTests.java +++ b/junit-vintage-engine/src/test/java/org/junit/vintage/engine/execution/TestRunTests.java @@ -32,7 +32,8 @@ class TestRunTests { void returnsEmptyOptionalForUnknownDescriptions() throws Exception { Class testClass = PlainJUnit4TestCaseWithSingleTestWhichFails.class; var runnerId = engineId().append(SEGMENT_TYPE_RUNNER, testClass.getName()); - var runnerTestDescriptor = new RunnerTestDescriptor(runnerId, testClass, new BlockJUnit4ClassRunner(testClass)); + var runnerTestDescriptor = new RunnerTestDescriptor(runnerId, testClass, new BlockJUnit4ClassRunner(testClass), + false); var unknownDescription = createTestDescription(testClass, "dynamicTest"); var testRun = new TestRun(runnerTestDescriptor); @@ -45,7 +46,8 @@ void returnsEmptyOptionalForUnknownDescriptions() throws Exception { void registersDynamicTestDescriptors() throws Exception { Class testClass = PlainJUnit4TestCaseWithSingleTestWhichFails.class; var runnerId = engineId().append(SEGMENT_TYPE_RUNNER, testClass.getName()); - var runnerTestDescriptor = new RunnerTestDescriptor(runnerId, testClass, new BlockJUnit4ClassRunner(testClass)); + var runnerTestDescriptor = new RunnerTestDescriptor(runnerId, testClass, new BlockJUnit4ClassRunner(testClass), + false); var dynamicTestId = runnerId.append(SEGMENT_TYPE_DYNAMIC, "dynamicTest"); var dynamicDescription = createTestDescription(testClass, "dynamicTest"); var dynamicTestDescriptor = new VintageTestDescriptor(dynamicTestId, dynamicDescription, null); diff --git a/junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit3/IgnoredJUnit3TestCase.java b/junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit3/IgnoredJUnit3TestCase.java new file mode 100644 index 000000000000..64015bf57d52 --- /dev/null +++ b/junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit3/IgnoredJUnit3TestCase.java @@ -0,0 +1,28 @@ +/* + * Copyright 2015-2023 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.vintage.engine.samples.junit3; + +import junit.framework.TestCase; + +import org.junit.Assert; +import org.junit.Ignore; + +/** + * @since 4.12 + */ +@Ignore("testing") +public class IgnoredJUnit3TestCase extends TestCase { + + public void test() { + Assert.fail("this test should be ignored"); + } + +} diff --git a/junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit3/JUnit4SuiteWithIgnoredJUnit3TestCase.java b/junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit3/JUnit4SuiteWithIgnoredJUnit3TestCase.java new file mode 100644 index 000000000000..ba89153ace0c --- /dev/null +++ b/junit-vintage-engine/src/testFixtures/java/org/junit/vintage/engine/samples/junit3/JUnit4SuiteWithIgnoredJUnit3TestCase.java @@ -0,0 +1,20 @@ +/* + * Copyright 2015-2023 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.vintage.engine.samples.junit3; + +import org.junit.runner.RunWith; +import org.junit.runners.Suite; +import org.junit.runners.Suite.SuiteClasses; + +@RunWith(Suite.class) +@SuiteClasses({ IgnoredJUnit3TestCase.class }) +public class JUnit4SuiteWithIgnoredJUnit3TestCase { +}