Skip to content
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

Merged
merged 18 commits into from
Apr 21, 2017
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 65 additions & 6 deletions src/main/java/org/junit/runners/Parameterized.java
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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Copy link
Member

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 override collectInitializationErrors(List<Throwable>) and call validatePublicStaticVoidMethods there?

Copy link
Contributor Author

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. using ThreadLocal).

Copy link
Member

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

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);
}
Expand All @@ -276,10 +330,15 @@ private ParametersRunnerFactory getParametersRunnerFactory()
}
}

private Integer parameterCount;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having this set as a side-effect of calling createRunners() is fragile. It would be better to make this final and assign this at construction time. I added a commit that does ths.


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;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reuse org.junit.internal.runners.statements.RunBefores and pass parameters to it?


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);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we reuse org.junit.internal.runners.statements.RunAfters and pass parameters to it?

}

@Override
Expand Down
Loading