-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
@BeforeParam/@AfterParam for Parameterized runner #1435
Changes from 8 commits
066e1d6
45064f7
3390335
1de2311
15ac6ad
70245b3
e270f2c
570ec30
754732b
35bbd8f
301a6e0
98e061a
f406970
f7e3ad3
76c1779
03e598f
4958ea8
ab15e7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package org.junit.runners; | ||
|
||
import java.lang.annotation.Annotation; | ||
import java.lang.annotation.ElementType; | ||
import java.lang.annotation.Inherited; | ||
import java.lang.annotation.Retention; | ||
|
@@ -13,6 +14,7 @@ | |
|
||
import org.junit.runner.Runner; | ||
import org.junit.runners.model.FrameworkMethod; | ||
import org.junit.runners.model.InvalidTestClassError; | ||
import org.junit.runners.model.TestClass; | ||
import org.junit.runners.parameterized.BlockJUnit4ClassRunnerWithParametersFactory; | ||
import org.junit.runners.parameterized.ParametersRunnerFactory; | ||
|
@@ -234,23 +236,75 @@ public class Parameterized extends Suite { | |
Class<? extends ParametersRunnerFactory> value() default BlockJUnit4ClassRunnerWithParametersFactory.class; | ||
} | ||
|
||
/** | ||
* Annotation for {@code public static void} methods which should be executed before | ||
* evaluating tests with a particular parameter. | ||
* | ||
* @see org.junit.BeforeClass | ||
* @see org.junit.Before | ||
* @since 4.13 | ||
*/ | ||
@Retention(RetentionPolicy.RUNTIME) | ||
@Target(ElementType.METHOD) | ||
public @interface BeforeParam { | ||
} | ||
|
||
/** | ||
* Annotation for {@code public static void} methods which should be executed after | ||
* evaluating tests with a particular parameter. | ||
* | ||
* @see org.junit.AfterClass | ||
* @see org.junit.After | ||
* @since 4.13 | ||
*/ | ||
@Retention(RetentionPolicy.RUNTIME) | ||
@Target(ElementType.METHOD) | ||
public @interface AfterParam { | ||
} | ||
|
||
/** | ||
* Only called reflectively. Do not use programmatically. | ||
*/ | ||
public Parameterized(Class<?> klass) throws Throwable { | ||
super(klass, RunnersFactory.createRunnersForClass(klass)); | ||
this(klass, new RunnersFactory(klass)); | ||
} | ||
|
||
private Parameterized(Class<?> klass, RunnersFactory runnersFactory) throws Throwable { | ||
super(klass, runnersFactory.createRunners()); | ||
validateBeforeParamAndAfterParamMethods(runnersFactory.parameterCount); | ||
} | ||
|
||
private void validateBeforeParamAndAfterParamMethods(Integer parameterCount) | ||
throws InvalidTestClassError { | ||
List<Throwable> errors = new ArrayList<Throwable>(); | ||
validatePublicStaticVoidMethods(Parameterized.BeforeParam.class, parameterCount, errors); | ||
validatePublicStaticVoidMethods(Parameterized.AfterParam.class, parameterCount, errors); | ||
if (!errors.isEmpty()) { | ||
throw new InvalidTestClassError(getTestClass().getJavaClass(), errors); | ||
} | ||
} | ||
|
||
private void validatePublicStaticVoidMethods( | ||
Class<? extends Annotation> annotation, Integer parameterCount, | ||
List<Throwable> errors) { | ||
List<FrameworkMethod> methods = getTestClass().getAnnotatedMethods(annotation); | ||
for (FrameworkMethod fm : methods) { | ||
fm.validatePublicVoid(true, errors); | ||
if (parameterCount != null) { | ||
final int methodParameterCount = fm.getMethod().getParameterTypes().length; | ||
if (methodParameterCount != 0 && methodParameterCount != parameterCount) { | ||
errors.add(new Exception("Method " + fm.getName() | ||
+ "() should have 0 or " + parameterCount + " parameter(s)")); | ||
} | ||
} | ||
} | ||
} | ||
|
||
private static class RunnersFactory { | ||
private static final ParametersRunnerFactory DEFAULT_FACTORY = new BlockJUnit4ClassRunnerWithParametersFactory(); | ||
|
||
private final TestClass testClass; | ||
|
||
static List<Runner> createRunnersForClass(Class<?> klass) | ||
throws Throwable { | ||
return new RunnersFactory(klass).createRunners(); | ||
} | ||
|
||
private RunnersFactory(Class<?> klass) { | ||
testClass = new TestClass(klass); | ||
} | ||
|
@@ -276,10 +330,15 @@ private ParametersRunnerFactory getParametersRunnerFactory() | |
} | ||
} | ||
|
||
private Integer parameterCount; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having this set as a side-effect of calling |
||
|
||
private TestWithParameters createTestWithNotNormalizedParameters( | ||
String pattern, int index, Object parametersOrSingleParameter) { | ||
Object[] parameters = (parametersOrSingleParameter instanceof Object[]) ? (Object[]) parametersOrSingleParameter | ||
: new Object[] { parametersOrSingleParameter }; | ||
if (parameterCount == null) { | ||
parameterCount = parameters.length; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a side effect we could have a check that number of parameters is always the same. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the number of parameters were different for some parameter sets, you would get a failure when the child runner is run. |
||
return createTestWithParameters(testClass, pattern, index, | ||
parameters); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,15 +2,18 @@ | |
|
||
import java.lang.annotation.Annotation; | ||
import java.lang.reflect.Field; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import org.junit.runner.RunWith; | ||
import org.junit.runner.notification.RunNotifier; | ||
import org.junit.runners.BlockJUnit4ClassRunner; | ||
import org.junit.runners.Parameterized; | ||
import org.junit.runners.Parameterized.Parameter; | ||
import org.junit.runners.model.FrameworkField; | ||
import org.junit.runners.model.FrameworkMethod; | ||
import org.junit.runners.model.InitializationError; | ||
import org.junit.runners.model.MultipleFailureException; | ||
import org.junit.runners.model.Statement; | ||
|
||
/** | ||
|
@@ -135,7 +138,73 @@ protected void validateFields(List<Throwable> errors) { | |
|
||
@Override | ||
protected Statement classBlock(RunNotifier notifier) { | ||
return childrenInvoker(notifier); | ||
Statement statement = childrenInvoker(notifier); | ||
statement = withBeforeParams(statement); | ||
statement = withAfterParams(statement); | ||
return statement; | ||
} | ||
|
||
private Statement withBeforeParams(Statement statement) { | ||
List<FrameworkMethod> befores = getTestClass() | ||
.getAnnotatedMethods(Parameterized.BeforeParam.class); | ||
return befores.isEmpty() ? statement : new RunBeforeParams(statement, befores); | ||
} | ||
|
||
private class RunBeforeParams extends Statement { | ||
private final Statement next; | ||
private final List<FrameworkMethod> befores; | ||
|
||
RunBeforeParams(Statement next, List<FrameworkMethod> befores) { | ||
this.next = next; | ||
this.befores = befores; | ||
} | ||
|
||
@Override | ||
public void evaluate() throws Throwable { | ||
for (FrameworkMethod before : befores) { | ||
int paramCount = before.getMethod().getParameterTypes().length; | ||
before.invokeExplosively( | ||
null, paramCount == 0 ? (Object[]) null : parameters); | ||
} | ||
next.evaluate(); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we reuse |
||
|
||
private Statement withAfterParams(Statement statement) { | ||
List<FrameworkMethod> afters = getTestClass() | ||
.getAnnotatedMethods(Parameterized.AfterParam.class); | ||
return afters.isEmpty() ? statement : new RunAfterParams(statement, afters); | ||
} | ||
|
||
private class RunAfterParams extends Statement { | ||
private final Statement next; | ||
private final List<FrameworkMethod> afters; | ||
|
||
RunAfterParams(Statement next, List<FrameworkMethod> afters) { | ||
this.next = next; | ||
this.afters = afters; | ||
} | ||
|
||
@Override | ||
public void evaluate() throws Throwable { | ||
List<Throwable> errors = new ArrayList<Throwable>(); | ||
try { | ||
next.evaluate(); | ||
} catch (Throwable e) { | ||
errors.add(e); | ||
} finally { | ||
for (FrameworkMethod each : afters) { | ||
try { | ||
int paramCount = each.getMethod().getParameterTypes().length; | ||
each.invokeExplosively( | ||
null, paramCount == 0 ? (Object[]) null : parameters); | ||
} catch (Throwable e) { | ||
errors.add(e); | ||
} | ||
} | ||
} | ||
MultipleFailureException.assertEmpty(errors); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we reuse |
||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we delete
validateBeforeParamAndAfterParamMethods()
and instead overridecollectInitializationErrors(List<Throwable>)
and callvalidatePublicStaticVoidMethods
there?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried that before,
collectInitializationErrors()
is called from the super class constructor and the number of method parameters can't be easily validated (only with some tricks, e.g. usingThreadLocal
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. I've never been a fan of calling non-static methods in constructors. It's caused no end of problems in JUnit4. Hopefully they avoided that in the JUnit5 code base