From b937a1d0e95ee59174286add8bb59c3cf82b9ab7 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Mon, 9 May 2022 16:38:50 +0530 Subject: [PATCH] Wire-In listeners consistently Closes #2752 --- CHANGES.txt | 3 +- .../src/main/java/org/testng/TestRunner.java | 30 +++------- .../testng/internal/ClassBasedWrapper.java | 33 ++++++++++ .../test/listeners/ListenerInXmlTest.java | 4 +- .../java/test/listeners/ListenersTest.java | 39 ++++++++++++ .../listeners/issue2752/ListenerSample.java | 60 +++++++++++++++++++ .../listeners/issue2752/TestClassSample.java | 15 +++++ 7 files changed, 159 insertions(+), 25 deletions(-) create mode 100644 testng-core/src/main/java/org/testng/internal/ClassBasedWrapper.java create mode 100644 testng-core/src/test/java/test/listeners/issue2752/ListenerSample.java create mode 100644 testng-core/src/test/java/test/listeners/issue2752/TestClassSample.java diff --git a/CHANGES.txt b/CHANGES.txt index 42d60a6fe9..4a85e436d5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,7 @@ Current 7.6.0 -New: GITHUB-2724: DataProvider: possibility to unload dataprovider class, when done with it (Dzmitry Sankouski) +Fixed: GITHUB-2752: TestListener is being lost when implenting both IClassListener and ITestListener (Krishnan Mahadevan) +New: GITHUB-2724: DataProvider: possibility to unload dataprovider class, when done with it (Dzmitry Sankouski) Fixed: GITHUB-217: Configure TestNG to fail when there's a failure in data provider (Krishnan Mahadevan) Fixed: GITHUB-2743: SuiteRunner could not be initial by default Configuration (Nan Liang) Fixed: GITHUB-2729: beforeConfiguration() listener method should be invoked for skipped configurations as well(Nan Liang) diff --git a/testng-core/src/main/java/org/testng/TestRunner.java b/testng-core/src/main/java/org/testng/TestRunner.java index 05f3e9ac3b..7705db831d 100644 --- a/testng-core/src/main/java/org/testng/TestRunner.java +++ b/testng-core/src/main/java/org/testng/TestRunner.java @@ -17,12 +17,14 @@ import java.util.concurrent.PriorityBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nonnull; import org.testng.collections.Lists; import org.testng.collections.Maps; import org.testng.collections.Sets; import org.testng.internal.Attributes; +import org.testng.internal.ClassBasedWrapper; import org.testng.internal.ClassInfoMap; import org.testng.internal.ConfigurationGroupMethods; import org.testng.internal.DefaultListenerFactory; @@ -374,11 +376,7 @@ private void initListeners() { // Instantiate all the listeners for (Class c : listenerClasses) { - if (IClassListener.class.isAssignableFrom(c) && m_classListeners.containsKey(c)) { - continue; - } ITestNGListener listener = factory.createListener(c); - addListener(listener); } } @@ -1119,20 +1117,11 @@ public List getTestListeners() { @Override public List getConfigurationListeners() { - List listeners = Lists.newArrayList(m_configurationListeners); - for (IConfigurationListener each : this.m_configuration.getConfigurationListeners()) { - boolean duplicate = false; - for (IConfigurationListener listener : listeners) { - if (each.getClass().equals(listener.getClass())) { - duplicate = true; - break; - } - } - if (!duplicate) { - listeners.add(each); - } - } - return Lists.newArrayList(listeners); + return m_configurationListeners.stream() + .map(ClassBasedWrapper::wrap) + .distinct() + .map(ClassBasedWrapper::unWrap) + .collect(Collectors.toUnmodifiableList()); } private void logFailedTest(ITestResult tr, boolean withinSuccessPercentage) { @@ -1170,7 +1159,6 @@ void addTestListener(ITestListener listener) { } public void addListener(ITestNGListener listener) { - // TODO a listener may be added many times if it implements many interfaces if (listener instanceof IMethodInterceptor) { m_methodInterceptors.add((IMethodInterceptor) listener); } @@ -1180,9 +1168,7 @@ public void addListener(ITestNGListener listener) { } if (listener instanceof IClassListener) { IClassListener classListener = (IClassListener) listener; - if (!m_classListeners.containsKey(classListener.getClass())) { - m_classListeners.put(classListener.getClass(), classListener); - } + m_classListeners.putIfAbsent(classListener.getClass(), classListener); } if (listener instanceof IConfigurationListener) { addConfigurationListener((IConfigurationListener) listener); diff --git a/testng-core/src/main/java/org/testng/internal/ClassBasedWrapper.java b/testng-core/src/main/java/org/testng/internal/ClassBasedWrapper.java new file mode 100644 index 0000000000..a46c0675f2 --- /dev/null +++ b/testng-core/src/main/java/org/testng/internal/ClassBasedWrapper.java @@ -0,0 +1,33 @@ +package org.testng.internal; + +import java.util.Objects; + +public final class ClassBasedWrapper { + + private final T object; + + private ClassBasedWrapper(T object) { + this.object = object; + } + + public static ClassBasedWrapper wrap(T object) { + return new ClassBasedWrapper<>(object); + } + + public T unWrap() { + return object; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ClassBasedWrapper wrapper = (ClassBasedWrapper) o; + return object.getClass().equals(wrapper.getClass()); + } + + @Override + public int hashCode() { + return Objects.hash(object); + } +} diff --git a/testng-core/src/test/java/test/listeners/ListenerInXmlTest.java b/testng-core/src/test/java/test/listeners/ListenerInXmlTest.java index 871c7ed375..32d208895b 100644 --- a/testng-core/src/test/java/test/listeners/ListenerInXmlTest.java +++ b/testng-core/src/test/java/test/listeners/ListenerInXmlTest.java @@ -1,6 +1,6 @@ package test.listeners; -import java.util.Arrays; +import java.util.List; import org.testng.Assert; import org.testng.TestNG; import org.testng.annotations.Test; @@ -11,7 +11,7 @@ public class ListenerInXmlTest extends SimpleBaseTest { @Test(description = "Make sure that listeners defined in testng.xml are invoked") public void listenerInXmlShouldBeInvoked() { TestNG tng = create(); - tng.setTestSuites(Arrays.asList(getPathToResource("listener-in-xml.xml"))); + tng.setTestSuites(List.of(getPathToResource("listener-in-xml.xml"))); LListener.invoked = false; tng.run(); Assert.assertTrue(LListener.invoked); diff --git a/testng-core/src/test/java/test/listeners/ListenersTest.java b/testng-core/src/test/java/test/listeners/ListenersTest.java index aa094e1970..2c961d3954 100644 --- a/testng-core/src/test/java/test/listeners/ListenersTest.java +++ b/testng-core/src/test/java/test/listeners/ListenersTest.java @@ -8,6 +8,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.assertj.core.api.Assertions; import org.testng.TestNG; @@ -20,6 +21,8 @@ import test.listeners.issue2638.TestClassBSample; import test.listeners.issue2685.InterruptedTestSample; import test.listeners.issue2685.SampleTestFailureListener; +import test.listeners.issue2752.ListenerSample; +import test.listeners.issue2752.TestClassSample; public class ListenersTest extends SimpleBaseTest { @@ -67,6 +70,42 @@ public Object[][] getSuites() throws IOException { }; } + @Test(description = "GITHUB-2752") + public void testWiringInOfListenersInMultipleTestTagsWithNoListenerInSuite() { + setupTest(false); + List expected = + Arrays.asList( + "onStart", "onBeforeClass", "onTestStart", "onTestSuccess", "onAfterClass", "onFinish"); + Map> logs = ListenerSample.getLogs(); + assertThat(logs.get("Xml_Test_1")).containsAll(expected); + assertThat(logs.get("Xml_Test_2")).containsAll(expected); + } + + @Test(description = "GITHUB-2752") + public void testWiringInOfListenersInMultipleTestTagsWithListenerInSuite() { + setupTest(true); + List expected = + Arrays.asList( + "onStart", "onBeforeClass", "onTestStart", "onTestSuccess", "onAfterClass", "onFinish"); + Map> logs = ListenerSample.getLogs(); + assertThat(logs.get("Xml_Test_1")).containsAll(expected); + assertThat(logs.get("Xml_Test_2")).containsAll(expected); + } + + private void setupTest(boolean addExplicitListener) { + TestNG testng = new TestNG(); + XmlSuite xmlSuite = createXmlSuite("Xml_Suite"); + createXmlTest(xmlSuite, "Xml_Test_1", TestClassSample.class); + createXmlTest(xmlSuite, "Xml_Test_2", TestClassSample.class); + testng.setXmlSuites(Collections.singletonList(xmlSuite)); + testng.setVerbose(2); + if (addExplicitListener) { + ListenerSample listener = new ListenerSample(); + testng.addListener(listener); + } + testng.run(); + } + private static Map getExpectations() { Map expected = new HashMap<>(); expected.put("Container_Suite", new String[] {}); diff --git a/testng-core/src/test/java/test/listeners/issue2752/ListenerSample.java b/testng-core/src/test/java/test/listeners/issue2752/ListenerSample.java new file mode 100644 index 0000000000..492cf9062d --- /dev/null +++ b/testng-core/src/test/java/test/listeners/issue2752/ListenerSample.java @@ -0,0 +1,60 @@ +package test.listeners.issue2752; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import org.testng.IClassListener; +import org.testng.ITestClass; +import org.testng.ITestContext; +import org.testng.ITestListener; +import org.testng.ITestResult; + +public class ListenerSample implements IClassListener, ITestListener { + + private static final Map> logs = new ConcurrentHashMap<>(); + + public ListenerSample() { + logs.clear(); + } + + public static Map> getLogs() { + return logs; + } + + @Override + public void onBeforeClass(ITestClass testClass) { + String key = testClass.getXmlTest().getName(); + logs.computeIfAbsent(key, k -> new ArrayList<>()).add("onBeforeClass"); + } + + @Override + public void onAfterClass(ITestClass testClass) { + String key = testClass.getXmlTest().getName(); + logs.computeIfAbsent(key, k -> new ArrayList<>()).add("onAfterClass"); + } + + @Override + public void onTestStart(ITestResult result) { + String key = result.getTestContext().getName(); + logs.computeIfAbsent(key, k -> new ArrayList<>()).add("onTestStart"); + } + + @Override + public void onTestSuccess(ITestResult result) { + String key = result.getTestContext().getName(); + logs.computeIfAbsent(key, k -> new ArrayList<>()).add("onTestSuccess"); + } + + @Override + public void onFinish(ITestContext context) { + String key = context.getName(); + logs.computeIfAbsent(key, k -> new ArrayList<>()).add("onFinish"); + } + + @Override + public void onStart(ITestContext context) { + String key = context.getName(); + logs.computeIfAbsent(key, k -> new ArrayList<>()).add("onStart"); + } +} diff --git a/testng-core/src/test/java/test/listeners/issue2752/TestClassSample.java b/testng-core/src/test/java/test/listeners/issue2752/TestClassSample.java new file mode 100644 index 0000000000..bf33cf08b6 --- /dev/null +++ b/testng-core/src/test/java/test/listeners/issue2752/TestClassSample.java @@ -0,0 +1,15 @@ +package test.listeners.issue2752; + +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +@Listeners(ListenerSample.class) +public class TestClassSample { + + @Test + public void testMethod() {} + + @BeforeClass + public void beforeClass() {} +}