Fixed: If inherited configuration methods had defined deps, they coul…

…d be invoked in incorrect order

(no new test is needed. Existing test.inheritance.* tests seem sufficent. Ensure that all tests passed)

CHANGES.txt

@@ -7,10 +7,10 @@ Added: ITestNGListenerFactory
 Added: Passing command line properties via the ant task and doc update (Todd Wells)
 Added: Hierarchical XmlSuites (Nalin Makar) 
 Added: Reporter#clear()
+Fixed: If inherited configuration methods had defined deps, they could be invoked in incorrect order (Nalin Makar)
 Fixed: Initialize all Suite/Test runners at beginning to catch configuration issues right at start (Nalin Makar)
-Fixed: Incorrect dates reported for configuration methods (http://code.google.com/p/testng/issues/detail?id=7 and 86)
-Fixed: Initialize all Suite/Test runners at beginning to catch configuration issues right at start
-Fixed: Issue24 OOM errors in SuiteHTMLReporter (Nalin Makar)
+Fixed: Issue7: Issue86 Incorrect dates reported for configuration methods
+Fixed: Issue24: OOM errors in SuiteHTMLReporter (Nalin Makar)
 Fixed: Time outs specified in XML were not honored for <suite parallel="tests">
 Fixed: <suite> and <test> time outs were hardcoded, they now honor their time-out attribute
 Fixed: TestNG was hanging if no test methods were found

src/org/testng/internal/MethodHelper.java

@@ -23,7 +23,6 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
@@ -75,11 +74,10 @@ private static ITestNGMethod[] internalCollectAndOrderMethods(ITestNGMethod[] me
         runInfo, 
         finder, 
         unique);
-    List<ITestNGMethod> vResult = sortMethods(forTests, includedMethods, finder);
 
-    ITestNGMethod[] result = vResult.toArray(new ITestNGMethod[vResult.size()]);
-
-    return result;
+    return includedMethods.size() > 1 ?  
+      sortMethods(forTests, includedMethods, finder).toArray(new ITestNGMethod[]{})
+      : includedMethods.toArray(new ITestNGMethod[]{});
   }
 
 
@@ -480,7 +478,7 @@ private static boolean isMethodAlreadyPresent(List<ITestNGMethod> result,
     return false;
   }
 
