Skip to content

Commit

Permalink
Merge pull request #1467 from krmahadevan/fix-799
Browse files Browse the repository at this point in the history 
@factory with dataProvider changes order of iterations
  • Loading branch information
@cbeust
cbeust committed Jul 15, 2017
2 parents 0b0d9a3 + 9c0524d commit 4c446b4
Show file tree
Hide file tree
Showing 18 changed files with 452 additions and 76 deletions.
1 change: 1 addition & 0 deletions CHANGES.txt
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Current
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)
Expand Down
70 changes: 59 additions & 11 deletions src/main/java/org/testng/SuiteRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ITestNGMethod> comparator) {
this(configuration, suite, outputDir, null, comparator);
}

public SuiteRunner(IConfiguration configuration, XmlSuite suite, String outputDir,
ITestRunnerFactory runnerFactory, Comparator<ITestNGMethod> comparator) {
this(configuration, suite, outputDir, runnerFactory, false, comparator);
}

public SuiteRunner(IConfiguration configuration,
XmlSuite suite,
String outputDir,
ITestRunnerFactory runnerFactory,
boolean useDefaultListeners, Comparator<ITestNGMethod> comparator)
{
this(configuration, suite, outputDir, runnerFactory, useDefaultListeners,
new ArrayList<IMethodInterceptor>() /* 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,
Expand All @@ -96,7 +123,7 @@ public SuiteRunner(IConfiguration configuration,
new ArrayList<IMethodInterceptor>() /* method interceptor */,
null /* invoked method listeners */,
null /* test listeners */,
null /* class listeners */);
null /* class listeners */, Systematiser.getComparator());
}

/**
Expand All @@ -115,9 +142,12 @@ protected SuiteRunner(IConfiguration configuration,
List<ITestListener> testListeners,
List<IClassListener> 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,
Expand All @@ -128,7 +158,21 @@ protected SuiteRunner(IConfiguration configuration,
Collection<ITestListener> testListeners,
Collection<IClassListener> 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<IMethodInterceptor> methodInterceptors,
Collection<IInvokedMethodListener> invokedMethodListeners,
Collection<ITestListener> testListeners,
Collection<IClassListener> classListeners, Comparator<ITestNGMethod> comparator)
{
init(configuration, suite, outputDir, runnerFactory, useDefaultListeners,
methodInterceptors, invokedMethodListeners, testListeners, classListeners, comparator);
}

private void init(IConfiguration configuration,
Expand All @@ -139,8 +183,7 @@ private void init(IConfiguration configuration,
List<IMethodInterceptor> methodInterceptors,
Collection<IInvokedMethodListener> invokedMethodListener,
Collection<ITestListener> testListeners,
Collection<IClassListener> classListeners)
{
Collection<IClassListener> classListeners, Comparator<ITestNGMethod> comparator) {
this.configuration = configuration;
xmlSuite = suite;
this.useDefaultListeners = useDefaultListeners;
Expand Down Expand Up @@ -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 <test> tags based on their order of appearance in testng.xml
List<XmlTest> xmlTests = xmlSuite.getTests();
Expand Down Expand Up @@ -240,13 +286,13 @@ private void setOutputDir(String outputdir) {
: null;
}

private ITestRunnerFactory buildRunnerFactory() {
private ITestRunnerFactory buildRunnerFactory(Comparator<ITestNGMethod> 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(
Expand Down Expand Up @@ -555,16 +601,18 @@ private static class DefaultTestRunnerFactory implements ITestRunnerFactory {
private boolean useDefaultListeners;
private boolean skipFailedInvocationCounts;
private IConfiguration configuration;
private final Comparator<ITestNGMethod> comparator;

public DefaultTestRunnerFactory(IConfiguration configuration,
ITestListener[] failureListeners,
boolean useDefaultListeners,
boolean skipFailedInvocationCounts)
boolean skipFailedInvocationCounts, Comparator<ITestNGMethod> comparator)
{
this.configuration = configuration;
failureGenerators = failureListeners;
this.useDefaultListeners = useDefaultListeners;
this.skipFailedInvocationCounts = skipFailedInvocationCounts;
this.comparator = comparator;
}

@Override
Expand All @@ -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());
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/testng/TestNG.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
42 changes: 36 additions & 6 deletions src/main/java/org/testng/TestRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,6 +68,8 @@ public class TestRunner

/* generated */
private static final long serialVersionUID = 4247820024988306670L;

