From 13275dda785ed00e1611e928da7c9a8d02bd69d9 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Tue, 11 Jul 2017 09:34:16 +0530 Subject: [PATCH 1/4] @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 @@ + + + + + + From 4bd8a35d8d99b32fed0b765f88b03833e4fec597 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Wed, 12 Jul 2017 14:46:46 +0530 Subject: [PATCH 2/4] Fixing review comments. --- src/main/java/org/testng/internal/Graph.java | 11 ++--- .../org/testng/internal/Systematiser.java | 41 ++++++++++++------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/testng/internal/Graph.java b/src/main/java/org/testng/internal/Graph.java index c8c7b6a8da..095d658f3a 100644 --- a/src/main/java/org/testng/internal/Graph.java +++ b/src/main/java/org/testng/internal/Graph.java @@ -122,10 +122,7 @@ 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 // - Comparator comparator = Systematiser.getComparator(); - if (comparator != null) { - Collections.sort(nodes2, comparator); - } + Collections.sort(nodes2, Systematiser.getComparator()); // // Sort @@ -164,10 +161,8 @@ 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. - Comparator comparator = Systematiser.getComparator(); - if (comparator != null) { - Collections.sort(list, comparator); - } + Collections.sort(list, Systematiser.getComparator()); + m_independentNodes = Maps.newLinkedHashMap(); for (Node node : list) { m_independentNodes.put(node.getObject(), node); diff --git a/src/main/java/org/testng/internal/Systematiser.java b/src/main/java/org/testng/internal/Systematiser.java index 8548e380a4..caca590d2b 100644 --- a/src/main/java/org/testng/internal/Systematiser.java +++ b/src/main/java/org/testng/internal/Systematiser.java @@ -4,53 +4,66 @@ import java.util.Comparator; -public class Systematiser { +public final class Systematiser { private Systematiser() { //Utility class. Defeat instantiation. } public static Comparator getComparator() { - Comparator comparator = null; + Comparator comparator; String text = System.getProperty("testng.order", Order.INSTANCES.getValue()); Order order = Order.parse(text); switch (order) { - case INSTANCES: + case METHOD_NAMES: comparator = new Comparator() { @Override public int compare(Graph.Node o1, Graph.Node o2) { - return o1.getObject().toString().compareTo(o2.getObject().toString()); + 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 "Instance_Names"; + return "Method_Names"; } }; break; - case METHOD_NAMES: + case NONE: + //Disables sorting by providing a dummy comparator which always regards two elements as equal. 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()); + return 0; } @Override public String toString() { - return "Method_Names"; + return "No_Sorting"; } }; break; - case NONE: default: + 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"; + } + }; + } return comparator; From 66d78dc55a5b1b3e9c1cb536744dc3727825d1c7 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Sat, 15 Jul 2017 09:49:02 +0200 Subject: [PATCH 3/4] Move Systematiser usage to TestNG class --- src/main/java/org/testng/SuiteRunner.java | 70 ++++++++++++++++--- src/main/java/org/testng/TestNG.java | 3 +- src/main/java/org/testng/TestRunner.java | 42 +++++++++-- src/main/java/org/testng/internal/Graph.java | 12 ++-- .../org/testng/internal/MethodHelper.java | 27 ++++--- .../org/testng/internal/Systematiser.java | 29 ++++---- src/main/java/org/testng/internal/Tarjan.java | 22 ------ .../testng/internal/TestNGMethodFinder.java | 8 ++- .../org/testng/reporters/FailedReporter.java | 4 +- src/test/java/test/BaseTest.java | 8 +-- src/test/java/test/GraphTest.java | 24 ++++--- 11 files changed, 162 insertions(+), 87 deletions(-) diff --git a/src/main/java/org/testng/SuiteRunner.java b/src/main/java/org/testng/SuiteRunner.java index c13b3f86c3..ad3547d321 100644 --- a/src/main/java/org/testng/SuiteRunner.java +++ b/src/main/java/org/testng/SuiteRunner.java @@ -7,6 +7,7 @@ import org.testng.internal.Attributes; import org.testng.internal.IConfiguration; import org.testng.internal.IInvoker; +import org.testng.internal.Systematiser; import org.testng.internal.Utils; import org.testng.internal.annotations.IAnnotationFinder; import org.testng.internal.thread.ThreadUtil; @@ -78,14 +79,40 @@ public class SuiteRunner implements ISuite, Serializable, IInvokedMethodListener private SuiteRunState suiteState = new SuiteRunState(); private IAttributes attributes = new Attributes(); + public SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir, + Comparator comparator) { + this(configuration, suite, outputDir, null, comparator); + } + + public SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir, + ITestRunnerFactory runnerFactory, Comparator comparator) { + this(configuration, suite, outputDir, runnerFactory, false, comparator); + } + + public SuiteRunner(IConfiguration configuration, + XmlSuite suite, + String outputDir, + ITestRunnerFactory runnerFactory, + boolean useDefaultListeners, Comparator comparator) + { + this(configuration, suite, outputDir, runnerFactory, useDefaultListeners, + new ArrayList() /* method interceptor */, + null /* invoked method listeners */, + null /* test listeners */, + null /* class listeners */, comparator); + } + + @Deprecated public SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir) { - this(configuration, suite, outputDir, null); + this(configuration, suite, outputDir, (ITestRunnerFactory) null); } + @Deprecated public SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir, ITestRunnerFactory runnerFactory) { this(configuration, suite, outputDir, runnerFactory, false); } + @Deprecated public SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir, @@ -96,7 +123,7 @@ public SuiteRunner(IConfiguration configuration, new ArrayList() /* method interceptor */, null /* invoked method listeners */, null /* test listeners */, - null /* class listeners */); + null /* class listeners */, Systematiser.getComparator()); } /** @@ -115,9 +142,12 @@ protected SuiteRunner(IConfiguration configuration, List testListeners, List classListeners) { - init(configuration, suite, outputDir, runnerFactory, useDefaultListeners, methodInterceptors, invokedMethodListeners, testListeners, classListeners); + init(configuration, suite, outputDir, runnerFactory, useDefaultListeners, + methodInterceptors, invokedMethodListeners, testListeners, classListeners, + Systematiser.getComparator()); } + @Deprecated protected SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir, @@ -128,7 +158,21 @@ protected SuiteRunner(IConfiguration configuration, Collection testListeners, Collection classListeners) { - init(configuration, suite, outputDir, runnerFactory, useDefaultListeners, methodInterceptors, invokedMethodListeners, testListeners, classListeners); + init(configuration, suite, outputDir, runnerFactory, useDefaultListeners, methodInterceptors, invokedMethodListeners, testListeners, classListeners, Systematiser.getComparator()); + } + + protected SuiteRunner(IConfiguration configuration, + XmlSuite suite, + String outputDir, + ITestRunnerFactory runnerFactory, + boolean useDefaultListeners, + List methodInterceptors, + Collection invokedMethodListeners, + Collection testListeners, + Collection classListeners, Comparator comparator) + { + init(configuration, suite, outputDir, runnerFactory, useDefaultListeners, + methodInterceptors, invokedMethodListeners, testListeners, classListeners, comparator); } private void init(IConfiguration configuration, @@ -139,8 +183,7 @@ private void init(IConfiguration configuration, List methodInterceptors, Collection invokedMethodListener, Collection testListeners, - Collection classListeners) - { + Collection classListeners, Comparator comparator) { this.configuration = configuration; xmlSuite = suite; this.useDefaultListeners = useDefaultListeners; @@ -170,7 +213,10 @@ private void init(IConfiguration configuration, this.classListeners.put(classListener.getClass(), classListener); } } - this.runnerFactory = buildRunnerFactory(); + if (comparator == null) { + throw new IllegalArgumentException("comparator must not be null"); + } + this.runnerFactory = buildRunnerFactory(comparator); // Order the tags based on their order of appearance in testng.xml List xmlTests = xmlSuite.getTests(); @@ -240,13 +286,13 @@ private void setOutputDir(String outputdir) { : null; } - private ITestRunnerFactory buildRunnerFactory() { + private ITestRunnerFactory buildRunnerFactory(Comparator comparator) { ITestRunnerFactory factory; if (null == tmpRunnerFactory) { factory = new DefaultTestRunnerFactory(configuration, testListeners.toArray(new ITestListener[testListeners.size()]), - useDefaultListeners, skipFailedInvocationCounts); + useDefaultListeners, skipFailedInvocationCounts, comparator); } else { factory = new ProxyTestRunnerFactory( @@ -555,16 +601,18 @@ private static class DefaultTestRunnerFactory implements ITestRunnerFactory { private boolean useDefaultListeners; private boolean skipFailedInvocationCounts; private IConfiguration configuration; + private final Comparator comparator; public DefaultTestRunnerFactory(IConfiguration configuration, ITestListener[] failureListeners, boolean useDefaultListeners, - boolean skipFailedInvocationCounts) + boolean skipFailedInvocationCounts, Comparator comparator) { this.configuration = configuration; failureGenerators = failureListeners; this.useDefaultListeners = useDefaultListeners; this.skipFailedInvocationCounts = skipFailedInvocationCounts; + this.comparator = comparator; } @Override @@ -576,7 +624,7 @@ public TestRunner newTestRunner(ISuite suite, XmlTest test, } TestRunner testRunner = new TestRunner(configuration, suite, test, suite.getOutputDirectory(), suite.getAnnotationFinder(), skip, - listeners, classListeners); + listeners, classListeners, comparator); if (useDefaultListeners) { testRunner.addListener(new TestHTMLReporter()); diff --git a/src/main/java/org/testng/TestNG.java b/src/main/java/org/testng/TestNG.java index 124efc0d93..b002dd612b 100644 --- a/src/main/java/org/testng/TestNG.java +++ b/src/main/java/org/testng/TestNG.java @@ -32,6 +32,7 @@ import org.testng.internal.IResultListener2; import org.testng.internal.OverrideProcessor; import org.testng.internal.SuiteRunnerMap; +import org.testng.internal.Systematiser; import org.testng.internal.Utils; import org.testng.internal.Version; import org.testng.internal.annotations.DefaultAnnotationTransformer; @@ -1393,7 +1394,7 @@ private SuiteRunner createSuiteRunner(XmlSuite xmlSuite) { m_methodInterceptors, m_invokedMethodListeners.values(), m_testListeners.values(), - m_classListeners.values()); + m_classListeners.values(), Systematiser.getComparator()); for (ISuiteListener isl : m_suiteListeners.values()) { result.addListener(isl); diff --git a/src/main/java/org/testng/TestRunner.java b/src/main/java/org/testng/TestRunner.java index 1e88cbc4ec..dfa101d2a5 100644 --- a/src/main/java/org/testng/TestRunner.java +++ b/src/main/java/org/testng/TestRunner.java @@ -36,6 +36,7 @@ import org.testng.internal.MethodInstance; import org.testng.internal.ResultMap; import org.testng.internal.RunInfo; +import org.testng.internal.Systematiser; import org.testng.internal.TestMethodWorker; import org.testng.internal.TestNGClassFinder; import org.testng.internal.TestNGMethodFinder; @@ -67,6 +68,8 @@ public class TestRunner /* generated */ private static final long serialVersionUID = 4247820024988306670L; + + private final Comparator comparator; private ISuite m_suite; private XmlTest m_xmlTest; private String m_testName; @@ -151,6 +154,7 @@ private enum PriorityWeight { groupByInstance, preserveOrder, priority, dependsOnGroups, dependsOnMethods } + @Deprecated protected TestRunner(IConfiguration configuration, ISuite suite, XmlTest test, @@ -160,14 +164,40 @@ protected TestRunner(IConfiguration configuration, Collection invokedMethodListeners, List classListeners) { + this.comparator = Systematiser.getComparator(); init(configuration, suite, test, outputDirectory, finder, skipFailedInvocationCounts, invokedMethodListeners, classListeners); } + @Deprecated public TestRunner(IConfiguration configuration, ISuite suite, XmlTest test, boolean skipFailedInvocationCounts, Collection invokedMethodListeners, List classListeners) { + this.comparator = Systematiser.getComparator(); + init(configuration, suite, test, suite.getOutputDirectory(), + suite.getAnnotationFinder(), + skipFailedInvocationCounts, invokedMethodListeners, classListeners); + } + + protected TestRunner(IConfiguration configuration, + ISuite suite, + XmlTest test, + String outputDirectory, + IAnnotationFinder finder, + boolean skipFailedInvocationCounts, + Collection invokedMethodListeners, + List classListeners, Comparator comparator) { + this.comparator = comparator; + init(configuration, suite, test, outputDirectory, finder, skipFailedInvocationCounts, + invokedMethodListeners, classListeners); + } + + public TestRunner(IConfiguration configuration, ISuite suite, XmlTest test, + boolean skipFailedInvocationCounts, + Collection invokedMethodListeners, + List classListeners, Comparator comparator) { + this.comparator = comparator; init(configuration, suite, test, suite.getOutputDirectory(), suite.getAnnotationFinder(), skipFailedInvocationCounts, invokedMethodListeners, classListeners); @@ -405,7 +435,7 @@ private void initMethods() { m_configuration, this); ITestMethodFinder testMethodFinder - = new TestNGMethodFinder(m_runInfo, m_annotationFinder); + = new TestNGMethodFinder(m_runInfo, m_annotationFinder, comparator); m_runInfo.setTestMethods(testMethods); @@ -462,21 +492,21 @@ private void initMethods() { m_runInfo, m_annotationFinder, true /* unique */, - m_excludedMethods); + m_excludedMethods, comparator); m_beforeXmlTestMethods = MethodHelper.collectAndOrderMethods(beforeXmlTestMethods, false /* forTests */, m_runInfo, m_annotationFinder, true /* unique (CQ added by me)*/, - m_excludedMethods); + m_excludedMethods, comparator); m_allTestMethods = MethodHelper.collectAndOrderMethods(testMethods, true /* forTest? */, m_runInfo, m_annotationFinder, false /* unique */, - m_excludedMethods); + m_excludedMethods, comparator); m_classMethodMap = new ClassMethodMap(testMethods, m_xmlMethodSelector); m_afterXmlTestMethods = MethodHelper.collectAndOrderMethods(afterXmlTestMethods, @@ -484,14 +514,14 @@ private void initMethods() { m_runInfo, m_annotationFinder, true /* unique (CQ added by me)*/, - m_excludedMethods); + m_excludedMethods, comparator); m_afterSuiteMethods = MethodHelper.collectAndOrderMethods(afterSuiteMethods, false /* forTests */, m_runInfo, m_annotationFinder, true /* unique */, - m_excludedMethods); + m_excludedMethods, comparator); // shared group methods m_groupMethods = new ConfigurationGroupMethods(m_allTestMethods, beforeGroupMethods, afterGroupMethods); diff --git a/src/main/java/org/testng/internal/Graph.java b/src/main/java/org/testng/internal/Graph.java index 095d658f3a..60a5823d48 100644 --- a/src/main/java/org/testng/internal/Graph.java +++ b/src/main/java/org/testng/internal/Graph.java @@ -22,12 +22,17 @@ public class Graph { private static boolean m_verbose = false; private Map> m_nodes = Maps.newLinkedHashMap(); private List m_strictlySortedNodes = null; + private final Comparator> comparator; // A map of nodes that are not the predecessors of any node // (not needed for the algorithm but convenient to calculate // the parallel/sequential lists in TestNG). private Map> m_independentNodes = null; + public Graph(Comparator> comparator) { + this.comparator = comparator; + } + public void addNode(T tm) { ppp("ADDING NODE " + tm + " " + tm.hashCode()); m_nodes.put(tm, new Node<>(tm)); @@ -122,7 +127,7 @@ 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, Systematiser.getComparator()); + Collections.sort(nodes2, comparator); // // Sort @@ -159,9 +164,8 @@ private void initializeIndependentNodes() { if (null == m_independentNodes) { List> list = Lists.newArrayList(m_nodes.values()); // 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, Systematiser.getComparator()); + // dependent nodes. + Collections.sort(list, comparator); m_independentNodes = Maps.newLinkedHashMap(); for (Node node : list) { diff --git a/src/main/java/org/testng/internal/MethodHelper.java b/src/main/java/org/testng/internal/MethodHelper.java index 199ffe4757..1e8c700d10 100644 --- a/src/main/java/org/testng/internal/MethodHelper.java +++ b/src/main/java/org/testng/internal/MethodHelper.java @@ -2,6 +2,7 @@ import java.lang.reflect.Method; import java.util.Collection; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Set; @@ -14,6 +15,7 @@ import org.testng.annotations.ITestOrConfiguration; import org.testng.collections.Lists; import org.testng.collections.Sets; +import org.testng.internal.Graph.Node; import org.testng.internal.annotations.AnnotationHelper; import org.testng.internal.annotations.IAnnotationFinder; import org.testng.internal.collections.Pair; @@ -43,8 +45,8 @@ public class MethodHelper { */ public static ITestNGMethod[] collectAndOrderMethods(List methods, boolean forTests, RunInfo runInfo, IAnnotationFinder finder, - boolean unique, List outExcludedMethods) - { + boolean unique, List outExcludedMethods, + Comparator comparator) { List includedMethods = Lists.newArrayList(); MethodGroupsHelper.collectMethodsByGroup(methods.toArray(new ITestNGMethod[methods.size()]), forTests, @@ -54,7 +56,7 @@ public static ITestNGMethod[] collectAndOrderMethods(List methods finder, unique); - return sortMethods(forTests, includedMethods, finder).toArray(new ITestNGMethod[]{}); + return sortMethods(forTests, includedMethods, comparator).toArray(new ITestNGMethod[]{}); } /** @@ -186,8 +188,14 @@ public static List uniqueMethodList(Collection topologicalSort(ITestNGMethod[] methods, - List sequentialList, List parallelList) { - Graph result = new Graph<>(); + List sequentialList, List parallelList, + final Comparator comparator) { + Graph result = new Graph<>(new Comparator>() { + @Override + public int compare(Node o1, Node o2) { + return comparator.compare(o1.getObject(), o2.getObject()); + } + }); if (methods.length == 0) { return result; @@ -265,7 +273,7 @@ private static String calculateMethodCanonicalName(Method m) { } private static List sortMethods(boolean forTests, - List allMethods, IAnnotationFinder finder) { + List allMethods, Comparator comparator) { List sl = Lists.newArrayList(); List pl = Lists.newArrayList(); ITestNGMethod[] allMethodsArray = allMethods.toArray(new ITestNGMethod[allMethods.size()]); @@ -281,7 +289,7 @@ private static List sortMethods(boolean forTests, MethodInheritance.fixMethodInheritance(allMethodsArray, before); } - topologicalSort(allMethodsArray, sl, pl); + topologicalSort(allMethodsArray, sl, pl, comparator); List result = Lists.newArrayList(); result.addAll(sl); @@ -292,12 +300,13 @@ private static List sortMethods(boolean forTests, /** * @return A sorted array containing all the methods 'method' depends on */ - public static List getMethodsDependedUpon(ITestNGMethod method, ITestNGMethod[] methods) { + public static List getMethodsDependedUpon(ITestNGMethod method, + ITestNGMethod[] methods, Comparator comparator) { Graph g = GRAPH_CACHE.get(methods); if (g == null) { List parallelList = Lists.newArrayList(); List sequentialList = Lists.newArrayList(); - g = topologicalSort(methods, sequentialList, parallelList); + g = topologicalSort(methods, sequentialList, parallelList, comparator); GRAPH_CACHE.put(methods, g); } diff --git a/src/main/java/org/testng/internal/Systematiser.java b/src/main/java/org/testng/internal/Systematiser.java index caca590d2b..4442d338bc 100644 --- a/src/main/java/org/testng/internal/Systematiser.java +++ b/src/main/java/org/testng/internal/Systematiser.java @@ -10,22 +10,19 @@ private Systematiser() { //Utility class. Defeat instantiation. } - public static Comparator getComparator() { - Comparator comparator; + public static Comparator getComparator() { + Comparator comparator; String text = System.getProperty("testng.order", Order.INSTANCES.getValue()); Order order = Order.parse(text); switch (order) { case METHOD_NAMES: - comparator = new Comparator() { + 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()); + public int compare(ITestNGMethod o1, ITestNGMethod o2) { + String n1 = o1.getMethodName(); + String n2 = o1.getMethodName(); + return n1.compareTo(n2); } @Override @@ -37,9 +34,9 @@ public String toString() { case NONE: //Disables sorting by providing a dummy comparator which always regards two elements as equal. - comparator = new Comparator() { + comparator = new Comparator() { @Override - public int compare(Graph.Node o1, Graph.Node o2) { + public int compare(ITestNGMethod o1, ITestNGMethod o2) { return 0; } @@ -50,12 +47,12 @@ public String toString() { }; break; - default: case INSTANCES: - comparator = new Comparator() { + default: + comparator = new Comparator() { @Override - public int compare(Graph.Node o1, Graph.Node o2) { - return o1.getObject().toString().compareTo(o2.getObject().toString()); + public int compare(ITestNGMethod o1, ITestNGMethod o2) { + return o1.toString().compareTo(o2.toString()); } @Override diff --git a/src/main/java/org/testng/internal/Tarjan.java b/src/main/java/org/testng/internal/Tarjan.java index 07ba35daad..5d3cce90f2 100644 --- a/src/main/java/org/testng/internal/Tarjan.java +++ b/src/main/java/org/testng/internal/Tarjan.java @@ -52,28 +52,6 @@ else if (m_s.contains(vprime)) { } - public static void main(String[] args) { - Graph g = new Graph<>(); - g.addNode("a"); - g.addNode("b"); - g.addNode("c"); - g.addNode("d"); - - String[] edges = new String[] { - "a", "b", - "b", "a", - "c", "d", - "d", "a", - "a", "c", - }; - - for (int i = 0; i < edges.length; i += 2) { - g.addPredecessor(edges[i], edges[i+1]); - } - - new Tarjan<>(g, "a"); - } - public List getCycle() { return m_cycle; } diff --git a/src/main/java/org/testng/internal/TestNGMethodFinder.java b/src/main/java/org/testng/internal/TestNGMethodFinder.java index a3ca0f88f7..c7ebb01796 100644 --- a/src/main/java/org/testng/internal/TestNGMethodFinder.java +++ b/src/main/java/org/testng/internal/TestNGMethodFinder.java @@ -2,6 +2,7 @@ import java.lang.reflect.Method; +import java.util.Comparator; import java.util.List; import java.util.Set; @@ -35,10 +36,13 @@ public class TestNGMethodFinder implements ITestMethodFinder { private RunInfo runInfo = null; private IAnnotationFinder annotationFinder = null; + private final Comparator comparator; - public TestNGMethodFinder(RunInfo runInfo, IAnnotationFinder annotationFinder) { + public TestNGMethodFinder(RunInfo runInfo, IAnnotationFinder annotationFinder, + Comparator comparator) { this.runInfo = runInfo; this.annotationFinder = annotationFinder; + this.comparator = comparator; } @Override @@ -192,7 +196,7 @@ private ITestNGMethod[] findConfiguration(final Class clazz, final int configura runInfo, annotationFinder, unique, - excludedMethods); + excludedMethods, comparator); } diff --git a/src/main/java/org/testng/reporters/FailedReporter.java b/src/main/java/org/testng/reporters/FailedReporter.java index f0ab87c351..f61849093d 100644 --- a/src/main/java/org/testng/reporters/FailedReporter.java +++ b/src/main/java/org/testng/reporters/FailedReporter.java @@ -12,6 +12,7 @@ import org.testng.collections.Maps; import org.testng.collections.Sets; import org.testng.internal.MethodHelper; +import org.testng.internal.Systematiser; import org.testng.internal.Utils; import org.testng.xml.XmlClass; import org.testng.xml.XmlInclude; @@ -94,7 +95,8 @@ private void generateXmlTest(XmlTest xmlTest, continue; } methodsToReRun.add(current); - List methodsDependedUpon = MethodHelper.getMethodsDependedUpon(current, context.getAllTestMethods()); + List methodsDependedUpon = MethodHelper.getMethodsDependedUpon(current, + context.getAllTestMethods(), Systematiser.getComparator()); for (ITestNGMethod m : methodsDependedUpon) { if (m.isTest()) { diff --git a/src/test/java/test/BaseTest.java b/src/test/java/test/BaseTest.java index 4ebb452a79..36dcaf2362 100644 --- a/src/test/java/test/BaseTest.java +++ b/src/test/java/test/BaseTest.java @@ -27,6 +27,7 @@ import org.testng.collections.Lists; import org.testng.internal.Configuration; import org.testng.internal.IConfiguration; +import org.testng.internal.Systematiser; import org.testng.reporters.JUnitXMLReporter; import org.testng.reporters.TestHTMLReporter; import org.testng.xml.XmlClass; @@ -185,7 +186,7 @@ protected void run() { m_suite.setVerbose(m_verbose != null ? m_verbose : 0); SuiteRunner suite = new SuiteRunner(m_configuration, - m_suite, m_outputDirectory, m_testRunnerFactory); + m_suite, m_outputDirectory, m_testRunnerFactory, Systematiser.getComparator()); suite.run(); } @@ -401,14 +402,11 @@ public InternalTestRunnerFactory(final BaseTest baseTest) { m_baseTest= baseTest; } - /** - * @see org.testng.ITestRunnerFactory#newTestRunner(org.testng.ISuite, org.testng.xml.XmlTest) - */ @Override public TestRunner newTestRunner(ISuite suite, XmlTest test, Collection listeners, List classListeners) { TestRunner testRunner= new TestRunner(m_baseTest.getConfiguration(), suite, test, false, - listeners, classListeners); + listeners, classListeners, Systematiser.getComparator()); testRunner.addListener(new TestHTMLReporter()); testRunner.addListener(new JUnitXMLReporter()); diff --git a/src/test/java/test/GraphTest.java b/src/test/java/test/GraphTest.java index 871f9597ab..35c0040ff8 100644 --- a/src/test/java/test/GraphTest.java +++ b/src/test/java/test/GraphTest.java @@ -1,24 +1,28 @@ package test; +import java.util.Comparator; import org.testng.Assert; import org.testng.TestNGException; import org.testng.annotations.Test; import org.testng.internal.Graph; +import org.testng.internal.Graph.Node; import org.testng.internal.Tarjan; import java.util.List; - -/** - * This class - * - * @author cbeust - */ public class GraphTest { + private static final Comparator> COMPARATOR = new Comparator>() { + + @Override + public int compare(Node o1, Node o2) { + return o1.getObject().compareTo(o2.getObject()); + } + }; + @Test public void sort() { - Graph g = new Graph<>(); + Graph g = new Graph<>(COMPARATOR); g.addNode("3"); g.addNode("1"); g.addNode("2.2"); @@ -70,7 +74,7 @@ public void cycleShouldBeCorrect() { } private Graph createCyclicGraph() { - Graph g = new Graph<>(); + Graph g = new Graph<>(COMPARATOR); g.addNode("3"); g.addNode("2"); g.addNode("1"); @@ -84,7 +88,7 @@ private Graph createCyclicGraph() { @Test public void findPredecessors() { - Graph g = new Graph<>(); + Graph g = new Graph<>(COMPARATOR); g.addNode("3"); g.addNode("1"); g.addNode("2.2"); @@ -146,7 +150,7 @@ public void findPredecessors() { // @Test(timeOut = 5000) // If this takes more than 5 seconds we've definitely regressed. public void findPredecessorsTiming() { - Graph g = new Graph<>(); + Graph g = new Graph<>(COMPARATOR); final String rootNode = "myroot"; final String independentNode = "independent"; From 9c0524dde4a71ba8f3e376b5bc32630f9897af68 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Sat, 15 Jul 2017 17:27:42 +0530 Subject: [PATCH 4/4] Fixing javadocs. --- CHANGES.txt | 2 +- src/main/java/org/testng/internal/Systematiser.java | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGES.txt b/CHANGES.txt index 926fc875e0..a489232987 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,5 @@ Current -Fixed: GITHUB-799: @Factory with dataProvider changes order of iterations (Krishnan Mahadevan) +Fixed: GITHUB-799: @Factory with dataProvider changes order of iterations (Krishnan Mahadevan & Julien Herr) 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/Systematiser.java b/src/main/java/org/testng/internal/Systematiser.java index 4442d338bc..3379b8172b 100644 --- a/src/main/java/org/testng/internal/Systematiser.java +++ b/src/main/java/org/testng/internal/Systematiser.java @@ -4,12 +4,23 @@ import java.util.Comparator; +/** + * Helps determine how should {@link ITestNGMethod} be ordered by TestNG. + */ public final class Systematiser { private Systematiser() { //Utility class. Defeat instantiation. } + /** + * @return - A {@link Comparator} that helps TestNG sort {@link ITestNGMethod}s in a specific order. + * Currently the following two orders are supported :
+ *
    + *
  1. Based on the name of methods
    2. + *
  2. Based on the toString() implementation that resides within the test class
    3. + *
+ */ public static Comparator getComparator() { Comparator comparator; String text = System.getProperty("testng.order", Order.INSTANCES.getValue());