From 69dc23237859f63164958e82a0d0364c36047768 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Tue, 19 Mar 2024 13:29:54 +0530 Subject: [PATCH] Honour inheritance when parsing listener factories Closes #3095 --- CHANGES.txt | 1 + .../testng/internal/TestListenerHelper.java | 65 +++++++++++-------- .../internal/TestListenerHelperTest.java | 22 +++++-- .../listeners/ListenerFactoryContainer.java | 21 ++++++ .../listeners/TestClassContainer.java | 17 +++++ .../TestClassWithCompositeListener.java | 2 +- ...estClassWithMultipleListenerFactories.java | 6 -- .../java/test/listeners/ListenersTest.java | 8 +++ .../listeners/issue3095/ChildClassSample.java | 8 +++ .../listeners/issue3095/MyTestNgFactory.java} | 7 +- .../listeners/issue3095/SuperClassSample.java | 6 ++ 11 files changed, 119 insertions(+), 44 deletions(-) create mode 100644 testng-core/src/test/java/org/testng/internal/listeners/ListenerFactoryContainer.java create mode 100644 testng-core/src/test/java/org/testng/internal/listeners/TestClassContainer.java delete mode 100644 testng-core/src/test/java/org/testng/internal/listeners/TestClassWithMultipleListenerFactories.java create mode 100644 testng-core/src/test/java/test/listeners/issue3095/ChildClassSample.java rename testng-core/src/test/java/{org/testng/internal/listeners/DummyListenerFactory.java => test/listeners/issue3095/MyTestNgFactory.java} (50%) create mode 100644 testng-core/src/test/java/test/listeners/issue3095/SuperClassSample.java diff --git a/CHANGES.txt b/CHANGES.txt index ea5e832bc9..8a80ab8d48 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,6 @@ Current (7.10.0) +Fixed: GITHUB-3095: Super class annotated with ITestNGListenerFactory makes derived test class throw TestNGException on execution (Krishnan Mahadevan) Fixed: GITHUB-3081: Discrepancy with combination of (Shared Thread pool and Method Interceptor) (Krishnan Mahadevan) Fixed: GITHUB-2381: Controlling the inclusion of the listener at runtime (Krishnan Mahadevan) Fixed: GITHUB-3082: IInvokedMethodListener Iinvoked method does not return correct instance during @BeforeMethod, @AfterMethod and @AfterClass (Krishnan Mahadevan) diff --git a/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java b/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java index d372924d9a..c585193ab7 100644 --- a/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java +++ b/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java @@ -1,6 +1,9 @@ package org.testng.internal; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Optional; import org.testng.IClass; import org.testng.IConfigurationListener; import org.testng.ITestContext; @@ -13,7 +16,6 @@ import org.testng.ListenerComparator; import org.testng.TestNGException; import org.testng.annotations.IListenersAnnotation; -import org.testng.collections.Lists; import org.testng.internal.annotations.IAnnotationFinder; /** A helper class that internally houses some of the listener related actions support. */ @@ -129,38 +131,16 @@ public static void runTestListeners(ITestResult tr, List listener * @return all the @Listeners annotations found in the current class and its superclasses and * inherited interfaces. */ - @SuppressWarnings("unchecked") public static ListenerHolder findAllListeners(Class cls, IAnnotationFinder finder) { ListenerHolder result = new ListenerHolder(); - result.listenerClasses = Lists.newArrayList(); while (cls != Object.class) { List annotations = finder.findInheritedAnnotations(cls, IListenersAnnotation.class); - IListenersAnnotation l = finder.findAnnotation(cls, IListenersAnnotation.class); - if (l != null) { - annotations.add(l); - } - annotations.forEach( - anno -> { - Class[] classes = anno.getValue(); - for (Class c : classes) { - result.listenerClasses.add(c); - - if (ITestNGListenerFactory.class.isAssignableFrom(c)) { - if (result.listenerFactoryClass == null) { - result.listenerFactoryClass = (Class) c; - } else { - throw new TestNGException( - "Found more than one class implementing " - + "ITestNGListenerFactory:" - + c - + " and " - + result.listenerFactoryClass); - } - } - } - }); + Optional.ofNullable(finder.findAnnotation(cls, IListenersAnnotation.class)) + .ifPresent(annotations::add); + + annotations.stream().flatMap(it -> Arrays.stream(it.getValue())).forEach(result::addListener); cls = cls.getSuperclass(); } return result; @@ -196,9 +176,38 @@ public static ITestNGListenerFactory createListenerFactory( } public static class ListenerHolder { - private List> listenerClasses; + private final List> listenerClasses = new ArrayList<>(); private Class listenerFactoryClass; + @SuppressWarnings("unchecked") + public void addListener(Class c) { + // @Listener annotation is now inheritable. So let's add it ONLY + // if it wasn't added already + if (!listenerClasses.contains(c)) { + listenerClasses.add(c); + } + if (ITestNGListenerFactory.class.isAssignableFrom(c)) { + setListenerFactoryClass((Class) c); + } + } + + private void setListenerFactoryClass(Class c) { + if (c.equals(listenerFactoryClass)) { + return; + } + if (listenerFactoryClass != null) { + // Let's say we already know of a ListenerFactoryClass called `MyFactory`. + // Now we are stumbling into another ListenerFactoryClass called `YourFactory` + // We need to throw an exception, because we can ONLY deal with 1 listener factory. + throw new TestNGException( + "Found more than one class implementing ITestNGListenerFactory:" + + c + + " and " + + listenerFactoryClass); + } + listenerFactoryClass = c; + } + public List> getListenerClasses() { return listenerClasses; } diff --git a/testng-core/src/test/java/org/testng/internal/TestListenerHelperTest.java b/testng-core/src/test/java/org/testng/internal/TestListenerHelperTest.java index 66f008adc2..79c60742b1 100644 --- a/testng-core/src/test/java/org/testng/internal/TestListenerHelperTest.java +++ b/testng-core/src/test/java/org/testng/internal/TestListenerHelperTest.java @@ -14,11 +14,11 @@ import org.testng.internal.annotations.DefaultAnnotationTransformer; import org.testng.internal.annotations.IAnnotationFinder; import org.testng.internal.annotations.JDK15AnnotationFinder; -import org.testng.internal.listeners.DummyListenerFactory; +import org.testng.internal.listeners.ListenerFactoryContainer; +import org.testng.internal.listeners.TestClassContainer; import org.testng.internal.listeners.TestClassDoublingupAsListenerFactory; import org.testng.internal.listeners.TestClassWithCompositeListener; import org.testng.internal.listeners.TestClassWithListener; -import org.testng.internal.listeners.TestClassWithMultipleListenerFactories; import org.testng.internal.paramhandler.FakeTestContext; import org.testng.xml.XmlClass; import test.SimpleBaseTest; @@ -45,13 +45,25 @@ public Object[][] getTestData() { }; } + @Test(description = "GITHUB-3095") + public void testFindAllListenersDuplicateListenerFactories() { + TestListenerHelper.ListenerHolder result = + TestListenerHelper.findAllListeners( + TestClassContainer.TestClassWithDuplicateListenerFactories.class, finder); + assertThat(result.getListenerFactoryClass()).isNotNull(); + } + @Test( + description = "GITHUB-3095", expectedExceptions = TestNGException.class, expectedExceptionsMessageRegExp = "\nFound more than one class implementing ITestNGListenerFactory:class " - + "org.testng.internal.listeners.DummyListenerFactory and class org.testng.internal.listeners.DummyListenerFactory") + + "org.testng.internal.listeners.ListenerFactoryContainer\\$DummyListenerFactory2 " + + "and class org.testng.internal.listeners" + + ".ListenerFactoryContainer\\$DummyListenerFactory") public void testFindAllListenersErrorCondition() { - TestListenerHelper.findAllListeners(TestClassWithMultipleListenerFactories.class, finder); + TestListenerHelper.findAllListeners( + TestClassContainer.TestClassWithMultipleUniqueListenerFactories.class, finder); } @Test(dataProvider = "getFactoryTestData") @@ -73,7 +85,7 @@ public void testCreateListenerFactory( @DataProvider(name = "getFactoryTestData") public Object[][] getFactoryTestData() { return new Object[][] { - {TestClassWithListener.class, DummyListenerFactory.class}, + {TestClassWithListener.class, ListenerFactoryContainer.DummyListenerFactory.class}, {TestClassDoublingupAsListenerFactory.class, TestClassDoublingupAsListenerFactory.class} }; } diff --git a/testng-core/src/test/java/org/testng/internal/listeners/ListenerFactoryContainer.java b/testng-core/src/test/java/org/testng/internal/listeners/ListenerFactoryContainer.java new file mode 100644 index 0000000000..6592298c37 --- /dev/null +++ b/testng-core/src/test/java/org/testng/internal/listeners/ListenerFactoryContainer.java @@ -0,0 +1,21 @@ +package org.testng.internal.listeners; + +import org.testng.IExecutionListener; +import org.testng.ITestNGListener; +import org.testng.ITestNGListenerFactory; + +public class ListenerFactoryContainer { + public static class DummyListenerFactory implements ITestNGListenerFactory, IExecutionListener { + @Override + public ITestNGListener createListener(Class listenerClass) { + return this; + } + } + + public static class DummyListenerFactory2 implements ITestNGListenerFactory, IExecutionListener { + @Override + public ITestNGListener createListener(Class listenerClass) { + return this; + } + } +} diff --git a/testng-core/src/test/java/org/testng/internal/listeners/TestClassContainer.java b/testng-core/src/test/java/org/testng/internal/listeners/TestClassContainer.java new file mode 100644 index 0000000000..95de7e1adc --- /dev/null +++ b/testng-core/src/test/java/org/testng/internal/listeners/TestClassContainer.java @@ -0,0 +1,17 @@ +package org.testng.internal.listeners; + +import org.testng.annotations.Listeners; + +public class TestClassContainer { + @Listeners({ + ListenerFactoryContainer.DummyListenerFactory.class, + ListenerFactoryContainer.DummyListenerFactory.class + }) + public static class TestClassWithDuplicateListenerFactories {} + + @Listeners({ + ListenerFactoryContainer.DummyListenerFactory.class, + ListenerFactoryContainer.DummyListenerFactory2.class + }) + public static class TestClassWithMultipleUniqueListenerFactories {} +} diff --git a/testng-core/src/test/java/org/testng/internal/listeners/TestClassWithCompositeListener.java b/testng-core/src/test/java/org/testng/internal/listeners/TestClassWithCompositeListener.java index 4be09ce05c..c7337a5e5a 100644 --- a/testng-core/src/test/java/org/testng/internal/listeners/TestClassWithCompositeListener.java +++ b/testng-core/src/test/java/org/testng/internal/listeners/TestClassWithCompositeListener.java @@ -3,7 +3,7 @@ import org.testng.annotations.Listeners; import org.testng.annotations.Test; -@Listeners(DummyListenerFactory.class) +@Listeners(ListenerFactoryContainer.DummyListenerFactory.class) public class TestClassWithCompositeListener { @Test public void testMethod() {} diff --git a/testng-core/src/test/java/org/testng/internal/listeners/TestClassWithMultipleListenerFactories.java b/testng-core/src/test/java/org/testng/internal/listeners/TestClassWithMultipleListenerFactories.java deleted file mode 100644 index bc8200c052..0000000000 --- a/testng-core/src/test/java/org/testng/internal/listeners/TestClassWithMultipleListenerFactories.java +++ /dev/null @@ -1,6 +0,0 @@ -package org.testng.internal.listeners; - -import org.testng.annotations.Listeners; - -@Listeners({DummyListenerFactory.class, DummyListenerFactory.class}) -public class TestClassWithMultipleListenerFactories {} diff --git a/testng-core/src/test/java/test/listeners/ListenersTest.java b/testng-core/src/test/java/test/listeners/ListenersTest.java index 70e5235d1e..9a19111017 100644 --- a/testng-core/src/test/java/test/listeners/ListenersTest.java +++ b/testng-core/src/test/java/test/listeners/ListenersTest.java @@ -54,6 +54,7 @@ import test.listeners.issue3064.SampleTestCase; import test.listeners.issue3082.ObjectRepository; import test.listeners.issue3082.ObjectTrackingMethodListener; +import test.listeners.issue3095.ChildClassSample; public class ListenersTest extends SimpleBaseTest { public static final String[] github2638ExpectedList = @@ -588,6 +589,13 @@ public void ensureListenerWorksWithCorrectTestClassInstance() { softly.assertAll(); } + @Test(description = "GITHUB-3095") + public void ensureInheritanceIsHandledWhenDealingWithListeners() { + TestNG testng = create(ChildClassSample.class); + testng.run(); + assertThat(testng.getStatus()).isZero(); + } + private void setupTest(boolean addExplicitListener) { TestNG testng = new TestNG(); XmlSuite xmlSuite = createXmlSuite("Xml_Suite"); diff --git a/testng-core/src/test/java/test/listeners/issue3095/ChildClassSample.java b/testng-core/src/test/java/test/listeners/issue3095/ChildClassSample.java new file mode 100644 index 0000000000..ab9ce95f35 --- /dev/null +++ b/testng-core/src/test/java/test/listeners/issue3095/ChildClassSample.java @@ -0,0 +1,8 @@ +package test.listeners.issue3095; + +import org.testng.annotations.Test; + +public class ChildClassSample extends SuperClassSample { + @Test + public void childTestMethod() {} +} diff --git a/testng-core/src/test/java/org/testng/internal/listeners/DummyListenerFactory.java b/testng-core/src/test/java/test/listeners/issue3095/MyTestNgFactory.java similarity index 50% rename from testng-core/src/test/java/org/testng/internal/listeners/DummyListenerFactory.java rename to testng-core/src/test/java/test/listeners/issue3095/MyTestNgFactory.java index fdd1874c83..9baebc9219 100644 --- a/testng-core/src/test/java/org/testng/internal/listeners/DummyListenerFactory.java +++ b/testng-core/src/test/java/test/listeners/issue3095/MyTestNgFactory.java @@ -1,12 +1,11 @@ -package org.testng.internal.listeners; +package test.listeners.issue3095; -import org.testng.IExecutionListener; import org.testng.ITestNGListener; import org.testng.ITestNGListenerFactory; -public class DummyListenerFactory implements ITestNGListenerFactory, IExecutionListener { +public class MyTestNgFactory implements ITestNGListener, ITestNGListenerFactory { @Override public ITestNGListener createListener(Class listenerClass) { - return this; + return null; } } diff --git a/testng-core/src/test/java/test/listeners/issue3095/SuperClassSample.java b/testng-core/src/test/java/test/listeners/issue3095/SuperClassSample.java new file mode 100644 index 0000000000..b2bd09a5c2 --- /dev/null +++ b/testng-core/src/test/java/test/listeners/issue3095/SuperClassSample.java @@ -0,0 +1,6 @@ +package test.listeners.issue3095; + +import org.testng.annotations.Listeners; + +@Listeners(MyTestNgFactory.class) +public class SuperClassSample {}