Minor Code tweaks

* Made some minor refactoring to graph implementation 
for better readability of code.
* Leveraged streams wherever possible in graphs
* Avoid code duplication

Author: krmahadevan

testng-core/src/main/java/org/testng/internal/BaseTestMethod.java

@@ -190,25 +190,23 @@ public Set<ITestNGMethod> upstreamDependencies() {
   }
 
   public void setDownstreamDependencies(Set<ITestNGMethod> methods) {
-    if (!downstreamDependencies.isEmpty()) {
-      downstreamDependencies.clear();
-    }
-    Set<ITestNGMethod> toAdd = methods;
-    if (RuntimeBehavior.isMemoryFriendlyMode()) {
-      toAdd = methods.stream().map(LiteWeightTestNGMethod::new).collect(Collectors.toSet());
-    }
-    downstreamDependencies.addAll(toAdd);
+    setupDependencies(downstreamDependencies, methods);
   }
 
   public void setUpstreamDependencies(Set<ITestNGMethod> methods) {
-    if (!upstreamDependencies.isEmpty()) {
-      upstreamDependencies.clear();
+    setupDependencies(upstreamDependencies, methods);
+  }
+
+  private static void setupDependencies(
+      Set<ITestNGMethod> dependencies, Set<ITestNGMethod> methods) {
+    if (!dependencies.isEmpty()) {
+      dependencies.clear();
     }
     Set<ITestNGMethod> toAdd = methods;
     if (RuntimeBehavior.isMemoryFriendlyMode()) {
       toAdd = methods.stream().map(LiteWeightTestNGMethod::new).collect(Collectors.toSet());
     }
-    upstreamDependencies.addAll(toAdd);
+    dependencies.addAll(toAdd);
   }
 
   /** {@inheritDoc} */

testng-core/src/main/java/org/testng/internal/Graph.java

@@ -7,6 +7,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import org.testng.TestNGException;
@@ -113,6 +114,7 @@ public void topologicalSort() {
       //
       Node<T> node = findNodeWithNoPredecessors(nodes2);
       if (null == node) {
+        // We found a cycle. Time to use the Tarjan's algorithm to get the cycle.
         List<T> cycle = new Tarjan<>(this, nodes2.get(0).getObject()).getCycle();
         StringBuilder sb = new StringBuilder();
         sb.append("The following methods have cyclic dependencies:\n");
@@ -132,16 +134,12 @@ public void topologicalSort() {
 
   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.
-      list.sort(comparator);
-
-      m_independentNodes = Maps.newLinkedHashMap();
-      for (Node<T> node : list) {
-        m_independentNodes.put(node.getObject(), node);
-      }
+      m_independentNodes =
+          Lists.newArrayList(m_nodes.values()).stream()
+              .sorted(comparator)
+              .collect(
+                  Collectors.toMap(
+                      Node::getObject, Function.identity(), (a, b) -> a, Maps::newLinkedHashMap));
     }
   }
 
@@ -159,9 +157,7 @@ private void dumpSortedNodes() {
    */
   private void removeFromNodes(List<Node<T>> nodes, Node<T> node) {
     nodes.remove(node);
-    for (Node<T> n : nodes) {
-      n.removePredecessor(node.getObject());
-    }
+    nodes.parallelStream().forEach(it -> it.removePredecessor(node.getObject()));
   }
 
   private static void log(String s) {
@@ -173,13 +169,7 @@ private static void log(Supplier<String> s) {
   }
 
   private Node<T> findNodeWithNoPredecessors(List<Node<T>> nodes) {
-    for (Node<T> n : nodes) {
-      if (!n.hasPredecessors()) {
-        return n;
-      }
-    }
-
-    return null;
+    return nodes.parallelStream().filter(it -> !it.hasPredecessors()).findFirst().orElse(null);
   }
 
   /**
@@ -295,7 +285,7 @@ public void addPredecessor(T tm) {
     }
 
     public boolean hasPredecessors() {
-      return m_predecessors.size() > 0;
+      return !m_predecessors.isEmpty();
     }
   }
 }

testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java

@@ -205,7 +205,7 @@ protected static void findGroupTransitiveClosure(
       //
       // Only keep going if new methods have been added
       //
-      keepGoing = newMethods.size() > 0;
+      keepGoing = !newMethods.isEmpty();
       includedMethods = Lists.newArrayList();
       includedMethods.addAll(newMethods.keySet());
       newMethods = Maps.newHashMap();

testng-core/src/main/java/org/testng/internal/Tarjan.java

@@ -14,39 +14,40 @@
  */
 public class Tarjan<T> {
   int m_index = 0;
-  private final Stack<T> m_s;
-  Map<T, Integer> m_indices = Maps.newHashMap();
+  private final Stack<T> stack;
+  Map<T, Integer> visitedNodes = Maps.newHashMap();
   Map<T, Integer> m_lowlinks = Maps.newHashMap();
   private List<T> m_cycle;
 
   public Tarjan(Graph<T> graph, T start) {
-    m_s = new Stack<>();
+    stack = new Stack<>();
     run(graph, start);
   }
 
-  private void run(Graph<T> graph, T v) {
-    m_indices.put(v, m_index);
-    m_lowlinks.put(v, m_index);
+  private void run(Graph<T> graph, T start) {
+    visitedNodes.put(start, m_index);
+    m_lowlinks.put(start, m_index);
     m_index++;
-    m_s.push(v);
+    stack.push(start);
 
-    for (T vprime : graph.getPredecessors(v)) {
-      if (!m_indices.containsKey(vprime)) {
-        run(graph, vprime);
-        int min = Math.min(m_lowlinks.get(v), m_lowlinks.get(vprime));
-        m_lowlinks.put(v, min);
-      } else if (m_s.contains(vprime)) {
-        m_lowlinks.put(v, Math.min(m_lowlinks.get(v), m_indices.get(vprime)));
+    for (T predecessor : graph.getPredecessors(start)) {
+      if (!visitedNodes.containsKey(predecessor)) {
+        run(graph, predecessor);
+        int min = Math.min(m_lowlinks.get(start), m_lowlinks.get(predecessor));
+        m_lowlinks.put(start, min);
+      } else if (stack.contains(predecessor)) {
+        int min = Math.min(m_lowlinks.get(start), visitedNodes.get(predecessor));
+        m_lowlinks.put(start, min);
       }
     }
 
-    if (Objects.equals(m_lowlinks.get(v), m_indices.get(v))) {
+    if (Objects.equals(m_lowlinks.get(start), visitedNodes.get(start))) {
       m_cycle = Lists.newArrayList();
       T n;
       do {
-        n = m_s.pop();
+        n = stack.pop();
         m_cycle.add(n);
-      } while (!n.equals(v));
+      } while (!n.equals(start));
     }
   }