Skip to content

Commit

Permalink
Honour Insertion order for listeners
Browse files Browse the repository at this point in the history
  • Loading branch information
krmahadevan committed Aug 6, 2021
1 parent 68feee1 commit 888cf56
Show file tree
Hide file tree
Showing 16 changed files with 437 additions and 36 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-2558: Make IExecutionListener, ITestListener, IInvokedMethodListener, IConfigurationListener, ISuiteListener execute in the order of insertion (Krishnan Mahadevan)
Fixed: GITHUB-2611: Config Failures not included in testng-failed.xml when its part of a different test class (Krishnan Mahadevan)
Fixed: GITHUB-2613: Ignored Tests are not retrieved for a mixed test class (test with enabled, disabled and ignored test method) (Krishnan Mahadevan)
Fixed: GITHUB-849: Performance improvement by fixing hashCode (testn & Vladimir Sitnikov)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.testng.collections;

import java.util.Collections;
import java.util.HashMap;
import java.util.Hashtable;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -36,6 +37,10 @@ public static <K, V> Map<K, V> newLinkedHashMap() {
return new LinkedHashMap<>();
}

public static <K, V> Map<K, V> synchronizedLinkedHashMap() {
return Collections.synchronizedMap(newLinkedHashMap());
}

public static <K, V> Map<K, V> newHashMap(Map<K, V> parameters) {
return new HashMap<>(parameters);
}
Expand Down
42 changes: 18 additions & 24 deletions testng-core/src/main/java/org/testng/SuiteRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ public class SuiteRunner implements ISuite, IInvokedMethodListener {
Collections.synchronizedMap(Maps.newLinkedHashMap());
private final List<TestRunner> testRunners = Lists.newArrayList();
private final Map<Class<? extends ISuiteListener>, ISuiteListener> listeners =
Maps.newConcurrentMap();
Maps.newLinkedHashMap();

private String outputDir;
private XmlSuite xmlSuite;
private Injector parentInjector;

private final List<ITestListener> testListeners = Lists.newArrayList();
private final Map<Class<? extends IClassListener>, IClassListener> classListeners =
Maps.newConcurrentMap();
Maps.newLinkedHashMap();
private ITestRunnerFactory tmpRunnerFactory;
private final DataProviderHolder holder = new DataProviderHolder();

Expand Down Expand Up @@ -137,13 +137,16 @@ private void init(
Collection<IClassListener> classListeners,
DataProviderHolder attribs,
Comparator<ITestNGMethod> comparator) {
if (comparator == null) {
throw new IllegalArgumentException("comparator must not be null");
}
this.holder.merge(attribs);
this.configuration = configuration;
xmlSuite = suite;
this.xmlSuite = suite;
this.useDefaultListeners = useDefaultListeners;
tmpRunnerFactory = runnerFactory;
this.tmpRunnerFactory = runnerFactory;
List<IMethodInterceptor> localMethodInterceptors =
methodInterceptors != null ? methodInterceptors : Lists.newArrayList();
Optional.ofNullable(methodInterceptors).orElse(Lists.newArrayList());
setOutputDir(outputDir);
if (suite.getObjectFactoryClass() == null) {
objectFactory = configuration.getObjectFactory();
Expand Down Expand Up @@ -189,25 +192,20 @@ public <T> T newInstance(Constructor<T> constructor, Object... parameters) {
};
}
// Add our own IInvokedMethodListener
invokedMethodListeners = Maps.newConcurrentMap();
if (invokedMethodListener != null) {
for (IInvokedMethodListener listener : invokedMethodListener) {
invokedMethodListeners.put(listener.getClass(), listener);
}
invokedMethodListeners = Maps.synchronizedLinkedHashMap();
for (IInvokedMethodListener listener :
Optional.ofNullable(invokedMethodListener).orElse(Collections.emptyList())) {
invokedMethodListeners.put(listener.getClass(), listener);
}
invokedMethodListeners.put(getClass(), this);

skipFailedInvocationCounts = suite.skipFailedInvocationCounts();
if (null != testListeners) {
this.testListeners.addAll(testListeners);
}
if (null != classListeners) {
for (IClassListener classListener : classListeners) {
this.classListeners.put(classListener.getClass(), classListener);
}
}
if (comparator == null) {
throw new IllegalArgumentException("comparator must not be null");
for (IClassListener classListener :
Optional.ofNullable(classListeners).orElse(Collections.emptyList())) {
this.classListeners.put(classListener.getClass(), classListener);
}
ITestRunnerFactory iTestRunnerFactory = buildRunnerFactory(comparator);

Expand Down Expand Up @@ -254,7 +252,7 @@ public void setReportResults(boolean reportResults) {
}

private void invokeListeners(boolean start) {
for (ISuiteListener sl : listeners.values()) {
for (ISuiteListener sl : Lists.newArrayList(listeners.values())) {
if (start) {
sl.onStart(this);
} else {
Expand Down Expand Up @@ -466,9 +464,7 @@ public void run() {

/** @param reporter The ISuiteListener interested in reporting the result of the current suite. */
protected void addListener(ISuiteListener reporter) {
if (!listeners.containsKey(reporter.getClass())) {
listeners.put(reporter.getClass(), reporter);
}
listeners.putIfAbsent(reporter.getClass(), reporter);
}

@Override
Expand All @@ -491,9 +487,7 @@ public void addListener(ITestNGListener listener) {
}
if (listener instanceof IClassListener) {
IClassListener classListener = (IClassListener) listener;
if (!classListeners.containsKey(classListener.getClass())) {
classListeners.put(classListener.getClass(), classListener);
}
classListeners.putIfAbsent(classListener.getClass(), classListener);
}
if (listener instanceof IDataProviderListener) {
IDataProviderListener listenerObject = (IDataProviderListener) listener;
Expand Down
18 changes: 9 additions & 9 deletions testng-core/src/main/java/org/testng/TestNG.java
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,16 @@ public class TestNG {

// These listeners can be overridden from the command line
private final Map<Class<? extends IClassListener>, IClassListener> m_classListeners =
Maps.newHashMap();
Maps.newLinkedHashMap();
private final Map<Class<? extends ITestListener>, ITestListener> m_testListeners =
Maps.newHashMap();
Maps.newLinkedHashMap();
private final Map<Class<? extends ISuiteListener>, ISuiteListener> m_suiteListeners =
Maps.newHashMap();
private final Map<Class<? extends IReporter>, IReporter> m_reporters = Maps.newHashMap();
Maps.newLinkedHashMap();
private final Map<Class<? extends IReporter>, IReporter> m_reporters = Maps.newLinkedHashMap();
private final Map<Class<? extends IDataProviderListener>, IDataProviderListener>
m_dataProviderListeners = Maps.newHashMap();
m_dataProviderListeners = Maps.newLinkedHashMap();
private final Map<Class<? extends IDataProviderInterceptor>, IDataProviderInterceptor>
m_dataProviderInterceptors = Maps.newHashMap();
m_dataProviderInterceptors = Maps.newLinkedHashMap();

private IExecutorFactory m_executorFactory = null;

Expand All @@ -173,7 +173,7 @@ public class TestNG {
private ITestObjectFactory m_objectFactory = DEFAULT_OBJECT_FACTORY;

private final Map<Class<? extends IInvokedMethodListener>, IInvokedMethodListener>
m_invokedMethodListeners = Maps.newHashMap();
m_invokedMethodListeners = Maps.newLinkedHashMap();

private Integer m_dataProviderThreadCount = null;

Expand All @@ -190,15 +190,15 @@ public class TestNG {
protected long m_start;

private final Map<Class<? extends IAlterSuiteListener>, IAlterSuiteListener>
m_alterSuiteListeners = Maps.newHashMap();
m_alterSuiteListeners = Maps.newLinkedHashMap();

private boolean m_isInitialized = false;
private boolean isSuiteInitialized = false;
private final org.testng.internal.ExitCodeListener exitCodeListener =
new org.testng.internal.ExitCodeListener();
private ExitCode exitCode;
private final Map<Class<? extends IExecutionVisualiser>, IExecutionVisualiser>
m_executionVisualisers = Maps.newHashMap();
m_executionVisualisers = Maps.newLinkedHashMap();

/** Default constructor. Setting also usage of default listeners/reporters. */
public TestNG() {
Expand Down
2 changes: 1 addition & 1 deletion testng-core/src/main/java/org/testng/TestRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public class TestRunner

private Collection<IInvokedMethodListener> m_invokedMethodListeners = Lists.newArrayList();
private final Map<Class<? extends IClassListener>, IClassListener> m_classListeners =
Maps.newHashMap();
Maps.newLinkedHashMap();
private final DataProviderHolder holder = new DataProviderHolder();

private Date m_startDate = new Date();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ public class Configuration implements IConfiguration {
private IHookable m_hookable;
private IConfigurable m_configurable;
private final Map<Class<? extends IExecutionListener>, IExecutionListener> m_executionListeners =
Maps.newHashMap();
Maps.newLinkedHashMap();
private final Map<Class<? extends IConfigurationListener>, IConfigurationListener>
m_configurationListeners = Maps.newHashMap();
m_configurationListeners = Maps.newLinkedHashMap();
private boolean alwaysRunListeners = true;
private IExecutorFactory m_executorFactory = new DefaultThreadPoolExecutorFactory();

Expand Down
77 changes: 77 additions & 0 deletions testng-core/src/test/java/test/listeners/ListenerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
import test.listeners.github1029.Issue1029SampleTestClassWithFiveMethods;
import test.listeners.github1029.Issue1029SampleTestClassWithOneMethod;
import test.listeners.github1393.Listener1393;
import test.listeners.github2558.CallHolder;
import test.listeners.github2558.ClassMethodListenersHolder;
import test.listeners.github2558.ExecutionListenersHolder;
import test.listeners.github2558.ReportersHolder;
import test.listeners.github2558.SuiteAlterListenersHolder;
import test.listeners.github2558.SuiteListenersHolder;
import test.listeners.github2558.TestClassSamples;
import test.listeners.github2558.TestListenersHolder;
import test.listeners.github956.ListenerFor956;
import test.listeners.github956.TestClassContainer;
import test.listeners.issue1952.TestclassSample;
Expand All @@ -32,6 +40,7 @@ public class ListenerTest extends SimpleBaseTest {
@BeforeMethod
public void bm() {
SimpleListener.m_list = Lists.newArrayList();
CallHolder.clear();
}

@Test(
Expand Down Expand Up @@ -353,4 +362,72 @@ public void ensureDynamicListenerAdditionsDontTriggerConcurrentModificationExcep
testng.run();
assertThat(testng.getStatus()).isEqualTo(0);
}

@Test(description = "GITHUB-2558")
public void ensureInsertionOrderIsHonouredByListeners() {
String prefix = "test.listeners.github2558.";
String[] expectedOrder =
new String[] {
prefix + "ExecutionListenersHolder$ExecutionListenerB.onExecutionStart()",
prefix + "ExecutionListenersHolder$ExecutionListenerA.onExecutionStart()",
prefix + "SuiteAlterListenersHolder$SuiteAlterB.alter()",
prefix + "SuiteAlterListenersHolder$SuiteAlterA.alter()",
prefix + "SuiteListenersHolder$SuiteListenerB.onStart()",
prefix + "SuiteListenersHolder$SuiteListenerA.onStart()",
prefix + "TestListenersHolder$TestListenerB.onStart(ctx)",
prefix + "TestListenersHolder$TestListenerA.onStart(ctx)",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.onBeforeClass()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.onBeforeClass()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.beforeInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.beforeInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.afterInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.afterInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.onBeforeClass()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.onBeforeClass()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.onBeforeClass()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.onBeforeClass()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.beforeInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.beforeInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.afterInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.afterInvocation()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerB.onBeforeClass()",
prefix + "ClassMethodListenersHolder$ClassMethodListenerA.onBeforeClass()",
prefix + "TestListenersHolder$TestListenerB.onFinish(ctx)",
prefix + "TestListenersHolder$TestListenerA.onFinish(ctx)",
prefix + "SuiteListenersHolder$SuiteListenerB.onFinish()",
prefix + "SuiteListenersHolder$SuiteListenerA.onFinish()",
prefix + "ReportersHolder$ReporterB.generateReport()",
prefix + "ReportersHolder$ReporterA.generateReport()",
prefix + "ExecutionListenersHolder$ExecutionListenerB.onExecutionFinish()",
prefix + "ExecutionListenersHolder$ExecutionListenerA.onExecutionFinish()"
};
List<Class<?>> listeners =
Arrays.asList(
ExecutionListenersHolder.ExecutionListenerB.class,
SuiteAlterListenersHolder.SuiteAlterB.class,
SuiteListenersHolder.SuiteListenerB.class,
TestListenersHolder.TestListenerB.class,
ClassMethodListenersHolder.ClassMethodListenerB.class,
ReportersHolder.ReporterB.class,
ExecutionListenersHolder.ExecutionListenerA.class,
SuiteAlterListenersHolder.SuiteAlterA.class,
SuiteListenersHolder.SuiteListenerA.class,
TestListenersHolder.TestListenerA.class,
ClassMethodListenersHolder.ClassMethodListenerA.class,
ReportersHolder.ReporterA.class);

XmlSuite xmlSuite = new XmlSuite();
xmlSuite.setName("Random_Suite");
listeners.forEach(each -> xmlSuite.addListener(each.getName()));
createXmlTest(
xmlSuite,
"random_test",
TestClassSamples.TestClassSampleA.class,
TestClassSamples.TestClassSampleB.class);
TestNG testng = create(xmlSuite);
testng.setUseDefaultListeners(false);
testng.run();
List<String> actual = CallHolder.getCalls();
assertThat(actual).containsExactly(expectedOrder);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package test.listeners.github2558;

import java.util.List;
import org.assertj.core.util.Lists;

public class CallHolder {

private static final List<String> calls = Lists.newArrayList();

public static void addCall(String identifier) {
calls.add(identifier);
}

public static void clear() {
calls.clear();
}

public static List<String> getCalls() {
return calls;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package test.listeners.github2558;

import org.testng.IClassListener;
import org.testng.IInvokedMethod;
import org.testng.IInvokedMethodListener;
import org.testng.ITestClass;
import org.testng.ITestResult;

public class ClassMethodListenersHolder {

public static class ClassMethodListenerA implements IClassListener, IInvokedMethodListener {

@Override
public void onBeforeClass(ITestClass testClass) {
CallHolder.addCall(getClass().getName() + ".onBeforeClass()");
}

@Override
public void onAfterClass(ITestClass testClass) {
CallHolder.addCall(getClass().getName() + ".onBeforeClass()");
}

@Override
public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
if (method.isConfigurationMethod()) {
return;
}
CallHolder.addCall(getClass().getName() + ".beforeInvocation()");
}

@Override
public void afterInvocation(IInvokedMethod method, ITestResult testResult) {
if (method.isConfigurationMethod()) {
return;
}
CallHolder.addCall(getClass().getName() + ".afterInvocation()");
}
}

public static class ClassMethodListenerB implements IClassListener, IInvokedMethodListener {

@Override
public void onBeforeClass(ITestClass testClass) {
CallHolder.addCall(getClass().getName() + ".onBeforeClass()");
}

@Override
public void onAfterClass(ITestClass testClass) {
CallHolder.addCall(getClass().getName() + ".onBeforeClass()");
}

@Override
public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
if (method.isConfigurationMethod()) {
return;
}
CallHolder.addCall(getClass().getName() + ".beforeInvocation()");
}

@Override
public void afterInvocation(IInvokedMethod method, ITestResult testResult) {
if (method.isConfigurationMethod()) {
return;
}
CallHolder.addCall(getClass().getName() + ".afterInvocation()");
}
}
}
Loading

0 comments on commit 888cf56

Please sign in to comment.