Discover children of ignored Vintage test classes

Prior to this commit, the Vintage test engine only returned a childless
TestDescriptor for test classes annotated with @ignore. However, build
tools like Gradle need to show an accurate number of tests, i.e. they
want to access the test methods of an ignored test class.

The Jupiter engine already discovers skipped containers, e.g. test
classes annotated with @disabled, including their children and
descendants. The Vintage engine now adopts this approach and returns
a full subtree of TestDescriptors for classes annotated with @ignore.
During execution, it will only report the TestDescriptor of the test
class as skipped which is consistent with how the Jupiter engine
reports skipped containers.

Fixes #1277.

Author: marcphilipp

documentation/src/docs/asciidoc/release-notes/release-notes-5.1.0-RC1.adoc

@@ -72,4 +72,13 @@ on GitHub.
 
 ==== New Features and Improvements
 
-* ❓
+* Prior to this release, the Vintage test engine only returned a childless
+  `TestDescriptor` for test classes annotated with `@Ignore`. However, build tools like
+  Gradle need to show an accurate number of tests, i.e. they want to access and count the
+  test methods of a test class regardless whether it's ignored.
+  The Jupiter engine already discovers skipped containers, e.g. test classes annotated
+  with `@Disabled`, including their children and descendants. The Vintage engine now
+  adopts this approach and returns a full subtree of `TestDescriptors` for classes
+  annotated with `@Ignore`. During execution, it will only report the `TestDescriptor`
+  of the test class as skipped which is consistent with how the Jupiter engine reports
+  skipped containers.

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/DefensiveAllDefaultPossibilitiesBuilder.java

@@ -13,13 +13,17 @@
 import java.lang.reflect.Method;
 import java.util.function.Predicate;
 
+import org.junit.Ignore;
 import org.junit.internal.builders.AllDefaultPossibilitiesBuilder;
 import org.junit.internal.builders.AnnotatedBuilder;
+import org.junit.internal.builders.IgnoredBuilder;
+import org.junit.internal.builders.IgnoredClassRunner;
 import org.junit.internal.builders.JUnit4Builder;
 import org.junit.platform.commons.logging.Logger;
 import org.junit.platform.commons.logging.LoggerFactory;
 import org.junit.platform.commons.util.ReflectionUtils;
 import org.junit.runner.Runner;
