From 675d707023d277da5700b1cc491e4c24a84cad50 Mon Sep 17 00:00:00 2001 From: nullin Date: Tue, 6 Jul 2010 00:06:14 -0700 Subject: [PATCH] Fixed: If inherited configuration methods had defined deps, they could be invoked in incorrect order (no new test is needed. Existing test.inheritance.* tests seem sufficent. Ensure that all tests passed) --- CHANGES.txt | 6 +-- src/org/testng/internal/MethodHelper.java | 21 ++++----- .../testng/internal/MethodInheritance.java | 43 +++++++++++++------ 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index bf0d2fd18a..fbd04a649c 100644 --- a/CHANGES.txt +++ b/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 Fixed: and time outs were hardcoded, they now honor their time-out attribute Fixed: TestNG was hanging if no test methods were found diff --git a/src/org/testng/internal/MethodHelper.java b/src/org/testng/internal/MethodHelper.java index ac6ed9cab2..05a1738560 100644 --- a/src/org/testng/internal/MethodHelper.java +++ b/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 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 result, return false; } - static public Graph topologicalSort(ITestNGMethod[] methods, + private static Graph topologicalSort(ITestNGMethod[] methods, List sequentialList, List parallelList) { Graph result = new Graph(); @@ -492,7 +490,7 @@ static public Graph topologicalSort(ITestNGMethod[] methods, for (ITestNGMethod m : methods) { result.addNode(m); - Map predecessors = Maps.newHashMap(); + List predecessors = Lists.newArrayList(); String[] methodsDependedUpon = m.getMethodsDependedUpon(); String[] groupsDependedUpon = m.getGroupsDependedUpon(); @@ -500,7 +498,7 @@ static public Graph topologicalSort(ITestNGMethod[] methods, ITestNGMethod[] methodsNamed = MethodHelper.findMethodsNamed(m, methods, methodsDependedUpon); for (ITestNGMethod pred : methodsNamed) { - predecessors.put(pred, pred); + predecessors.add(pred); } } if (groupsDependedUpon.length > 0) { @@ -508,13 +506,12 @@ static public Graph topologicalSort(ITestNGMethod[] methods, 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); diff --git a/src/org/testng/internal/MethodInheritance.java b/src/org/testng/internal/MethodInheritance.java index 6172e44aab..1807348b3b 100755 --- a/src/org/testng/internal/MethodInheritance.java +++ b/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> 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();