Skip to content

Commit

Permalink
Merge pull request testng-team#923 from nitinverma/feat-data-providers
Browse files Browse the repository at this point in the history
Fixes: Array in dataProvider not matching Varargs parameter testng-team#122 testng-team#920
  • Loading branch information
cbeust committed Jan 4, 2016
2 parents b79364e + f3e44d6 commit 1262de6
Show file tree
Hide file tree
Showing 21 changed files with 1,894 additions and 44 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,5 @@ test-output-tests
testng.iml
z_build
.kobalt
.DS_Store

1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Current
Fixed: GITHUB-923 Refactored data provider's parameter values passing to a varargs or non-varargs method with @NoInjection handling (Nitin Verma)
New: GITHUB-933: Deprecate XmlTest#getTestParameters (Julien Herr)
Fixed: GITHUB-911: TestListener#onTestStart should be invoked if a suite configuration method fails (Harmin Parra Rueda & Julien Herr)
Fixed: GITHUB-793: Test suite with tests using dependency and priority has wrong behavior (Martin Hereu & Julien Herr)
Expand Down
49 changes: 8 additions & 41 deletions src/main/java/org/testng/internal/Invoker.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.testng.internal;

import java.lang.annotation.Annotation;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Collection;
Expand Down Expand Up @@ -30,7 +29,6 @@
import org.testng.TestException;
import org.testng.TestNGException;
import org.testng.annotations.IConfigurationAnnotation;
import org.testng.annotations.NoInjection;
import org.testng.collections.Lists;
import org.testng.collections.Maps;
import org.testng.collections.Sets;
Expand All @@ -40,6 +38,9 @@
import org.testng.internal.annotations.IAnnotationFinder;
import org.testng.internal.invokers.InvokedMethodListenerInvoker;
import org.testng.internal.invokers.InvokedMethodListenerMethod;
import org.testng.internal.reflect.DataProviderMethodMatcher;
import org.testng.internal.reflect.MethodMatcher;
import org.testng.internal.reflect.MethodMatcherContext;
import org.testng.internal.thread.ThreadExecutionException;
import org.testng.internal.thread.ThreadUtil;
import org.testng.internal.thread.graph.IWorker;
Expand Down Expand Up @@ -1212,6 +1213,7 @@ private ITestResult registerSkippedTestResult(ITestNGMethod testMethod, Object i
* Gets an array of parameter values returned by data provider or the ones that
* are injected based on parameter type. The method also checks for {@code NoInjection}
* annotation
*
* @param parameterValues parameter values from a data provider
* @param method method to be invoked
* @param context test context
Expand All @@ -1220,45 +1222,10 @@ private ITestResult registerSkippedTestResult(ITestNGMethod testMethod, Object i
private Object[] injectParameters(Object[] parameterValues, Method method,
ITestContext context, ITestResult testResult)
throws TestNGException {
List<Object> vResult = Lists.newArrayList();
int i = 0;
int numValues = parameterValues.length;
int numParams = method.getParameterTypes().length;

if (numValues > numParams && ! method.isVarArgs()) {
throw new TestNGException("The data provider is trying to pass " + numValues
+ " parameters but the method "
+ method.getDeclaringClass().getName() + "#" + method.getName()
+ " takes " + numParams);
}

// beyond this, numValues <= numParams
for (Class<?> cls : method.getParameterTypes()) {
Annotation[] annotations = method.getParameterAnnotations()[i];
boolean noInjection = false;
for (Annotation a : annotations) {
if (a instanceof NoInjection) {
noInjection = true;
break;
}
}
Object injected = Parameters.getInjectedParameter(cls, method, context, testResult);
if (injected != null && ! noInjection) {
vResult.add(injected);
} else {
try {
if (method.isVarArgs()) vResult.add(parameterValues);
else vResult.add(parameterValues[i++]);
} catch (ArrayIndexOutOfBoundsException ex) {
throw new TestNGException("The data provider is trying to pass " + numValues
+ " parameters but the method "
+ method.getDeclaringClass().getName() + "#" + method.getName()
+ " takes " + numParams
+ " and TestNG is unable in inject a suitable object", ex);
}
}
}
return vResult.toArray(new Object[vResult.size()]);
final MethodMatcher matcher = new DataProviderMethodMatcher(
new MethodMatcherContext(method, parameterValues, context, testResult)
);
return matcher.getConformingArguments();
}

private ParameterBag handleParameters(ITestNGMethod testMethod,
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/org/testng/internal/ThreeStateBoolean.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.testng.internal;

/**
* For boolean use cases were 'non-existence' can not be defaulted to either true or false.
*
* @author <a href="mailto:[email protected]">Nitin Verma</a>
*/
public enum ThreeStateBoolean {
NONE, TRUE, FALSE;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package org.testng.internal.reflect;

import org.testng.internal.ThreeStateBoolean;

import static org.testng.internal.ThreeStateBoolean.FALSE;
import static org.testng.internal.ThreeStateBoolean.TRUE;

/**
* Created on 1/4/16.
*
* @author <a href="mailto:[email protected]">Nitin Verma</a>
*/
public abstract class AbstractMethodMatcher implements MethodMatcher {
private final MethodMatcherContext context;
private ThreeStateBoolean conforms = ThreeStateBoolean.NONE;

public AbstractMethodMatcher(final MethodMatcherContext context) {
this.context = context;
}

protected MethodMatcherContext getContext() {
return context;
}

protected ThreeStateBoolean getConforms() {
return conforms;
}

/**
* {@inheritDoc}
*/
@Override
public boolean conforms() {
boolean hasConformance = false;
try {
hasConformance = hasConformance();
} finally {
conforms = hasConformance ? TRUE : FALSE;
}
return hasConformance;
}

/**
* Checks if the arguments conform to the method.
*
* @return conformance
* @throws MethodMatcherException if any internal failure.
*/
protected abstract boolean hasConformance();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package org.testng.internal.reflect;

import org.testng.internal.ThreeStateBoolean;

import java.util.List;
import java.util.Set;

/**
* Created on 1/4/16.
*
* @author <a href="mailto:[email protected]">Nitin Verma</a>
*/
public abstract class AbstractNodeMethodMatcher extends AbstractMethodMatcher {

private Parameter[] conformingParameters = null;
private Set<InjectableParameter> conformingInjects = null;

public AbstractNodeMethodMatcher(final MethodMatcherContext context) {
super(context);
}

protected Parameter[] getConformingParameters() {
return conformingParameters;
}

protected Set<InjectableParameter> getConformingInjects() {
return conformingInjects;
}

/**
* {@inheritDoc}
*/
@Override
protected boolean hasConformance() {
boolean matching = false;
for (final Set<InjectableParameter> injects : getConformanceInjectsOrder()) {
final Parameter[] parameters = ReflectionRecipes.filter(getContext().getMethodParameter(), injects);
matching = match(parameters, getContext().getArguments());
if (matching) {
conformingParameters = parameters;
conformingInjects = injects;
break;
}
}
return matching;
}

/**
* @return injects to check against.
*/
protected abstract List<Set<InjectableParameter>> getConformanceInjectsOrder();

/**
* Checks if its possible to gives an array consumable by java method invoker.
*
* @param parameters array of parameter instances under question.
* @param arguments instances to be verified.
* @return matches or not
*/
protected abstract boolean match(final Parameter[] parameters, final Object[] arguments);

/**
* {@inheritDoc}
*/
@Override
public Object[] getConformingArguments() throws MethodMatcherException {
if (ThreeStateBoolean.NONE.equals(getConforms())) {
conforms();
}
if (getConformingParameters() == null) {
throw new MethodMatcherException(this.getClass().getSimpleName() + " mismatch", getContext().getMethod(),
getContext().getArguments());
}

return ReflectionRecipes.inject(
getContext().getMethodParameter(),
InjectableParameter.Assistant.ALL_INJECTS,
matchingArguments(getConformingParameters(), getContext().getArguments()),
getContext().getMethod(),
getContext().getTestContext(),
getContext().getTestResult()
);
}

/**
* If possible gives an array consumable by java method invoker.
*
* @param parameters array of parameter instances under question.
* @param arguments instances to conform.
* @return conforming argument array
*/
protected abstract Object[] matchingArguments(final Parameter[] parameters, final Object[] arguments);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package org.testng.internal.reflect;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;

import static org.testng.internal.reflect.InjectableParameter.Assistant.ALL_INJECTS;

/**
* Checks for array ending method argument match with or without filtering injectables.
*
* @author <a href="mailto:[email protected]">Nitin Verma</a>
* @see ReflectionRecipes#matchArrayEnding(Class[], Object[])
*/
public class ArrayEndingMethodMatcher extends AbstractNodeMethodMatcher {

public ArrayEndingMethodMatcher(final MethodMatcherContext context) {
super(context);
}

/**
* {@inheritDoc}
*/
@Override
protected List<Set<InjectableParameter>> getConformanceInjectsOrder() {
final List<Set<InjectableParameter>> injectsOrder = new ArrayList<>(1);
injectsOrder.add(ALL_INJECTS);
return injectsOrder;
}

/**
* {@inheritDoc}
*
* @see ReflectionRecipes#matchArrayEnding(Class[], Object[])
*/
@Override
protected boolean match(final Parameter[] parameters, final Object[] arguments) {
return ReflectionRecipes.matchArrayEnding(parameters, getContext().getArguments());
}

/**
* {@inheritDoc}
*/
@Override
protected Object[] matchingArguments(final Parameter[] parameters, final Object[] arguments) {
final Class<?>[] classes = ReflectionRecipes.classesFromParameters(parameters);
final Object[] objects = new Object[classes.length];
final Class<?> componentType = classes[classes.length - 1].getComponentType();
final Object array = Array.newInstance(componentType, arguments.length - classes.length + 1);
System.arraycopy(arguments, 0, objects, 0, classes.length - 1);
int j = 0;
for (int i = classes.length - 1; i < arguments.length; i++, j++) {
Array.set(array, j, arguments[i]);
}
objects[classes.length - 1] = array;
return objects;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.testng.internal.reflect;

import org.testng.internal.ThreeStateBoolean;

import static org.testng.internal.ThreeStateBoolean.FALSE;
import static org.testng.internal.ThreeStateBoolean.TRUE;

/**
* Checks the conformance as per data-provide specifications.
*
* @author <a href="mailto:[email protected]">Nitin Verma</a>
*/
public class DataProviderMethodMatcher extends AbstractMethodMatcher {

private final DirectMethodMatcher directMethodMatcher;
private final ArrayEndingMethodMatcher arrayEndingMethodMatcher;
private MethodMatcher matchingMatcher = null;

public DataProviderMethodMatcher(final MethodMatcherContext context) {
super(context);
this.directMethodMatcher = new DirectMethodMatcher(context);
this.arrayEndingMethodMatcher = new ArrayEndingMethodMatcher(context);
}

/**
* {@inheritDoc}
*/
@Override
protected boolean hasConformance() {
boolean matching = false;
if (directMethodMatcher.conforms()) {
matching = true;
matchingMatcher = directMethodMatcher;
} else if (arrayEndingMethodMatcher.conforms()) {
matching = true;
matchingMatcher = arrayEndingMethodMatcher;
}
return matching;
}

/**
* {@inheritDoc}
*/
@Override
public Object[] getConformingArguments() throws MethodMatcherException {
if (ThreeStateBoolean.NONE.equals(getConforms())) {
conforms();
}
if (matchingMatcher != null) {
return matchingMatcher.getConformingArguments();
}
throw new MethodMatcherException("Data provider mismatch", getContext().getMethod(), getContext().getArguments());
}
}
Loading

0 comments on commit 1262de6

Please sign in to comment.