Fixes Pragmatists#122: Apply filters consistently

src/main/java/junitparams/JUnitParamsRunner.java

@@ -1,28 +1,29 @@
 package junitparams;
 
-import java.util.ArrayList;
 import java.util.List;
 
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.runner.Description;
-import org.junit.runner.manipulation.Filter;
-import org.junit.runner.manipulation.NoTestsRemainException;
 import org.junit.runner.notification.RunNotifier;
 import org.junit.runners.BlockJUnit4ClassRunner;
 import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.InitializationError;
 import org.junit.runners.model.Statement;
 import org.junit.validator.PublicClassValidator;
 
-import static org.junit.internal.runners.rules.RuleMemberValidator.*;
-
-import junitparams.internal.ParameterisedTestClassRunner;
-import junitparams.internal.ParametrizedTestMethodsFilter;
+import junitparams.internal.DescribableFrameworkMethod;
+import junitparams.internal.InstanceFrameworkMethod;
+import junitparams.internal.InvokableFrameworkMethod;
+import junitparams.internal.MethodBlockSupplier;
+import junitparams.internal.NonParameterisedFrameworkMethod;
+import junitparams.internal.ParameterisedFrameworkMethod;
 import junitparams.internal.TestMethod;
 
+import static org.junit.internal.runners.rules.RuleMemberValidator.*;
+
 /**
  * <h1>JUnitParams</h1><br>
  * <p>
@@ -392,19 +393,17 @@
  */
 public class JUnitParamsRunner extends BlockJUnit4ClassRunner {
 
-    private ParametrizedTestMethodsFilter parametrizedTestMethodsFilter = new ParametrizedTestMethodsFilter(this);
-    private ParameterisedTestClassRunner parameterisedRunner;
+    private final MethodBlockSupplier methodBlockSupplier;
     private Description description;
 
     public JUnitParamsRunner(Class<?> klass) throws InitializationError {
         super(klass);
-        parameterisedRunner = new ParameterisedTestClassRunner(getTestClass());
-    }
-
-    @Override
-    public void filter(Filter filter) throws NoTestsRemainException {
-        super.filter(filter);
-        this.parametrizedTestMethodsFilter = new ParametrizedTestMethodsFilter(this, filter);
+        methodBlockSupplier = new MethodBlockSupplier() {
+            @Override
+            public Statement getMethodBlock(InvokableFrameworkMethod method) {
+                return methodBlock(method);
+            }
+        };
     }
 
     @Override
@@ -438,73 +437,67 @@ private void validateLifecycleMethods(List<Throwable> errors) {
 
     @Override
     protected void runChild(FrameworkMethod method, RunNotifier notifier) {
-        if (handleIgnored(method, notifier))
+        DescribableFrameworkMethod describableMethod = getDescribableMethod(method);
+        if (handleIgnored(describableMethod, notifier))
             return;
 
-        TestMethod testMethod = parameterisedRunner.testMethodFor(method);
-        if (parameterisedRunner.shouldRun(testMethod)) {
-            parameterisedRunner.runParameterisedTest(testMethod, methodBlock(method), notifier);
-        } else {
-            verifyMethodCanBeRunByStandardRunner(testMethod);
-            super.runChild(method, notifier);
+        if (describableMethod instanceof InvokableFrameworkMethod) {
+            ((InvokableFrameworkMethod) describableMethod).run(methodBlockSupplier, notifier);
+        }
+        else {
+            throw new IllegalStateException(
+                    "Unsupported FrameworkMethod class: " + method.getClass());
         }
     }
 
-    private void verifyMethodCanBeRunByStandardRunner(TestMethod testMethod) {
-        List<Throwable> errors = new ArrayList<Throwable>();
-        testMethod.frameworkMethod().validatePublicVoidNoArg(false, errors);
-        if (!errors.isEmpty()) {
-            throw new RuntimeException(errors.get(0));
+    /**
+     * Check that the supplied method is one that was originally in the list returned by
+     * {@link #computeTestMethods()}.
+     *
+     * @param method the method, must be an instance of {@link DescribableFrameworkMethod}
+     * @return the supplied method cast to {@link DescribableFrameworkMethod}
+     * @throws IllegalArgumentException if the supplied method is not a
+     *         {@link DescribableFrameworkMethod}
+     */
+    private DescribableFrameworkMethod getDescribableMethod(FrameworkMethod method) {
+        if (!(method instanceof DescribableFrameworkMethod)) {
+            throw new IllegalArgumentException(
+                    "Unsupported FrameworkMethod class: " + method.getClass()
+                            + ", expected a DescribableFrameworkMethod subclass");
         }
+
+        return (DescribableFrameworkMethod) method;
     }
 
-    private boolean handleIgnored(FrameworkMethod method, RunNotifier notifier) {
-        TestMethod testMethod = parameterisedRunner.testMethodFor(method);
-        if (testMethod.isIgnored())
-            notifier.fireTestIgnored(describeMethod(method));
+    private boolean handleIgnored(DescribableFrameworkMethod method, RunNotifier notifier) {
+        // A parameterised method that is ignored (either due to @Ignore or due to empty parameters)
+        // is treated as if it was a non-parameterised method.
+        boolean ignored = (method instanceof NonParameterisedFrameworkMethod)
+                && ((NonParameterisedFrameworkMethod) method).isIgnored();
+        if (ignored)
+            notifier.fireTestIgnored(method.getDescription());
 
-        return testMethod.isIgnored();
+        return ignored;
     }
 
     @Override
     protected List<FrameworkMethod> computeTestMethods() {
-        return parameterisedRunner.computeFrameworkMethods();
+        return TestMethod.listFrom(getTestClass());
     }
 
     @Override
     protected Statement methodInvoker(FrameworkMethod method, Object test) {
-        Statement methodInvoker = parameterisedRunner.parameterisedMethodInvoker(method, test);
-        if (methodInvoker == null)
-            methodInvoker = super.methodInvoker(method, test);
-
-        return methodInvoker;
-    }
-
-    @Override
-    public Description getDescription() {
-        if (description == null) {
-            description = Description.createSuiteDescription(getName(), getTestClass().getAnnotations());
-            List<FrameworkMethod> resultMethods = getListOfMethods();
-
-            for (FrameworkMethod method : resultMethods)
-                description.addChild(describeMethod(method));
+        if (method instanceof InvokableFrameworkMethod) {
+            return ((InvokableFrameworkMethod) method).getInvokeStatement(test);
         }
-
-        return description;
+        throw new IllegalStateException(
+                "Unsupported FrameworkMethod class: " + method.getClass()
+                        + ", expected an InvokableFrameworkMethod subclass");
     }
 
-    private List<FrameworkMethod> getListOfMethods() {
-        List<FrameworkMethod> frameworkMethods = parameterisedRunner.returnListOfMethods();
-        return parametrizedTestMethodsFilter.filteredMethods(frameworkMethods);
-    }
-
-    public Description describeMethod(FrameworkMethod method) {
-        Description child = parameterisedRunner.describeParameterisedMethod(method);
-
-        if (child == null)
-            child = describeChild(method);
-
-        return child;
+    @Override
+    protected Description describeChild(FrameworkMethod method) {
+        return getDescribableMethod(method).getDescription();
     }
 
     /**

src/main/java/junitparams/internal/DeferredErrorFrameworkMethod.java

@@ -0,0 +1,45 @@
+package junitparams.internal;
+
+import java.lang.reflect.Method;
+
+import org.junit.runner.Description;
+import org.junit.runner.notification.RunNotifier;
+import org.junit.runners.model.Statement;
+
+/**
+ * Encapsulates a {@link Throwable} that was caught during initialization so that it can be
+ * thrown during execution in order to preserve previous behavior.
+ */
+public class DeferredErrorFrameworkMethod extends InvokableFrameworkMethod {
+
+    private final Description description;
+    private final Throwable throwable;
+
+    DeferredErrorFrameworkMethod(Method method, Description description, Throwable throwable) {
+        super(method);
+        this.description = description;
+        this.throwable = throwable;
+    }
+
+    @Override
+    public Description getDescription() {
+        return description;
+    }
+
+    @Override
+    public Statement getInvokeStatement(Object test) {
+        return new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                throw throwable;
+            }
+        };
+    }
+
+    @Override
+    public void run(MethodBlockSupplier supplier, RunNotifier notifier) {
+        // Do not call the MethodBlockSupplier as that could introduce additional errors, simply
+        // throw the encapsulated Throwable immediately.
+        runMethodInvoker(notifier, getInvokeStatement(notifier), getDescription());
+    }
+}

src/main/java/junitparams/internal/DescribableFrameworkMethod.java

@@ -0,0 +1,17 @@
+package junitparams.internal;
+
+import java.lang.reflect.Method;
+
+import org.junit.runner.Describable;
+import org.junit.runner.Description;
+import org.junit.runners.model.FrameworkMethod;
+
+/**
+ * A {@link FrameworkMethod} that also provides a {@link Description}.
+ */
+public abstract class DescribableFrameworkMethod extends FrameworkMethod implements Describable {
+
+    DescribableFrameworkMethod(Method method) {
+        super(method);
+    }
+}

src/main/java/junitparams/internal/InstanceFrameworkMethod.java

@@ -0,0 +1,65 @@
+package junitparams.internal;
+
+import java.lang.reflect.Method;
+
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runner.Runner;
+import org.junit.runner.notification.RunListener;
+import org.junit.runner.notification.RunNotifier;
+import org.junit.runners.model.FrameworkMethod;
+import org.junit.runners.model.Statement;
+
+/**
+ * A {@link FrameworkMethod} that represents an instance of an
+ * {@link ParameterisedFrameworkMethod}, that is the combination of the test method with the
+ * parameter set that it will be passed.
+ */
+public class InstanceFrameworkMethod extends InvokableFrameworkMethod {
+
+    private final Description description;
+
+    private final Description instanceDescription;
+
+    private final Object parametersSet;
+
+    /**
+     * Create an {@link InstanceFrameworkMethod}.
+     *
+     * <p>It has two {@link Description} instances because it has to provide different
+     * {@link Description} to {@link TestRule} instances than other usages in order to maintain
+     * backwards compatibility.
+     *
+     * @param method the test method
+     * @param description the description that is supplied to {@link TestRule} instances.
+     * @param instanceDescription the description used for all other purposes, e.g. filtering,
+     *         {@link Runner#getDescription()} and {@link RunListener}.
+     * @param parametersSet the set of parameters to pass to the method.
+     */
+    InstanceFrameworkMethod(Method method, Description description,
+            Description instanceDescription, Object parametersSet) {
+        super(method);
+        this.description = description;
+        this.instanceDescription = instanceDescription;
+        this.parametersSet = parametersSet;
+    }
+
+    @Override
+    public Description getDescription() {
+        return description;
+    }
+
+    Description getInstanceDescription() {
+        return instanceDescription;
+    }
+
+    @Override
+    public Statement getInvokeStatement(Object test) {
+        return new InvokeParameterisedMethod(this, test, parametersSet);
+    }
+
+    @Override
+    public void run(MethodBlockSupplier supplier, RunNotifier notifier) {
+        runMethodInvoker(notifier, supplier.getMethodBlock(this), getInstanceDescription());
+    }
+}

src/main/java/junitparams/internal/InvokableFrameworkMethod.java

@@ -0,0 +1,51 @@
+package junitparams.internal;
+
+import java.lang.reflect.Method;
+import junitparams.JUnitParamsRunner;
+import org.junit.internal.AssumptionViolatedException;
+import org.junit.internal.runners.model.EachTestNotifier;
+import org.junit.runner.Description;
+import org.junit.runner.notification.RunNotifier;
+import org.junit.runners.model.FrameworkMethod;
+import org.junit.runners.model.Statement;
+
+/**
+ * Base for {@link FrameworkMethod} classes that provide a {@link Statement} for invoking.
+ */
+public abstract class InvokableFrameworkMethod extends DescribableFrameworkMethod {
+
+    InvokableFrameworkMethod(Method method) {
+        super(method);
+    }
+
+    /**
+     * Create a {@link Statement} that when called will invoke the method.
+     *
+     * <p>This is usually called from the
+     * {@link JUnitParamsRunner#methodInvoker(FrameworkMethod, Object)} method via the
+     * {@link MethodBlockSupplier} which is usually called from within the
+     * {@link #run(MethodBlockSupplier, RunNotifier)} method.
+     *
+     * @param test
+     *         the object on which the method will be invoked.
+     * @return the {@link Statement}.
+     */
+    public abstract Statement getInvokeStatement(Object test);
+
+    void runMethodInvoker(RunNotifier notifier, Statement methodInvoker,
+            Description methodWithParams) {
+        EachTestNotifier eachNotifier = new EachTestNotifier(notifier, methodWithParams);
+        eachNotifier.fireTestStarted();
+        try {
+            methodInvoker.evaluate();
+        } catch (AssumptionViolatedException e) {
+            eachNotifier.addFailedAssumption(e);
+        } catch (Throwable e) {
+            eachNotifier.addFailure(e);
+        } finally {
+            eachNotifier.fireTestFinished();
+        }
+    }
+
+    public abstract void run(MethodBlockSupplier supplier, RunNotifier notifier);
+}

src/main/java/junitparams/internal/InvokeNonParameterisedMethod.java

@@ -0,0 +1,24 @@
+package junitparams.internal;
+
+import org.junit.runners.model.FrameworkMethod;
+import org.junit.runners.model.Statement;
+
+/**
+ * JUnit invoker for non-parameterised test methods
+ */
+public class InvokeNonParameterisedMethod extends Statement {
+
+    private final FrameworkMethod testMethod;
+    private final Object testClass;
+
+    InvokeNonParameterisedMethod(FrameworkMethod testMethod, Object testClass) {
+        this.testMethod = testMethod;
+        this.testClass = testClass;
+    }
+
+    @Override
+    public void evaluate() throws Throwable {
+        testMethod.invokeExplosively(testClass);
+    }
+
+}