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..ee47d1ec8e01 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 @@ -55,11 +55,12 @@ class DefensiveAllDefaultPossibilitiesBuilder extends AllDefaultPossibilitiesBui @Override public Runner runnerForClass(Class testClass) throws Throwable { Runner runner = super.runnerForClass(testClass); - if (testClass.getAnnotation(Ignore.class) != null) { + Ignore ignoreAnnotation = testClass.getAnnotation(Ignore.class); + if (ignoreAnnotation != null) { if (runner == null) { return new IgnoredClassRunner(testClass); } - return decorateIgnoredTestClass(runner); + return decorateIgnoredTestClass(runner, ignoreAnnotation); } return runner; } @@ -72,11 +73,11 @@ 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 Runner decorateIgnoredTestClass(Runner runner, Ignore ignoreAnnotation) { if (runner instanceof Filterable) { - return new FilterableIgnoringRunnerDecorator(runner); + return new FilterableIgnoringRunnerDecorator(runner, ignoreAnnotation); } - return new IgnoringRunnerDecorator(runner); + return new IgnoringRunnerDecorator(runner, ignoreAnnotation); } @Override diff --git a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/FilterableIgnoringRunnerDecorator.java b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/FilterableIgnoringRunnerDecorator.java index 636ef2554802..29306be25caa 100644 --- a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/FilterableIgnoringRunnerDecorator.java +++ b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/FilterableIgnoringRunnerDecorator.java @@ -10,6 +10,7 @@ package org.junit.vintage.engine.discovery; +import org.junit.Ignore; import org.junit.platform.commons.util.Preconditions; import org.junit.runner.Runner; import org.junit.runner.manipulation.Filter; @@ -23,8 +24,8 @@ */ class FilterableIgnoringRunnerDecorator extends IgnoringRunnerDecorator implements Filterable { - FilterableIgnoringRunnerDecorator(Runner runner) { - super(runner); + FilterableIgnoringRunnerDecorator(Runner runner, Ignore ignoreAnnotation) { + super(runner, ignoreAnnotation); Preconditions.condition(runner instanceof Filterable, () -> "Runner must be an instance of Filterable: " + runner.getClass().getName()); } diff --git a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/IgnoringRunnerDecorator.java b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/IgnoringRunnerDecorator.java index 3b5597e51169..a53fd9cb2566 100644 --- a/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/IgnoringRunnerDecorator.java +++ b/junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/IgnoringRunnerDecorator.java @@ -10,11 +10,20 @@ package org.junit.vintage.engine.discovery; +import java.lang.annotation.Annotation; +import java.util.ArrayList; +import java.util.List; + +import org.junit.Ignore; +import org.junit.internal.runners.JUnit38ClassRunner; +import org.junit.platform.commons.logging.Logger; +import org.junit.platform.commons.logging.LoggerFactory; import org.junit.platform.commons.util.Preconditions; import org.junit.runner.Description; import org.junit.runner.Runner; import org.junit.runner.notification.RunNotifier; import org.junit.vintage.engine.descriptor.RunnerDecorator; +import org.junit.vintage.engine.descriptor.RunnerTestDescriptor; /** * Decorator for Runners that will be ignored completely. @@ -26,15 +35,29 @@ */ class IgnoringRunnerDecorator extends Runner implements RunnerDecorator { + private static final Logger logger = LoggerFactory.getLogger(RunnerTestDescriptor.class); + protected final Runner runner; + private final Ignore testClassIgnoreAnnotation; - IgnoringRunnerDecorator(Runner runner) { + IgnoringRunnerDecorator(Runner runner, Ignore ignoreAnnotation) { this.runner = Preconditions.notNull(runner, "Runner must not be null"); + this.testClassIgnoreAnnotation = Preconditions.notNull(ignoreAnnotation, + "Test class @Ignore annotation must not be null"); } @Override public Description getDescription() { - return runner.getDescription(); + Description originalDescription = runner.getDescription(); + + if (runner instanceof JUnit38ClassRunner) { + return junit38ClassRunnerDescriptionWithIgnoreAnnotation(originalDescription); + } + else if (originalDescription.getAnnotation(Ignore.class) == null) { + warnAboutMissingIgnoreAnnotation(originalDescription); + } + + return originalDescription; } @Override @@ -46,4 +69,26 @@ public void run(RunNotifier notifier) { public Runner getDecoratedRunner() { return runner; } + + /** + * {@link JUnit38ClassRunner} does not add class-level annotations to the runner description, + * which results in an inconsistent behavior when combined with the vintage engine: the runner description + * will be marked as started because the runner told so, but it will alos be reported as skipped by IgnoringRunnerDecorator + * which detected the @Ignore annotation on the test Java class. + */ + private Description junit38ClassRunnerDescriptionWithIgnoreAnnotation(Description runnerDescription) { + List effectiveAnnotations = new ArrayList<>(runnerDescription.getAnnotations()); + effectiveAnnotations.add(testClassIgnoreAnnotation); + + Description updatedRunnerDescription = Description.createTestDescription(runnerDescription.getClassName(), + runnerDescription.getMethodName(), effectiveAnnotations.toArray(new Annotation[0])); + + runnerDescription.getChildren().forEach(updatedRunnerDescription::addChild); + return updatedRunnerDescription; + } + + private void warnAboutMissingIgnoreAnnotation(Description originalDescription) { + logger.warn(() -> "Custom test runner '" + runner.getClass().getName() + + "' did not add an @Ignore annotation to the runner description " + originalDescription); + } } 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 781b7ead1b9d..638066a1be9e 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 @@ -902,7 +902,7 @@ void executesIgnoredJUnit3TestCase() { var suiteClass = IgnoredJUnit3TestCase.class; execute(suiteClass).allEvents().assertEventsMatchExactly( // event(engine(), started()), // - event(container(suiteClass), skippedWithReason(__ -> true)), // + event(container(suiteClass), skippedWithReason("respected by Vintage engine")), // event(engine(), finishedSuccessfully())); } 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 index 6f16db32e5c3..21c125221154 100644 --- 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 @@ -18,7 +18,7 @@ /** * @since 4.12 */ -@Ignore +@Ignore("respected by Vintage engine") public class IgnoredJUnit3TestCase extends TestCase { public void test() {