Skip to content

Commit

Permalink
Minor Code tweaks
Browse files Browse the repository at this point in the history
* Made some minor refactoring to graph implementation 
for better readability of code.
* Leveraged streams wherever possible in graphs
* Avoid code duplication
  • Loading branch information
krmahadevan committed Nov 28, 2023
1 parent 7357104 commit ea53982
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 50 deletions.
20 changes: 9 additions & 11 deletions testng-core/src/main/java/org/testng/internal/BaseTestMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -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} */
Expand Down
32 changes: 11 additions & 21 deletions testng-core/src/main/java/org/testng/internal/Graph.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand All @@ -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));
}
}

Expand All @@ -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) {
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -295,7 +285,7 @@ public void addPredecessor(T tm) {
}

public boolean hasPredecessors() {
return m_predecessors.size() > 0;
return !m_predecessors.isEmpty();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
35 changes: 18 additions & 17 deletions testng-core/src/main/java/org/testng/internal/Tarjan.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down

0 comments on commit ea53982

Please sign in to comment.