Skip to content

Commit

Permalink
Segregate native & user listeners before ordering
Browse files Browse the repository at this point in the history
Closes #2771
  • Loading branch information
krmahadevan committed Feb 1, 2023
1 parent c087eaa commit 9588843
Show file tree
Hide file tree
Showing 17 changed files with 221 additions and 33 deletions.
1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Current
Fixed: GITHUB-2771: After upgrading to TestNG 7.5.0, setting ITestResult.status to FAILURE doesn't fail the test anymore (Julien Herr & Krishnan Mahadevan)
Fixed: GITHUB-2796: Option for onAfterClass to run after @AfterClass
Fixed: GITHUB-2857: XmlTest index is not set for test suites invoked with YAML

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,4 @@ public static <T> List<T> merge(List<T> l1, BiPredicate<T, T> condition, List<T>
});
return result;
}

public static <K> List<K> newReversedArrayList(Collection<K> c) {
List<K> list = newArrayList(c);
Collections.reverse(list);
return list;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package org.testng.internal;

import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.TimeZone;

/** This class houses handling all JVM arguments by TestNG */
Expand All @@ -16,13 +19,20 @@ public final class RuntimeBehavior {
public static final String TESTNG_DEFAULT_VERBOSE = "testng.default.verbose";
public static final String IGNORE_CALLBACK_INVOCATION_SKIPS = "testng.ignore.callback.skip";
public static final String SYMMETRIC_LISTENER_EXECUTION = "testng.listener.execution.symmetric";
public static final String IDE_PACKAGE_NAMES = "testng.ide.listeners.package.name";

private RuntimeBehavior() {}

public static boolean ignoreCallbackInvocationSkips() {
return Boolean.getBoolean(IGNORE_CALLBACK_INVOCATION_SKIPS);
}

public static List<String> getIdePackageNames() {
String packages =
Optional.ofNullable(System.getProperty(IDE_PACKAGE_NAMES)).orElse("com.intellij.rt.*");
return Arrays.asList(packages.split(","));
}

public static boolean strictParallelism() {
return Boolean.getBoolean(STRICTLY_HONOUR_PARALLEL_MODE);
}
Expand Down
39 changes: 30 additions & 9 deletions testng-core/src/main/java/org/testng/SuiteRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public class SuiteRunner implements ISuite, IInvokedMethodListener {
private final SuiteRunState suiteState = new SuiteRunState();
private final IAttributes attributes = new Attributes();
private final Set<IExecutionVisualiser> visualisers = Sets.newHashSet();
private ITestListener exitCodeListener;

public SuiteRunner(
IConfiguration configuration,
Expand All @@ -94,7 +95,7 @@ public SuiteRunner(
useDefaultListeners,
new ArrayList<>() /* method interceptor */,
null /* invoked method listeners */,
null /* test listeners */,
new TestListenersContainer() /* test listeners */,
null /* class listeners */,
new DataProviderHolder(),
comparator);
Expand All @@ -108,7 +109,7 @@ protected SuiteRunner(
boolean useDefaultListeners,
List<IMethodInterceptor> methodInterceptors,
Collection<IInvokedMethodListener> invokedMethodListeners,
Collection<ITestListener> testListeners,
TestListenersContainer container,
Collection<IClassListener> classListeners,
DataProviderHolder holder,
Comparator<ITestNGMethod> comparator) {
Expand All @@ -120,7 +121,7 @@ protected SuiteRunner(
useDefaultListeners,
methodInterceptors,
invokedMethodListeners,
testListeners,
container,
classListeners,
holder,
comparator);
Expand All @@ -134,7 +135,7 @@ private void init(
boolean useDefaultListeners,
List<IMethodInterceptor> methodInterceptors,
Collection<IInvokedMethodListener> invokedMethodListener,
Collection<ITestListener> testListeners,
TestListenersContainer container,
Collection<IClassListener> classListeners,
DataProviderHolder attribs,
Comparator<ITestNGMethod> comparator) {
Expand All @@ -146,6 +147,7 @@ private void init(
this.xmlSuite = suite;
this.useDefaultListeners = useDefaultListeners;
this.tmpRunnerFactory = runnerFactory;
this.exitCodeListener = container.exitCodeListener;
List<IMethodInterceptor> localMethodInterceptors =
Optional.ofNullable(methodInterceptors).orElse(Lists.newArrayList());
setOutputDir(outputDir);
Expand Down Expand Up @@ -206,9 +208,7 @@ public <T> T newInstance(Constructor<T> constructor, Object... parameters) {
}

skipFailedInvocationCounts = suite.skipFailedInvocationCounts();
if (null != testListeners) {
this.testListeners.addAll(testListeners);
}
this.testListeners.addAll(container.listeners);
for (IClassListener classListener :
Optional.ofNullable(classListeners).orElse(Collections.emptyList())) {
this.classListeners.put(classListener.getClass(), classListener);
Expand Down Expand Up @@ -257,13 +257,19 @@ public void setReportResults(boolean reportResults) {
useDefaultListeners = reportResults;
}

ITestListener getExitCodeListener() {
return exitCodeListener;
}

private void invokeListeners(boolean start) {
if (start) {
for (ISuiteListener sl : Lists.newArrayList(listeners.values())) {
for (ISuiteListener sl :
ListenerOrderDeterminer.order(Lists.newArrayList(listeners.values()))) {
sl.onStart(this);
}
} else {
List<ISuiteListener> suiteListenersReversed = Lists.newReversedArrayList(listeners.values());
List<ISuiteListener> suiteListenersReversed =
ListenerOrderDeterminer.reversedOrder(listeners.values());
for (ISuiteListener sl : suiteListenersReversed) {
sl.onFinish(this);
}
Expand Down Expand Up @@ -813,4 +819,19 @@ public List<ITestNGMethod> getAllMethods() {
.flatMap(tr -> Arrays.stream(tr.getAllTestMethods()))
.collect(Collectors.toList());
}

static class TestListenersContainer {
private final List<ITestListener> listeners = Lists.newArrayList();
private final ITestListener exitCodeListener;

TestListenersContainer() {
this(Collections.emptyList(), null);
}

TestListenersContainer(List<ITestListener> listeners, ITestListener exitCodeListener) {
this.listeners.addAll(listeners);
this.exitCodeListener =
Objects.requireNonNullElseGet(exitCodeListener, () -> new ITestListener() {});
}
}
}
22 changes: 17 additions & 5 deletions testng-core/src/main/java/org/testng/TestNG.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Set;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import org.testng.SuiteRunner.TestListenersContainer;
import org.testng.annotations.ITestAnnotation;
import org.testng.collections.Lists;
import org.testng.collections.Maps;
Expand All @@ -29,6 +30,7 @@
import org.testng.internal.DynamicGraph;
import org.testng.internal.ExitCode;
import org.testng.internal.IConfiguration;
import org.testng.internal.ListenerOrderDeterminer;
import org.testng.internal.OverrideProcessor;
import org.testng.internal.ReporterConfig;
import org.testng.internal.RuntimeBehavior;
Expand Down Expand Up @@ -924,7 +926,6 @@ private void initializeDefaultListeners() {
if (m_failIfAllTestsSkipped) {
this.exitCodeListener.failIfAllTestsSkipped();
}
addListener(this.exitCodeListener);
if (m_useDefaultListeners) {
addReporter(SuiteHTMLReporter.class);
addReporter(Main.class);
Expand Down Expand Up @@ -1106,17 +1107,22 @@ private void runSuiteAlterationListeners() {
}

private void runExecutionListeners(boolean start) {
List<IExecutionListener> executionListeners = m_configuration.getExecutionListeners();
List<IExecutionListener> executionListeners =
ListenerOrderDeterminer.order(m_configuration.getExecutionListeners());
if (start) {
for (IExecutionListener l : executionListeners) {
l.onExecutionStart();
}
// Invoke our exit code listener after all the user's listeners have run.
exitCodeListener.onExecutionStart();
} else {
List<IExecutionListener> executionListenersReversed =
Lists.newReversedArrayList(executionListeners);
ListenerOrderDeterminer.reversedOrder(executionListeners);
for (IExecutionListener l : executionListenersReversed) {
l.onExecutionFinish();
}
// Invoke our exit code listener after all the user's listeners have run.
exitCodeListener.onExecutionFinish();
}
}

Expand All @@ -1128,7 +1134,11 @@ private static void usage() {
}

private void generateReports(List<ISuite> suiteRunners) {
for (IReporter reporter : m_reporters.values()) {
List<IReporter> reporters = new ArrayList<>(m_reporters.values());
// Add our Exit code listener as the last of the reporter so that we can still accommodate
// whatever changes were done by a user's reporting listener
reporters.add(exitCodeListener);
for (IReporter reporter : reporters) {
try {
long start = System.currentTimeMillis();
reporter.generateReport(m_suites, suiteRunners, m_outputDir);
Expand Down Expand Up @@ -1334,6 +1344,8 @@ private SuiteRunner createSuiteRunner(XmlSuite xmlSuite) {
DataProviderHolder holder = new DataProviderHolder();
holder.addListeners(m_dataProviderListeners.values());
holder.addInterceptors(m_dataProviderInterceptors.values());
TestListenersContainer container =
new TestListenersContainer(getTestListeners(), this.exitCodeListener);
SuiteRunner result =
new SuiteRunner(
getConfiguration(),
Expand All @@ -1343,7 +1355,7 @@ private SuiteRunner createSuiteRunner(XmlSuite xmlSuite) {
m_useDefaultListeners,
m_methodInterceptors,
m_invokedMethodListeners.values(),
m_testListeners.values(),
container,
m_classListeners.values(),
holder,
Systematiser.getComparator());
Expand Down
27 changes: 25 additions & 2 deletions testng-core/src/main/java/org/testng/TestRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
Expand All @@ -36,6 +37,7 @@
import org.testng.internal.IContainer;
import org.testng.internal.ITestClassConfigInfo;
import org.testng.internal.ITestResultNotifier;
import org.testng.internal.ListenerOrderDeterminer;
import org.testng.internal.MethodGroupsHelper;
import org.testng.internal.MethodHelper;
import org.testng.internal.ResultMap;
Expand Down Expand Up @@ -118,6 +120,8 @@ public class TestRunner
// The XML method selector (groups/methods included/excluded in XML)
private final XmlMethodSelector m_xmlMethodSelector = new XmlMethodSelector();

private ITestListener exitCodeListener;

//
// These next fields contain all the configuration methods found on this class.
// At initialization time, they just contain all the various @Configuration methods
Expand Down Expand Up @@ -252,6 +256,13 @@ private void init(
m_injectorFactory = m_configuration.getInjectorFactory();
m_objectFactory = suite.getObjectFactory();
setVerbose(test.getVerbose());
if (suiteRunner == null ) {
if (suite instanceof SuiteRunner) {
setExitCodeListener(((SuiteRunner) suite).getExitCodeListener());
}
} else {
setExitCodeListener(suiteRunner.getExitCodeListener());
}

boolean preserveOrder = test.getPreserveOrder();
IMethodInterceptor builtinInterceptor =
Expand Down Expand Up @@ -963,15 +974,18 @@ private void logStart() {
*/
private void fireEvent(boolean isStart) {
if (isStart) {
for (ITestListener itl : m_testListeners) {
for (ITestListener itl : ListenerOrderDeterminer.order(m_testListeners)) {
itl.onStart(this);
}
this.exitCodeListener.onStart(this);

} else {
List<ITestListener> testListenersReversed = Lists.newReversedArrayList(m_testListeners);
List<ITestListener> testListenersReversed =
ListenerOrderDeterminer.reversedOrder(m_testListeners);
for (ITestListener itl : testListenersReversed) {
itl.onFinish(this);
}
this.exitCodeListener.onFinish(this);
}
if (!isStart) {
MethodHelper.clear(methods(this.getPassedConfigurations()));
Expand Down Expand Up @@ -1238,6 +1252,15 @@ void addConfigurationListener(IConfigurationListener icl) {
}
}

private void setExitCodeListener(ITestListener exitCodeListener) {
this.exitCodeListener = exitCodeListener;
}

@Override
public ITestListener getExitCodeListener() {
return Objects.requireNonNull(exitCodeListener, "ExitCodeListener cannot be null.");
}

private void dumpInvokedMethods() {
MethodHelper.dumpInvokedMethodInfoToConsole(getAllTestMethods(), getVerbose());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package org.testng.internal;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.testng.collections.Lists;
import org.testng.internal.collections.Pair;

/**
* A Utility that helps us differentiate between a user's listener and an IDE Listener.
*
* <p>When dealing with TestNG listeners we would need to ensure that the user created listeners are
* invoked first followed by IDE listeners. This is required so that we always honour any state
* changes that a user's listener may have done to the internal state of objects that can affect the
* outcome of the execution (for e.g.,{@link org.testng.ITestResult})
*
* <p>The ordering must be done such that, when dealing with "beforeXXX|afterXXX" we group all the
* IDE listeners at the end. That way, we can always ensure that the IDE listeners always honour
* the changes that were done to the TestNG internal states and give a consistent experience for
* users in the IDE.
*/
public final class ListenerOrderDeterminer {

private ListenerOrderDeterminer() {
// Defeat instantiation
}

private static final List<String> IDE_PACKAGES =
RuntimeBehavior.getIdePackageNames().stream()
.map(each -> each.replaceAll("\\Q.*\\E", ""))
.collect(Collectors.toList());

private static final Predicate<Class<?>> IDE_LISTENER =
clazz -> IDE_PACKAGES.stream().anyMatch(each -> clazz.getPackage().getName().contains(each));

/**
* @param original - The original collection of listeners
* @return - A re-ordered collection wherein TestNG's known listeners are added at the end
*/
public static <T> List<T> order(Collection<T> original) {
Pair<List<T>, List<T>> ordered = arrange(original);
List<T> ideListeners = ordered.first();
List<T> regularListeners = ordered.second();
return Lists.merge(regularListeners, ideListeners);
}

/**
* @param original - The original collection of listeners
* @return - A reversed ordered list wherein the user listeners are found in reverse order
* followed by TestNG known listeners also in reverse order.
*/
public static <T> List<T> reversedOrder(Collection<T> original) {
Pair<List<T>, List<T>> ordered = arrange(original);
List<T> ideListeners = ordered.first();
List<T> regularListeners = ordered.second();
Collections.reverse(regularListeners);
Collections.reverse(ideListeners);
return Lists.merge(regularListeners, ideListeners);
}

private static boolean isIDEListener(Class<?> clazz) {
return IDE_LISTENER.test(clazz);
}

private static <T> Pair<List<T>, List<T>> arrange(Collection<T> original) {
List<T> ideListeners = new ArrayList<>();
List<T> regularListeners = new ArrayList<>();
original.stream()
.filter(Objects::nonNull)
.forEach(
each -> {
if (isIDEListener(each.getClass())) {
ideListeners.add(each);
} else {
regularListeners.add(each);
}
});
return new Pair<>(ideListeners, regularListeners);
}
}
Loading

0 comments on commit 9588843

Please sign in to comment.