-  static public Graph<ITestNGMethod> topologicalSort(ITestNGMethod[] methods,
+  private static Graph<ITestNGMethod> topologicalSort(ITestNGMethod[] methods,
       List<ITestNGMethod> sequentialList, List<ITestNGMethod> parallelList) {
     Graph<ITestNGMethod> result = new Graph<ITestNGMethod>();
 
@@ -492,29 +490,28 @@ static public Graph<ITestNGMethod> topologicalSort(ITestNGMethod[] methods,
     for (ITestNGMethod m : methods) {
       result.addNode(m);
 
-      Map<ITestNGMethod, ITestNGMethod> predecessors = Maps.newHashMap();
+      List<ITestNGMethod> predecessors = Lists.newArrayList();
 
       String[] methodsDependedUpon = m.getMethodsDependedUpon();
       String[] groupsDependedUpon = m.getGroupsDependedUpon();
       if (methodsDependedUpon.length > 0) {
         ITestNGMethod[] methodsNamed = 
           MethodHelper.findMethodsNamed(m, methods, methodsDependedUpon);
         for (ITestNGMethod pred : methodsNamed) {
-          predecessors.put(pred, pred);
+          predecessors.add(pred);
         }
       }
       if (groupsDependedUpon.length > 0) {
         for (String group : groupsDependedUpon) {
           ITestNGMethod[] methodsThatBelongToGroup = 
             MethodHelper.findMethodsThatBelongToGroup(m, methods, group);
           for (ITestNGMethod pred : methodsThatBelongToGroup) {
-            predecessors.put(pred, pred);
+            predecessors.add(pred);
           }
         }
       }
 
-      for (ITestNGMethod predecessor : predecessors.values()) {
-        // ppp("METHOD:" + m + " ADDED PREDECESSOR:" + predecessor);
+      for (ITestNGMethod predecessor : predecessors) {
         result.addPredecessor(m, predecessor);
       }
     }
@@ -790,8 +787,6 @@ public static long calculateTimeOut(ITestNGMethod tm) {
   public static void invokeWithTimeout(ITestNGMethod tm, Object instance, Object[] parameterValues, 
       ITestResult testResult) 
   throws InterruptedException, ThreadExecutionException {
-//    ICountDown done= ThreadUtil.createCountDown(1);
-//    IThreadFactory factory= ThreadUtil.createFactory();
     IExecutor exec= ThreadUtil.createExecutor(1, tm.getMethod().getName());
 
     InvokeMethodRunnable imr = new InvokeMethodRunnable(tm, instance, parameterValues);

src/org/testng/internal/MethodInheritance.java

@@ -57,17 +57,6 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean baseCla
     // Map of classes -> List of methods that belong to this class or same hierarchy
     Map<Class, List<ITestNGMethod>> map = Maps.newHashMap();
 
-    //
-    // First, make sure that none of these methods define a dependency of its own
-    //
-    for (ITestNGMethod m : methods) {
-      String[] mdu = m.getMethodsDependedUpon();
-      String[] groups = m.getGroupsDependedUpon();
-      if ((mdu != null && mdu.length > 0) || (groups != null && groups.length > 0)) {
-        return;
-      }
-    }
-
     //
     // Put the list of methods in their hierarchy buckets
     //
@@ -106,7 +95,7 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean baseCla
           for (int i = 1; i < l.size(); i++) {
             ITestNGMethod m1 = l.get(i - 1);
             ITestNGMethod m2 = l.get(i);
-            if (! equalsEffectiveClass(m1, m2)) {
+            if (!equalsEffectiveClass(m1, m2) && !dependencyExists(m1, m2, methods)) {
               Utils.log("MethodInheritance", 4, m2 + " DEPENDS ON " + m1);
               m2.addMethodDependedUpon(MethodHelper.calculateMethodCanonicalName(m1));
             }
@@ -116,7 +105,7 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean baseCla
           for (int i = 0; i < l.size() - 1; i++) {
             ITestNGMethod m1 = l.get(i);
             ITestNGMethod m2 = l.get(i + 1);
-            if (! equalsEffectiveClass(m1, m2)) {
+            if (!equalsEffectiveClass(m1, m2) && !dependencyExists(m1, m2, methods)) {
               m2.addMethodDependedUpon(MethodHelper.calculateMethodCanonicalName(m1));
               Utils.log("MethodInheritance", 4, m2 + " DEPENDS ON " + m1);
             }
@@ -126,6 +115,34 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean baseCla
     }
   }
 
+  private static boolean dependencyExists(ITestNGMethod m1, ITestNGMethod m2, ITestNGMethod[] methods) {
+    return true == internalDependencyExists(m1, m2, methods) 
+       ? true : internalDependencyExists(m2, m1, methods);
+  }
+
+  private static boolean internalDependencyExists(ITestNGMethod m1, ITestNGMethod m2, ITestNGMethod[] methods) {
+    ITestNGMethod[] methodsNamed = 
+      MethodHelper.findMethodsNamed(m1, methods, m1.getMethodsDependedUpon());
+
+    for (ITestNGMethod method : methodsNamed) {
+      if (method.equals(m2)) {
+        return true;
+      }
+    }
+
+    for (String group : m1.getGroupsDependedUpon()) {
+      ITestNGMethod[] methodsThatBelongToGroup = 
+        MethodHelper.findMethodsThatBelongToGroup(m1, methods, group);
+      for (ITestNGMethod method : methodsThatBelongToGroup) {
+         if (method.equals(m2)) {
+           return true;
+         }
+       }
+    }
+
+    return false;
+  }
+
   private static boolean equalsEffectiveClass(ITestNGMethod m1, ITestNGMethod m2) {
     try {
       Class c1 = m1.getRealClass();