Manually add @Ignore annotation to the JUnit38ClassRunner root description

Author: pshevche

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

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());
 	}

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<Annotation> 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);
+	}
 }

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()));
 	}
 

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() {