From 13275dda785ed00e1611e928da7c9a8d02bd69d9 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Tue, 11 Jul 2017 09:34:16 +0530 Subject: [PATCH] @Factory with dataProvider changes order of iterations Closes #799 --- CHANGES.txt | 1 + src/main/java/org/testng/internal/Graph.java | 17 ++-- .../org/testng/internal/Systematiser.java | 86 +++++++++++++++++++ ...EnsureInstancesAreOrderedViaFactories.java | 66 ++++++++++++++ .../test/github799/InstanceTestSample.java | 37 ++++++++ .../test/github799/MethodsTestSample.java | 28 ++++++ .../github799/ReverseOrderTestSample.java | 27 ++++++ src/test/java/test/github799/TestSample.java | 27 ++++++ src/test/resources/testng.xml | 7 ++ 9 files changed, 289 insertions(+), 7 deletions(-) create mode 100644 src/main/java/org/testng/internal/Systematiser.java create mode 100644 src/test/java/test/github799/EnsureInstancesAreOrderedViaFactories.java create mode 100644 src/test/java/test/github799/InstanceTestSample.java create mode 100644 src/test/java/test/github799/MethodsTestSample.java create mode 100644 src/test/java/test/github799/ReverseOrderTestSample.java create mode 100644 src/test/java/test/github799/TestSample.java diff --git a/CHANGES.txt b/CHANGES.txt index 757ff36388..926fc875e0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-799: @Factory with dataProvider changes order of iterations (Krishnan Mahadevan) New: Enhance XML Reporter to be able to customize the file name (Krishnan Mahadevan) Fixed: GITHUB-1417: Class param injection is not working with @BeforeClass (Krishnan Mahadevan) Fixed: GITHUB-1440: Improve error message when wrong params on configuration methods (Krishnan Mahadevan) diff --git a/src/main/java/org/testng/internal/Graph.java b/src/main/java/org/testng/internal/Graph.java index d833610e47..c8c7b6a8da 100644 --- a/src/main/java/org/testng/internal/Graph.java +++ b/src/main/java/org/testng/internal/Graph.java @@ -6,6 +6,7 @@ import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -121,7 +122,10 @@ public void topologicalSort() { // Sort the nodes alphabetically to make sure that methods of the same class // get run close to each other as much as possible // - Collections.sort(nodes2); + Comparator comparator = Systematiser.getComparator(); + if (comparator != null) { + Collections.sort(nodes2, comparator); + } // // Sort @@ -160,7 +164,10 @@ private void initializeIndependentNodes() { // Ideally, we should not have to sort this. However, due to a bug where it treats all the methods as // dependent nodes. Therefore, all the nodes were mostly sorted alphabetically. So we need to keep // the behavior for now. - Collections.sort(list); + Comparator comparator = Systematiser.getComparator(); + if (comparator != null) { + Collections.sort(list, comparator); + } m_independentNodes = Maps.newLinkedHashMap(); for (Node node : list) { m_independentNodes.put(node.getObject(), node); @@ -255,7 +262,7 @@ public String toString() { ///// // class Node // - public static class Node implements Comparable> { + public static class Node { private T m_object = null; private Map m_predecessors = Maps.newHashMap(); @@ -335,9 +342,5 @@ public boolean hasPredecessor(T m) { return m_predecessors.containsKey(m); } - @Override - public int compareTo(Node o) { - return getObject().toString().compareTo(o.getObject().toString()); - } } } diff --git a/src/main/java/org/testng/internal/Systematiser.java b/src/main/java/org/testng/internal/Systematiser.java new file mode 100644 index 0000000000..8548e380a4 --- /dev/null +++ b/src/main/java/org/testng/internal/Systematiser.java @@ -0,0 +1,86 @@ +package org.testng.internal; + +import org.testng.ITestNGMethod; + +import java.util.Comparator; + +public class Systematiser { + + private Systematiser() { + //Utility class. Defeat instantiation. + } + + public static Comparator getComparator() { + Comparator comparator = null; + String text = System.getProperty("testng.order", Order.INSTANCES.getValue()); + + Order order = Order.parse(text); + switch (order) { + case INSTANCES: + comparator = new Comparator() { + @Override + public int compare(Graph.Node o1, Graph.Node o2) { + return o1.getObject().toString().compareTo(o2.getObject().toString()); + } + + @Override + public String toString() { + return "Instance_Names"; + } + }; + break; + + case METHOD_NAMES: + comparator = new Comparator() { + @Override + public int compare(Graph.Node o1, Graph.Node o2) { + if (o1.getObject() instanceof ITestNGMethod && o2.getObject() instanceof ITestNGMethod) { + String n1 = ((ITestNGMethod) o1.getObject()).getMethodName(); + String n2 = ((ITestNGMethod) o1.getObject()).getMethodName(); + return n1.compareTo(n2); + } + return o1.getObject().getClass().getName().compareTo(o2.getObject().getClass().getName()); + } + + @Override + public String toString() { + return "Method_Names"; + } + }; + break; + + case NONE: + default: + } + + return comparator; + } + + enum Order { + METHOD_NAMES("methods"), + INSTANCES("instances"), + NONE("none"); + + Order(String value) { + this.value = value; + } + + private String value; + + public String getValue() { + return value; + } + + public static Order parse(String value) { + if (value == null || value.trim().isEmpty()) { + return INSTANCES; + } + for (Order each : values()) { + if (each.getValue().equalsIgnoreCase(value)) { + return each; + } + } + return INSTANCES; + } + } +} diff --git a/src/test/java/test/github799/EnsureInstancesAreOrderedViaFactories.java b/src/test/java/test/github799/EnsureInstancesAreOrderedViaFactories.java new file mode 100644 index 0000000000..1e93587962 --- /dev/null +++ b/src/test/java/test/github799/EnsureInstancesAreOrderedViaFactories.java @@ -0,0 +1,66 @@ +package test.github799; + +import org.testng.Assert; +import org.testng.IInvokedMethod; +import org.testng.IInvokedMethodListener; +import org.testng.ITestNGListener; +import org.testng.ITestResult; +import org.testng.Reporter; +import org.testng.TestNG; +import org.testng.annotations.Test; +import org.testng.collections.Lists; +import test.SimpleBaseTest; + +import java.util.List; + +public class EnsureInstancesAreOrderedViaFactories extends SimpleBaseTest { + + @Test + public void testMethod() { + System.setProperty("testng.order", "none"); + runTest(TestSample.class, "1", "2", "3", "4"); + } + + @Test + public void randomOrderTestMethod() { + System.setProperty("testng.order", "none"); + runTest(ReverseOrderTestSample.class, "4", "1", "3", "2"); + } + + @Test + public void methodsOrderTest() { + System.setProperty("testng.order", "methods"); + runTest(MethodsTestSample.class, "android", "angry", "birds"); + } + + @Test + public void testInstancesOrder() { + System.setProperty("testng.order", "instances"); + runTest(InstanceTestSample.class, "Master Oogway:90", "Master Shifu:50"); + } + + private void runTest(Class clazz, String... expected) { + TestNG tng = create(clazz); + OrderEavesdropper listener = new OrderEavesdropper(); + tng.addListener((ITestNGListener) listener); + tng.run(); + + for (int i = 0; i < expected.length; i++) { + String actual = listener.messages.get(i); + Assert.assertEquals(actual, expected[i]); + } + } + + public static class OrderEavesdropper implements IInvokedMethodListener { + List messages = Lists.newArrayList(); + + @Override + public void beforeInvocation(IInvokedMethod method, ITestResult testResult) { + } + + @Override + public void afterInvocation(IInvokedMethod method, ITestResult testResult) { + messages.addAll(Reporter.getOutput(testResult)); + } + } +} \ No newline at end of file diff --git a/src/test/java/test/github799/InstanceTestSample.java b/src/test/java/test/github799/InstanceTestSample.java new file mode 100644 index 0000000000..3408253cc9 --- /dev/null +++ b/src/test/java/test/github799/InstanceTestSample.java @@ -0,0 +1,37 @@ +package test.github799; + +import org.testng.Assert; +import org.testng.Reporter; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Factory; +import org.testng.annotations.Test; + +public class InstanceTestSample { + private String name; + private int age; + + @Factory(dataProvider = "dp") + public InstanceTestSample(String name, int age) { + this.name = name; + this.age = age; + } + + @DataProvider(name = "dp") + public static Object[][] getData() { + return new Object[][]{ + {"Master Shifu", 50}, + {"Master Oogway", 90} + }; + } + + @Test + public void testMethod() { + Reporter.log(toString()); + Assert.assertNotNull(this.name); + } + + @Override + public String toString() { + return name + ":" + age; + } +} diff --git a/src/test/java/test/github799/MethodsTestSample.java b/src/test/java/test/github799/MethodsTestSample.java new file mode 100644 index 0000000000..6286d7b232 --- /dev/null +++ b/src/test/java/test/github799/MethodsTestSample.java @@ -0,0 +1,28 @@ +package test.github799; + +import org.testng.Assert; +import org.testng.Reporter; +import org.testng.annotations.Test; + +public class MethodsTestSample { + @Test + public void angry() { + String methodName = Reporter.getCurrentTestResult().getMethod().getMethodName(); + Reporter.log(methodName); + Assert.assertNotNull(methodName); + } + + @Test + public void birds() { + String methodName = Reporter.getCurrentTestResult().getMethod().getMethodName(); + Reporter.log(methodName); + Assert.assertNotNull(methodName); + } + + @Test + public void android() { + String methodName = Reporter.getCurrentTestResult().getMethod().getMethodName(); + Reporter.log(methodName); + Assert.assertNotNull(methodName); + } +} diff --git a/src/test/java/test/github799/ReverseOrderTestSample.java b/src/test/java/test/github799/ReverseOrderTestSample.java new file mode 100644 index 0000000000..7c26acf378 --- /dev/null +++ b/src/test/java/test/github799/ReverseOrderTestSample.java @@ -0,0 +1,27 @@ +package test.github799; + +import org.testng.Assert; +import org.testng.Reporter; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Factory; +import org.testng.annotations.Test; + +public class ReverseOrderTestSample { + int num; + + @Factory(dataProvider = "data") + public ReverseOrderTestSample(int n) { + num = n; + } + + @DataProvider + public static Object[][] data() { + return new Object[][]{{4}, {1}, {3}, {2}}; + } + + @Test + public void test() { + Reporter.log(Integer.toString(num)); + Assert.assertTrue(num > 0); + } +} \ No newline at end of file diff --git a/src/test/java/test/github799/TestSample.java b/src/test/java/test/github799/TestSample.java new file mode 100644 index 0000000000..2661df3ccf --- /dev/null +++ b/src/test/java/test/github799/TestSample.java @@ -0,0 +1,27 @@ +package test.github799; + +import org.testng.Assert; +import org.testng.Reporter; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Factory; +import org.testng.annotations.Test; + +public class TestSample { + int num; + + @Factory(dataProvider = "data") + public TestSample(int n) { + num = n; + } + + @DataProvider + public static Object[][] data() { + return new Object[][]{{1}, {2}, {3}, {4}}; + } + + @Test + public void test() { + Reporter.log(Integer.toString(num)); + Assert.assertTrue(num > 0); + } +} \ No newline at end of file diff --git a/src/test/resources/testng.xml b/src/test/resources/testng.xml index e3f8836af6..5dd4a7e9c7 100644 --- a/src/test/resources/testng.xml +++ b/src/test/resources/testng.xml @@ -815,6 +815,7 @@ + @@ -822,5 +823,11 @@ + + + + + +