+import org.junit.runner.manipulation.Filterable;
 import org.junit.runners.model.RunnerBuilder;
 
 /**
@@ -30,18 +34,48 @@
  * @since 4.12
  * @see DefensiveAnnotatedBuilder
  * @see DefensiveJUnit4Builder
+ * @see IgnoredClassRunner
  */
 class DefensiveAllDefaultPossibilitiesBuilder extends AllDefaultPossibilitiesBuilder {
 
 	private static final Logger logger = LoggerFactory.getLogger(DefensiveAllDefaultPossibilitiesBuilder.class);
 
 	private final AnnotatedBuilder annotatedBuilder;
-	private final DefensiveJUnit4Builder defensiveJUnit4Builder;
+	private final JUnit4Builder junit4Builder;
+	private final IgnoredBuilder ignoredBuilder;
 
 	DefensiveAllDefaultPossibilitiesBuilder() {
 		super(true);
 		annotatedBuilder = new DefensiveAnnotatedBuilder(this);
-		defensiveJUnit4Builder = new DefensiveJUnit4Builder();
+		junit4Builder = new DefensiveJUnit4Builder();
+		ignoredBuilder = new NullIgnoredBuilder();
+	}
+
+	@Override
+	public Runner runnerForClass(Class<?> testClass) throws Throwable {
+		Runner runner = super.runnerForClass(testClass);
+		if (testClass.getAnnotation(Ignore.class) != null) {
+			if (runner == null) {
+				return new IgnoredClassRunner(testClass);
+			}
+			return decorateIgnoredTestClass(runner);
+		}
+		return runner;
+	}
+
+	/**
+	 * Instead of checking for the {@link Ignore} annotation and returning an
+	 * {@link IgnoredClassRunner} from {@link IgnoredBuilder}, we've let the
+	 * super class determine the regular runner that would have been used if
+	 * {@link Ignore} hadn't been present. Now, we decorate the runner to
+	 * override its runtime behavior (i.e. skip execution) but return its
+	 * regular {@link org.junit.runner.Description}.
+	 */
+	private Runner decorateIgnoredTestClass(Runner runner) {
+		if (runner instanceof Filterable) {
+			return new FilterableIgnoringRunnerDecorator(runner);
+		}
+		return new IgnoringRunnerDecorator(runner);
 	}
 
 	@Override
@@ -51,7 +85,12 @@ protected AnnotatedBuilder annotatedBuilder() {
 
 	@Override
 	protected JUnit4Builder junit4Builder() {
-		return defensiveJUnit4Builder;
+		return junit4Builder;
+	}
+
+	@Override
+	protected IgnoredBuilder ignoredBuilder() {
+		return ignoredBuilder;
 	}
 
 	/**
@@ -96,4 +135,16 @@ private boolean containsTestMethods(Class<?> testClass) {
 		}
 	}
 
+	/**
+	 * Customization of {@link IgnoredBuilder} that always returns {@code null}.
+	 *
+	 * @since 5.1
+	 */
+	private static class NullIgnoredBuilder extends IgnoredBuilder {
+		@Override
+		public Runner runnerForClass(Class<?> testClass) {
+			// don't ignore entire test classes just yet
+			return null;
+		}
+	}
 }

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/FilterableIgnoringRunnerDecorator.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright 2015-2018 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
+ *
+ * http://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.vintage.engine.discovery;
+
+import org.junit.platform.commons.util.Preconditions;
+import org.junit.runner.Runner;
+import org.junit.runner.manipulation.Filter;
+import org.junit.runner.manipulation.Filterable;
+import org.junit.runner.manipulation.NoTestsRemainException;
+
+/**
+ * {@link Filterable} {@link IgnoringRunnerDecorator}.
+ *
+ * @since 5.1
+ */
+class FilterableIgnoringRunnerDecorator extends IgnoringRunnerDecorator implements Filterable {
+
+	FilterableIgnoringRunnerDecorator(Runner runner) {
+		super(runner);
+		Preconditions.condition(runner instanceof Filterable,
+			() -> "Runner must be an instance of Filterable: " + runner.getClass().getName());
+	}
+
+	@Override
+	public void filter(Filter filter) throws NoTestsRemainException {
+		((Filterable) runner).filter(filter);
+	}
+}

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/IgnoringRunnerDecorator.java

@@ -0,0 +1,48 @@
+/*
+ * Copyright 2015-2018 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
+ *
+ * http://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.vintage.engine.discovery;
+
+import org.junit.platform.commons.util.Preconditions;
+import org.junit.runner.Description;
+import org.junit.runner.Runner;
+import org.junit.runner.notification.RunNotifier;
+
+/**
+ * Decorator for Runners that will be ignored completely.
+ *
+ * <p>Contrary to {@link org.junit.internal.builders.IgnoredClassRunner}, this
+ * runner returns a complete description including all children.
+ *
+ * @since 5.1
+ */
+class IgnoringRunnerDecorator extends Runner implements RunnerDecorator {
+
+	protected final Runner runner;
+
+	IgnoringRunnerDecorator(Runner runner) {
+		this.runner = Preconditions.notNull(runner, "Runner must not be null");
+	}
+
+	@Override
+	public Description getDescription() {
+		return runner.getDescription();
+	}
+
+	@Override
+	public void run(RunNotifier notifier) {
+		notifier.fireTestIgnored(getDescription());
+	}
+
+	@Override
+	public Runner getDecoratedRunner() {
+		return runner;
+	}
+}

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/RunnerDecorator.java

@@ -0,0 +1,19 @@
+/*
+ * Copyright 2015-2018 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
+ *
+ * http://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.vintage.engine.discovery;
+
+import org.junit.runner.Runner;
+
+public interface RunnerDecorator {
+
+	Runner getDecoratedRunner();
+
+}

junit-vintage-engine/src/main/java/org/junit/vintage/engine/discovery/TestClassRequestResolver.java

@@ -69,7 +69,10 @@ private RunnerTestDescriptor determineRunnerTestDescriptor(Class<?> testClass, R
 				runnerTestDescriptor = createCompleteRunnerTestDescriptor(testClass, filteredRunner, engineId);
 			}
 			else {
-				logger.warn(() -> "Runner " + runner.getClass().getName() //
+				Runner runnerToReport = (runner instanceof RunnerDecorator)
+						? ((RunnerDecorator) runner).getDecoratedRunner()
+						: runner;
+				logger.warn(() -> "Runner " + runnerToReport.getClass().getName() //
 						+ " (used on " + testClass.getName() + ") does not support filtering" //
 						+ " and will therefore be run completely.");
 			}

junit-vintage-engine/src/main/java/org/junit/vintage/engine/execution/RunListenerAdapter.java

@@ -44,14 +44,6 @@ class RunListenerAdapter extends RunListener {
 		this.uniqueIdExtractor = new UniqueIdReader().andThen(new UniqueIdStringifier());
 	}
 
-	@Override
-	public void testRunStarted(Description description) {
-		// If it's not a suite it might be skipped entirely later on.
-		if (description.isSuite()) {
-			fireExecutionStarted(testRun.getRunnerTestDescriptor());
-		}
-	}
-
 	@Override
 	public void testIgnored(Description description) {
 		testIgnored(lookupOrRegisterTestDescriptor(description), determineReasonForIgnoredTest(description));