From 9a709285382c7939fc693476c7d68e69297354ec Mon Sep 17 00:00:00 2001 From: James Phillpotts Date: Wed, 11 Nov 2015 14:11:39 +0000 Subject: [PATCH 1/2] Cartesian product for data providers --- .../java/org/testng/annotations/Factory.java | 2 +- .../testng/annotations/ITestAnnotation.java | 4 +- .../java/org/testng/annotations/Test.java | 2 +- .../java/org/testng/internal/Invoker.java | 11 +- .../org/testng/internal/ParameterHolder.java | 7 +- .../java/org/testng/internal/Parameters.java | 114 ++++++++++++------ .../annotations/FactoryAnnotation.java | 6 +- .../internal/annotations/IDataProvidable.java | 4 +- .../internal/annotations/TestAnnotation.java | 6 +- .../reporters/XMLSuiteResultWriter.java | 7 +- .../FactoryTransformer.java | 2 +- .../dataprovider/CartesianProductTest.java | 71 +++++++++++ .../mannotation/MAnnotationSampleTest.java | 8 +- 13 files changed, 183 insertions(+), 61 deletions(-) create mode 100644 src/test/java/test/dataprovider/CartesianProductTest.java diff --git a/src/main/java/org/testng/annotations/Factory.java b/src/main/java/org/testng/annotations/Factory.java index 2f97cb1b22..92586d3dd7 100755 --- a/src/main/java/org/testng/annotations/Factory.java +++ b/src/main/java/org/testng/annotations/Factory.java @@ -26,7 +26,7 @@ * The name of the data provider for this test method. * @see org.testng.annotations.DataProvider */ - public String dataProvider() default ""; + public String[] dataProvider() default {}; /** * The class where to look for the data provider. If not diff --git a/src/main/java/org/testng/annotations/ITestAnnotation.java b/src/main/java/org/testng/annotations/ITestAnnotation.java index 2c63493abc..cddc9afcc9 100755 --- a/src/main/java/org/testng/annotations/ITestAnnotation.java +++ b/src/main/java/org/testng/annotations/ITestAnnotation.java @@ -57,8 +57,8 @@ public interface ITestAnnotation extends ITestOrConfiguration, IDataProvidable { public boolean getSingleThreaded(); public void setSingleThreaded(boolean f); - public String getDataProvider(); - public void setDataProvider(String v); + public String[] getDataProvider(); + public void setDataProvider(String[] v); public Class getDataProviderClass(); public void setDataProviderClass(Class v); diff --git a/src/main/java/org/testng/annotations/Test.java b/src/main/java/org/testng/annotations/Test.java index 8c7f812a76..e381b18382 100755 --- a/src/main/java/org/testng/annotations/Test.java +++ b/src/main/java/org/testng/annotations/Test.java @@ -91,7 +91,7 @@ * The name of the data provider for this test method. * @see org.testng.annotations.DataProvider */ - public String dataProvider() default ""; + public String[] dataProvider() default {}; /** * The class where to look for the data provider. If not diff --git a/src/main/java/org/testng/internal/Invoker.java b/src/main/java/org/testng/internal/Invoker.java index 94073b790f..c76d9af91e 100644 --- a/src/main/java/org/testng/internal/Invoker.java +++ b/src/main/java/org/testng/internal/Invoker.java @@ -1095,7 +1095,7 @@ public List invokeTestMethods(ITestNGMethod testMethod, List workers = Lists.newArrayList(); if (bag.parameterHolder.origin == ParameterOrigin.ORIGIN_DATA_PROVIDER && - bag.parameterHolder.dataProviderHolder.annotation.isParallel()) { + hasParallelDataProvider(bag.parameterHolder)) { while (allParameterValues.hasNext()) { Object[] parameterValues = injectParameters(allParameterValues.next(), testMethod.getMethod(), testContext, null /* test result */); @@ -1190,6 +1190,15 @@ public List invokeTestMethods(ITestNGMethod testMethod, } // invokeTestMethod + private boolean hasParallelDataProvider(ParameterHolder parameterHolder) { + for (DataProviderHolder holder : parameterHolder.dataProviderHolders) { + if (holder.annotation.isParallel()) { + return true; + } + } + return false; + } + private ITestResult registerSkippedTestResult(ITestNGMethod testMethod, Object instance, long start, Throwable throwable) { ITestResult result = diff --git a/src/main/java/org/testng/internal/ParameterHolder.java b/src/main/java/org/testng/internal/ParameterHolder.java index 8bd091404f..0bdb87d831 100755 --- a/src/main/java/org/testng/internal/ParameterHolder.java +++ b/src/main/java/org/testng/internal/ParameterHolder.java @@ -1,6 +1,7 @@ package org.testng.internal; import java.util.Iterator; +import java.util.List; /** * A simple holder for parameters that contains the parameters and where these came from @@ -17,15 +18,15 @@ public enum ParameterOrigin { ORIGIN_XML // TestNG XML suite }; - public DataProviderHolder dataProviderHolder; + public List dataProviderHolders; public Iterator parameters; public ParameterOrigin origin; - public ParameterHolder(Iterator parameters, ParameterOrigin origin, DataProviderHolder dph) { + public ParameterHolder(Iterator parameters, ParameterOrigin origin, List dphs) { super(); this.parameters = parameters; this.origin = origin; - this.dataProviderHolder = dph; + this.dataProviderHolders = dphs; } } diff --git a/src/main/java/org/testng/internal/Parameters.java b/src/main/java/org/testng/internal/Parameters.java index cd86eb841f..afa22c99cc 100755 --- a/src/main/java/org/testng/internal/Parameters.java +++ b/src/main/java/org/testng/internal/Parameters.java @@ -5,6 +5,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; @@ -245,24 +246,18 @@ else if (type.isEnum()) { return result; } - private static DataProviderHolder findDataProvider(Object instance, ITestClass clazz, + private static List findDataProvider(Object instance, ITestClass clazz, ConstructorOrMethod m, IAnnotationFinder finder, ITestContext context) { - DataProviderHolder result = null; + List result = null; IDataProvidable dp = findDataProviderInfo(clazz, m, finder); if (dp != null) { - String dataProviderName = dp.getDataProvider(); + String[] dataProviderNames = dp.getDataProvider(); Class dataProviderClass = dp.getDataProviderClass(); - if (! Utils.isStringEmpty(dataProviderName)) { - result = findDataProvider(instance, clazz, finder, dataProviderName, dataProviderClass, context); - - if(null == result) { - throw new TestNGException("Method " + m + " requires a @DataProvider named : " - + dataProviderName + (dataProviderClass != null ? " in class " + dataProviderClass.getName() : "") - ); - } + if (dataProviderNames.length > 0) { + result = findDataProvider(m, instance, clazz, finder, dataProviderNames, dataProviderClass, context); } } @@ -306,10 +301,8 @@ private static IDataProvidable findDataProviderInfo(ITestClass clazz, Constructo /** * Find a method that has a @DataProvider(name=name) */ - private static DataProviderHolder findDataProvider(Object instance, ITestClass clazz, - IAnnotationFinder finder, - String name, Class dataProviderClass, - ITestContext context) + private static List findDataProvider(ConstructorOrMethod annotated, Object instance, + ITestClass clazz, IAnnotationFinder finder, String[] names, Class dataProviderClass, ITestContext context) { DataProviderHolder result = null; @@ -320,24 +313,33 @@ private static DataProviderHolder findDataProvider(Object instance, ITestClass c shouldBeStatic = true; } - for (Method m : ClassHelper.getAvailableMethods(cls)) { - IDataProviderAnnotation dp = finder.findAnnotation(m, IDataProviderAnnotation.class); - if (null != dp && name.equals(getDataProviderName(dp, m))) { - if (shouldBeStatic && (m.getModifiers() & Modifier.STATIC) == 0) { - Injector injector = context.getInjector(clazz); - if (injector != null) { - instance = injector.getInstance(dataProviderClass); + List providers = new ArrayList<>(); + for (String name : names) { + for (Method m : ClassHelper.getAvailableMethods(cls)) { + IDataProviderAnnotation dp = finder.findAnnotation(m, IDataProviderAnnotation.class); + if (null != dp && name.equals(getDataProviderName(dp, m))) { + if (shouldBeStatic && (m.getModifiers() & Modifier.STATIC) == 0) { + Injector injector = context.getInjector(clazz); + if (injector != null) { + instance = injector.getInstance(dataProviderClass); + } } - } - if (result != null) { - throw new TestNGException("Found two providers called '" + name + "' on " + cls); + if (result != null) { + throw new TestNGException("Found two providers called '" + name + "' on " + cls); + } + result = new DataProviderHolder(dp, m, instance); } - result = new DataProviderHolder(dp, m, instance); } + if(null == result) { + throw new TestNGException("Method " + annotated + " requires a @DataProvider named : " + + name + (dataProviderClass != null ? " in class " + dataProviderClass.getName() : "") + ); + } + providers.add(result); } - return result; + return providers; } private static String getDataProviderName(IDataProviderAnnotation dp, Method m) { @@ -415,11 +417,11 @@ public static ParameterHolder handleParameters(ITestNGMethod testMethod, * Do we have a @DataProvider? If yes, then we have several * sets of parameters for this method */ - DataProviderHolder dataProviderHolder = + List dataProviderHolders = findDataProvider(instance, testMethod.getTestClass(), testMethod.getConstructorOrMethod(), annotationFinder, methodParams.context); - if (null != dataProviderHolder) { + if (null != dataProviderHolders && !dataProviderHolders.isEmpty()) { int parameterCount = testMethod.getConstructorOrMethod().getParameterTypes().length; for (int i = 0; i < parameterCount; i++) { @@ -427,19 +429,23 @@ public static ParameterHolder handleParameters(ITestNGMethod testMethod, allParameterNames.put(n, n); } - parameters = MethodInvocationHelper.invokeDataProvider( - dataProviderHolder.instance, /* a test instance or null if the dataprovider is static*/ - dataProviderHolder.method, - testMethod, - methodParams.context, - fedInstance, - annotationFinder); + List> cartesianParameters = new ArrayList<>(); + for (DataProviderHolder dataProviderHolder : dataProviderHolders) { + cartesianParameters.add(MethodInvocationHelper.invokeDataProvider( + dataProviderHolder.instance, /* a test instance or null if the dataprovider is static*/ + dataProviderHolder.method, + testMethod, + methodParams.context, + fedInstance, + annotationFinder)); + } + parameters = cartesianProduct(cartesianParameters); Iterator filteredParameters = filterParameters(parameters, testMethod.getInvocationNumbers()); result = new ParameterHolder(filteredParameters, ParameterOrigin.ORIGIN_DATA_PROVIDER, - dataProviderHolder); + dataProviderHolders); } else { // @@ -464,6 +470,40 @@ public static ParameterHolder handleParameters(ITestNGMethod testMethod, return result; } + private static Iterator cartesianProduct(List> cartesianParameters) { + if (cartesianParameters.size() == 0) { + throw new IllegalArgumentException("Must provide more than one collection of data"); + } else if (cartesianParameters.size() == 1) { + return cartesianParameters.get(0); + } + + List result = new ArrayList<>(); + + Iterator left = cartesianParameters.get(0); + Iterator right = cartesianParameters.get(1); + while (left.hasNext()) { + Object[] leftRow = left.next(); + while (right.hasNext()) { + Object[] rightRow = right.next(); + Object[] row = new Object[leftRow.length + rightRow.length]; + System.arraycopy(leftRow, 0, row, 0, leftRow.length); + System.arraycopy(rightRow, 0, row, leftRow.length, rightRow.length); + result.add(row); + } + } + + if (cartesianParameters.size()> 2) { + List> nextProduct = new ArrayList<>(); + nextProduct.add(result.iterator()); + for (int i = 2; i < cartesianParameters.size(); i++) { + nextProduct.add(cartesianParameters.get(i)); + } + return cartesianProduct(nextProduct); + } + + return result.iterator(); + } + /** * If numbers is empty, return parameters, otherwise, return a subset of parameters * whose ordinal number match these found in numbers. diff --git a/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java b/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java index f82467f5d3..88c319818d 100755 --- a/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java +++ b/src/main/java/org/testng/internal/annotations/FactoryAnnotation.java @@ -13,17 +13,17 @@ public class FactoryAnnotation implements IFactoryAnnotation { private String[] m_parameters = {}; - private String m_dataProvider = null; + private String[] m_dataProvider = null; private Class m_dataProviderClass; private boolean m_enabled = true; @Override - public String getDataProvider() { + public String[] getDataProvider() { return m_dataProvider; } @Override - public void setDataProvider(String dataProvider) { + public void setDataProvider(String[] dataProvider) { m_dataProvider = dataProvider; } diff --git a/src/main/java/org/testng/internal/annotations/IDataProvidable.java b/src/main/java/org/testng/internal/annotations/IDataProvidable.java index 3570e3fc0f..82049dc78a 100644 --- a/src/main/java/org/testng/internal/annotations/IDataProvidable.java +++ b/src/main/java/org/testng/internal/annotations/IDataProvidable.java @@ -6,8 +6,8 @@ * @author Cedric Beust */ public interface IDataProvidable { - public String getDataProvider(); - public void setDataProvider(String v); + public String[] getDataProvider(); + public void setDataProvider(String[] v); public Class getDataProviderClass(); public void setDataProviderClass(Class v); diff --git a/src/main/java/org/testng/internal/annotations/TestAnnotation.java b/src/main/java/org/testng/internal/annotations/TestAnnotation.java index 19612a94f4..d1222ef165 100755 --- a/src/main/java/org/testng/internal/annotations/TestAnnotation.java +++ b/src/main/java/org/testng/internal/annotations/TestAnnotation.java @@ -15,7 +15,7 @@ public class TestAnnotation extends TestOrConfiguration implements ITestAnnotati private int m_invocationCount = 1; private int m_threadPoolSize = 0; private int m_successPercentage = 100; - private String m_dataProvider = ""; + private String[] m_dataProvider = {}; private boolean m_alwaysRun = false; private Class[] m_expectedExceptions = {}; private String m_expectedExceptionsMessageRegExp = ".*"; @@ -61,7 +61,7 @@ public void setAlwaysRun(boolean alwaysRun) { } @Override - public void setDataProvider(String dataProvider) { + public void setDataProvider(String[] dataProvider) { m_dataProvider = dataProvider; } @@ -107,7 +107,7 @@ public int getSuccessPercentage() { } @Override - public String getDataProvider() { + public String[] getDataProvider() { return m_dataProvider; } diff --git a/src/main/java/org/testng/reporters/XMLSuiteResultWriter.java b/src/main/java/org/testng/reporters/XMLSuiteResultWriter.java index c8ebebdd5a..ebee846111 100755 --- a/src/main/java/org/testng/reporters/XMLSuiteResultWriter.java +++ b/src/main/java/org/testng/reporters/XMLSuiteResultWriter.java @@ -16,6 +16,7 @@ import java.io.File; import java.text.SimpleDateFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -226,9 +227,9 @@ private Properties getTestResultAttributes(ITestResult testResult) { if (cm.getMethod() != null) { testAnnotation = cm.getMethod().getAnnotation(Test.class); if (testAnnotation != null) { - String dataProvider = testAnnotation.dataProvider(); - if (!Strings.isNullOrEmpty(dataProvider)) { - attributes.setProperty(XMLReporterConfig.ATTR_DATA_PROVIDER, dataProvider); + String[] dataProvider = testAnnotation.dataProvider(); + if (dataProvider.length > 0) { + attributes.setProperty(XMLReporterConfig.ATTR_DATA_PROVIDER, Arrays.toString(dataProvider)); } } } diff --git a/src/test/java/test/annotationtransformer/FactoryTransformer.java b/src/test/java/test/annotationtransformer/FactoryTransformer.java index fb2ae1793f..352ccbaf31 100644 --- a/src/test/java/test/annotationtransformer/FactoryTransformer.java +++ b/src/test/java/test/annotationtransformer/FactoryTransformer.java @@ -29,6 +29,6 @@ public void transform(ITestAnnotation annotation, Class testClass, @Override public void transform(IFactoryAnnotation annotation, Method testMethod) { - annotation.setDataProvider("dataProvider"); + annotation.setDataProvider(new String[] {"dataProvider"}); } } diff --git a/src/test/java/test/dataprovider/CartesianProductTest.java b/src/test/java/test/dataprovider/CartesianProductTest.java new file mode 100644 index 0000000000..d72b8dbf02 --- /dev/null +++ b/src/test/java/test/dataprovider/CartesianProductTest.java @@ -0,0 +1,71 @@ +package test.dataprovider; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class CartesianProductTest { + + @DataProvider + public Object[][] data() { + return new Object[][] { + new Object[] { "one" }, + new Object[] { "two" }, + new Object[] { "three" } + }; + } + + @DataProvider(indices = { 0, 2 }) + public Object[][] filteredData() { + return new Object[][] { + new Object[] { 1 }, + new Object[] { 2 }, + new Object[] { 3 } + }; + } + + @DataProvider(name = "iterator") + public Iterator iteratorData() { + return new ArrayList() {{ + add(new Object[] { "a" }); + add(new Object[] { "b" }); + add(new Object[] { "c" }); + }}.iterator(); + } + + private Set cartesianProducts = new HashSet<>(); + @Test(dataProvider = { "data", "filteredData", "iterator" }) + public void testOrder(String data, int filtered, String iterator) { + cartesianProducts.add(String.format("%s:%d:%s", data, filtered, iterator)); + } + + @Test(dependsOnMethods = "testOrder") + public void verifyOrder() { + assertThat(cartesianProducts).containsOnly( + "one:1:a", + "one:3:a", + "one:1:b", + "one:3:b", + "one:1:c", + "one:3:c", + "two:1:a", + "two:3:a", + "two:1:b", + "two:3:b", + "two:1:c", + "two:3:c", + "three:1:a", + "three:3:a", + "three:1:b", + "three:3:b", + "three:1:c", + "three:3:c" + ); + } +} diff --git a/src/test/java/test/mannotation/MAnnotationSampleTest.java b/src/test/java/test/mannotation/MAnnotationSampleTest.java index b0fecd6e8a..689a801a5c 100644 --- a/src/test/java/test/mannotation/MAnnotationSampleTest.java +++ b/src/test/java/test/mannotation/MAnnotationSampleTest.java @@ -42,7 +42,7 @@ public void verifyTestClassLevel() { Assert.assertEquals(test1.getInvocationCount(), 43); Assert.assertEquals(test1.getSuccessPercentage(), 44); Assert.assertEquals(test1.getThreadPoolSize(), 3); - Assert.assertEquals(test1.getDataProvider(), "dp"); + Assert.assertEquals(test1.getDataProvider(), new String[] {"dp"}); Assert.assertEquals(test1.getDescription(), "Class level description"); // @@ -55,7 +55,7 @@ public void verifyTestClassLevel() { Assert.assertEquals(test2.getTimeOut(), 0); Assert.assertEquals(test2.getInvocationCount(), 1); Assert.assertEquals(test2.getSuccessPercentage(), 100); - Assert.assertEquals(test2.getDataProvider(), ""); + Assert.assertEquals(test2.getDataProvider(), new String[] {}); } public void verifyTestMethodLevel() throws SecurityException, NoSuchMethodException @@ -74,7 +74,7 @@ public void verifyTestMethodLevel() throws SecurityException, NoSuchMethodExcept Assert.assertEquals(test1.getTimeOut(), 142); Assert.assertEquals(test1.getInvocationCount(), 143); Assert.assertEquals(test1.getSuccessPercentage(), 61); - Assert.assertEquals(test1.getDataProvider(), "dp2"); + Assert.assertEquals(test1.getDataProvider(), new String[] {"dp2"}); Assert.assertEquals(test1.getDescription(), "Method description"); Class[] exceptions = test1.getExpectedExceptions(); Assert.assertEquals(exceptions.length, 1); @@ -98,7 +98,7 @@ public void verifyTestConstructorLevel() throws SecurityException, NoSuchMethodE Assert.assertEquals(test1.getTimeOut(), 242); Assert.assertEquals(test1.getInvocationCount(), 243); Assert.assertEquals(test1.getSuccessPercentage(), 62); - Assert.assertEquals(test1.getDataProvider(), "dp3"); + Assert.assertEquals(test1.getDataProvider(), new String[] {"dp3"}); Assert.assertEquals(test1.getDescription(), "Constructor description"); Class[] exceptions = test1.getExpectedExceptions(); Assert.assertEquals(exceptions.length, 1); From 4d9cc6c72c45353e0be511ecfb6d7a25eb091f45 Mon Sep 17 00:00:00 2001 From: James Phillpotts Date: Wed, 11 Nov 2015 16:44:11 +0000 Subject: [PATCH 2/2] Fix cartesian product --- src/main/java/org/testng/internal/Parameters.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/testng/internal/Parameters.java b/src/main/java/org/testng/internal/Parameters.java index afa22c99cc..eb3f890109 100755 --- a/src/main/java/org/testng/internal/Parameters.java +++ b/src/main/java/org/testng/internal/Parameters.java @@ -304,8 +304,6 @@ private static IDataProvidable findDataProviderInfo(ITestClass clazz, Constructo private static List findDataProvider(ConstructorOrMethod annotated, Object instance, ITestClass clazz, IAnnotationFinder finder, String[] names, Class dataProviderClass, ITestContext context) { - DataProviderHolder result = null; - Class cls = clazz.getRealClass(); boolean shouldBeStatic = false; if (dataProviderClass != null) { @@ -315,6 +313,7 @@ private static List findDataProvider(ConstructorOrMethod ann List providers = new ArrayList<>(); for (String name : names) { + DataProviderHolder result = null; for (Method m : ClassHelper.getAvailableMethods(cls)) { IDataProviderAnnotation dp = finder.findAnnotation(m, IDataProviderAnnotation.class); if (null != dp && name.equals(getDataProviderName(dp, m))) { @@ -480,11 +479,13 @@ private static Iterator cartesianProduct(List> cart List result = new ArrayList<>(); Iterator left = cartesianParameters.get(0); - Iterator right = cartesianParameters.get(1); + List right = new ArrayList<>(); + while (cartesianParameters.get(1).hasNext()) { + right.add(cartesianParameters.get(1).next()); + } while (left.hasNext()) { Object[] leftRow = left.next(); - while (right.hasNext()) { - Object[] rightRow = right.next(); + for (Object[] rightRow : right) { Object[] row = new Object[leftRow.length + rightRow.length]; System.arraycopy(leftRow, 0, row, 0, leftRow.length); System.arraycopy(rightRow, 0, row, leftRow.length, rightRow.length);