private final Comparator<ITestNGMethod> comparator;
private ISuite m_suite;
private XmlTest m_xmlTest;
private String m_testName;
Expand Down Expand Up @@ -151,6 +154,7 @@ private enum PriorityWeight {
groupByInstance, preserveOrder, priority, dependsOnGroups, dependsOnMethods
}

@Deprecated
protected TestRunner(IConfiguration configuration,
ISuite suite,
XmlTest test,
Expand All @@ -160,14 +164,40 @@ protected TestRunner(IConfiguration configuration,
Collection<IInvokedMethodListener> invokedMethodListeners,
List<IClassListener> 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<IInvokedMethodListener> invokedMethodListeners,
List<IClassListener> 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<IInvokedMethodListener> invokedMethodListeners,
List<IClassListener> classListeners, Comparator<ITestNGMethod> comparator) {
this.comparator = comparator;
init(configuration, suite, test, outputDirectory, finder, skipFailedInvocationCounts,
invokedMethodListeners, classListeners);
}

public TestRunner(IConfiguration configuration, ISuite suite, XmlTest test,
boolean skipFailedInvocationCounts,
Collection<IInvokedMethodListener> invokedMethodListeners,
List<IClassListener> classListeners, Comparator<ITestNGMethod> comparator) {
this.comparator = comparator;
init(configuration, suite, test, suite.getOutputDirectory(),
suite.getAnnotationFinder(),
skipFailedInvocationCounts, invokedMethodListeners, classListeners);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -462,36 +492,36 @@ 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,
false /* forTests */,
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);

Expand Down
20 changes: 11 additions & 9 deletions src/main/java/org/testng/internal/Graph.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -21,12 +22,17 @@ public class Graph<T> {
private static boolean m_verbose = false;
private Map<T, Node<T>> m_nodes = Maps.newLinkedHashMap();
private List<T> m_strictlySortedNodes = null;
private final Comparator<Node<T>> 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<T, Node<T>> m_independentNodes = null;

public Graph(Comparator<Node<T>> comparator) {
this.comparator = comparator;
}

public void addNode(T tm) {
ppp("ADDING NODE " + tm + " " + tm.hashCode());
m_nodes.put(tm, new Node<>(tm));
Expand Down Expand Up @@ -121,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);
Collections.sort(nodes2, comparator);

//
// Sort
Expand Down Expand Up @@ -158,9 +164,9 @@ private void initializeIndependentNodes() {
if (null == m_independentNodes) {
List<Node<T>> 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);
// dependent nodes.
Collections.sort(list, comparator);

m_independentNodes = Maps.newLinkedHashMap();
for (Node<T> node : list) {
m_independentNodes.put(node.getObject(), node);
Expand Down Expand Up @@ -255,7 +261,7 @@ public String toString() {
/////
// class Node
//
public static class Node<T> implements Comparable<Node<T>> {
public static class Node<T> {
private T m_object = null;
private Map<T, T> m_predecessors = Maps.newHashMap();

Expand Down Expand Up @@ -335,9 +341,5 @@ public boolean hasPredecessor(T m) {
return m_predecessors.containsKey(m);
}

@Override
public int compareTo(Node<T> o) {
return getObject().toString().compareTo(o.getObject().toString());
}
}
}
Loading

0 comments on commit 4c446b4

Please sign in to comment.