From e8f17fe5b3f735ce107133e5860236c372049316 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Fri, 26 Nov 2021 09:42:52 +0530 Subject: [PATCH] Allow ITestObjectFactory injection via listeners (#2677) Closes #2676 --- CHANGES.txt | 1 + .../src/main/java/org/testng/SuiteRunner.java | 15 ++-- .../src/main/java/org/testng/TestNG.java | 3 +- .../src/main/java/org/testng/TestRunner.java | 2 +- .../java/org/testng/internal/ClassImpl.java | 13 +++- .../objects/DefaultTestObjectFactory.java | 12 +++ .../test/objectfactory/ObjectFactoryTest.java | 78 +++++++++++++++++++ .../github1131/GitHub1131Test.java | 51 ------------ .../github1827/GitHub1827Test.java | 19 ----- .../issue2676/LocalSuiteAlteringListener.java | 13 ++++ .../issue2676/LoggingObjectFactorySample.java | 16 ++++ .../issue2676/TestClassSample.java | 9 +++ testng-core/src/test/resources/testng.xml | 3 +- 13 files changed, 153 insertions(+), 82 deletions(-) create mode 100644 testng-core/src/main/java/org/testng/internal/objects/DefaultTestObjectFactory.java create mode 100644 testng-core/src/test/java/test/objectfactory/ObjectFactoryTest.java delete mode 100644 testng-core/src/test/java/test/objectfactory/github1131/GitHub1131Test.java delete mode 100644 testng-core/src/test/java/test/objectfactory/github1827/GitHub1827Test.java create mode 100644 testng-core/src/test/java/test/objectfactory/issue2676/LocalSuiteAlteringListener.java create mode 100644 testng-core/src/test/java/test/objectfactory/issue2676/LoggingObjectFactorySample.java create mode 100644 testng-core/src/test/java/test/objectfactory/issue2676/TestClassSample.java diff --git a/CHANGES.txt b/CHANGES.txt index ffea1f6ec5..9adb8da57e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-2676: NPE is triggered when working with ITestObjectFactory (Krishnan Mahadevan) Fixed: GITHUB-2674: Run onTestSkipped for each value from data provider (Krishnan Mahadevan) Fixed: GITHUB-2672: Log real stacktrace when test times out. (cdalexndr) Fixed: GITHUB-2669: A failed retry with ITestContext will lose the ITestContext. (Nan Liang) diff --git a/testng-core/src/main/java/org/testng/SuiteRunner.java b/testng-core/src/main/java/org/testng/SuiteRunner.java index 3de8c1af06..ea43e07542 100644 --- a/testng-core/src/main/java/org/testng/SuiteRunner.java +++ b/testng-core/src/main/java/org/testng/SuiteRunner.java @@ -155,6 +155,9 @@ private void init( !configuration.getObjectFactory().getClass().equals(suite.getObjectFactoryClass()); final ITestObjectFactory suiteObjectFactory; if (create) { + if (objectFactory == null) { + objectFactory = configuration.getObjectFactory(); + } // Dont keep creating the object factory repeatedly since our current object factory // Was already created based off of a suite level object factory. suiteObjectFactory = objectFactory.newInstance(suite.getObjectFactoryClass()); @@ -166,27 +169,27 @@ private void init( @Override public T newInstance(Class cls, Object... parameters) { try { - return configuration.getObjectFactory().newInstance(cls, parameters); - } catch (Exception e) { return suiteObjectFactory.newInstance(cls, parameters); + } catch (Exception e) { + return configuration.getObjectFactory().newInstance(cls, parameters); } } @Override public T newInstance(String clsName, Object... parameters) { try { - return configuration.getObjectFactory().newInstance(clsName, parameters); - } catch (Exception e) { return suiteObjectFactory.newInstance(clsName, parameters); + } catch (Exception e) { + return configuration.getObjectFactory().newInstance(clsName, parameters); } } @Override public T newInstance(Constructor constructor, Object... parameters) { try { - return configuration.getObjectFactory().newInstance(constructor, parameters); - } catch (Exception e) { return suiteObjectFactory.newInstance(constructor, parameters); + } catch (Exception e) { + return configuration.getObjectFactory().newInstance(constructor, parameters); } } }; diff --git a/testng-core/src/main/java/org/testng/TestNG.java b/testng-core/src/main/java/org/testng/TestNG.java index 158c224fdd..e9e1ab59b4 100644 --- a/testng-core/src/main/java/org/testng/TestNG.java +++ b/testng-core/src/main/java/org/testng/TestNG.java @@ -39,6 +39,7 @@ import org.testng.internal.annotations.JDK15AnnotationFinder; import org.testng.internal.invokers.SuiteRunnerMap; import org.testng.internal.invokers.objects.GuiceContext; +import org.testng.internal.objects.DefaultTestObjectFactory; import org.testng.internal.objects.Dispenser; import org.testng.internal.objects.IObjectDispenser; import org.testng.internal.objects.pojo.BasicAttributes; @@ -169,7 +170,7 @@ public class TestNG { private final Set m_selectors = Sets.newLinkedHashSet(); - private static final ITestObjectFactory DEFAULT_OBJECT_FACTORY = new ITestObjectFactory() {}; + private static final ITestObjectFactory DEFAULT_OBJECT_FACTORY = new DefaultTestObjectFactory(); private ITestObjectFactory m_objectFactory = DEFAULT_OBJECT_FACTORY; private final Map, IInvokedMethodListener> diff --git a/testng-core/src/main/java/org/testng/TestRunner.java b/testng-core/src/main/java/org/testng/TestRunner.java index 9680a3ed15..bfcab4b23a 100644 --- a/testng-core/src/main/java/org/testng/TestRunner.java +++ b/testng-core/src/main/java/org/testng/TestRunner.java @@ -240,7 +240,7 @@ private void init( m_host = suite.getHost(); m_testClassesFromXml = test.getXmlClasses(); m_injectorFactory = m_configuration.getInjectorFactory(); - m_objectFactory = m_configuration.getObjectFactory(); + m_objectFactory = suite.getObjectFactory(); setVerbose(test.getVerbose()); boolean preserveOrder = test.getPreserveOrder(); diff --git a/testng-core/src/main/java/org/testng/internal/ClassImpl.java b/testng-core/src/main/java/org/testng/internal/ClassImpl.java index a76d938166..287436f87c 100644 --- a/testng-core/src/main/java/org/testng/internal/ClassImpl.java +++ b/testng-core/src/main/java/org/testng/internal/ClassImpl.java @@ -10,6 +10,7 @@ import org.testng.collections.Lists; import org.testng.collections.Objects; import org.testng.internal.annotations.IAnnotationFinder; +import org.testng.internal.objects.DefaultTestObjectFactory; import org.testng.internal.objects.Dispenser; import org.testng.internal.objects.IObjectDispenser; import org.testng.internal.objects.pojo.BasicAttributes; @@ -94,7 +95,11 @@ private Object getDefaultInstance(boolean create, String errMsgPrefix) { if (m_instance != null) { m_defaultInstance = m_instance; } else { - IObjectDispenser dispenser = Dispenser.newInstance(m_objectFactory); + ITestObjectFactory factory = m_objectFactory; + if (factory instanceof DefaultTestObjectFactory) { + factory = m_testContext.getSuite().getObjectFactory(); + } + IObjectDispenser dispenser = Dispenser.newInstance(factory); BasicAttributes basic = new BasicAttributes(this, null); DetailedAttributes detailed = newDetailedAttributes(create, errMsgPrefix); CreationAttributes attributes = new CreationAttributes(m_testContext, basic, detailed); @@ -118,7 +123,11 @@ public Object[] getInstances(boolean create, String errorMsgPrefix) { if (create) { DetailedAttributes ea = newDetailedAttributes(create, errorMsgPrefix); CreationAttributes attributes = new CreationAttributes(m_testContext, null, ea); - result = new Object[] {Dispenser.newInstance(m_objectFactory).dispense(attributes)}; + result = + new Object[] { + Dispenser.newInstance(m_testContext.getSuite().getObjectFactory()) + .dispense(attributes) + }; } } if (m_instances.size() > 0) { diff --git a/testng-core/src/main/java/org/testng/internal/objects/DefaultTestObjectFactory.java b/testng-core/src/main/java/org/testng/internal/objects/DefaultTestObjectFactory.java new file mode 100644 index 0000000000..e107318b38 --- /dev/null +++ b/testng-core/src/main/java/org/testng/internal/objects/DefaultTestObjectFactory.java @@ -0,0 +1,12 @@ +package org.testng.internal.objects; + +import org.testng.ITestObjectFactory; + +/** + * Intended to be the default way of instantiating objects within TestNG. Intentionally does not + * provide any specific implementation because the interface already defines the default behavior. + * This class still exists to ensure that we dont use an anonymous object instantiation so that its + * easy to find out what type of object factory is being injected into our object dispensing + * mechanisms. + */ +public class DefaultTestObjectFactory implements ITestObjectFactory {} diff --git a/testng-core/src/test/java/test/objectfactory/ObjectFactoryTest.java b/testng-core/src/test/java/test/objectfactory/ObjectFactoryTest.java new file mode 100644 index 0000000000..263b67dbc3 --- /dev/null +++ b/testng-core/src/test/java/test/objectfactory/ObjectFactoryTest.java @@ -0,0 +1,78 @@ +package test.objectfactory; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.testng.TestNG; +import org.testng.TestNGException; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import org.testng.xml.XmlSuite; +import test.SimpleBaseTest; +import test.objectfactory.github1131.EmptyConstructorSample; +import test.objectfactory.github1131.IntConstructorSample; +import test.objectfactory.github1131.MyObjectFactory; +import test.objectfactory.github1131.StringConstructorSample; +import test.objectfactory.github1827.GitHub1827Sample; +import test.objectfactory.issue2676.LocalSuiteAlteringListener; +import test.objectfactory.issue2676.LoggingObjectFactorySample; +import test.objectfactory.issue2676.TestClassSample; + +public class ObjectFactoryTest extends SimpleBaseTest { + + @Test(description = "GITHUB-2676") + public void ensureObjectFactoryIsInvokedWhenAddedViaListeners() { + TestNG testng = create(TestClassSample.class); + testng.addListener(new LocalSuiteAlteringListener()); + testng.run(); + assertThat(LoggingObjectFactorySample.wasInvoked).isTrue(); + } + + @Test( + expectedExceptions = TestNGException.class, + expectedExceptionsMessageRegExp = ".*Check to make sure it can be instantiated", + description = "GITHUB-1827") + public void ensureExceptionThrownWhenNoSuitableConstructorFound() { + + TestNG testng = create(GitHub1827Sample.class); + testng.run(); + } + + @Test(dataProvider = "dp", description = "GITHUB-1131") + public void factoryWithEmptyConstructorShouldWork(boolean bool) { + testFactory(bool, EmptyConstructorSample.class); + assertThat(MyObjectFactory.allParams).containsExactly(new Object[] {}, new Object[] {}); + } + + @Test(dataProvider = "dp", description = "GITHUB-1131") + public void factoryWithIntConstructorShouldWork(boolean bool) { + testFactory(bool, IntConstructorSample.class); + assertThat(MyObjectFactory.allParams).containsExactly(new Object[] {1}, new Object[] {2}); + } + + @Test(dataProvider = "dp", description = "GITHUB-1131") + public void factoryWithStringConstructorShouldWork(boolean bool) { + testFactory(bool, StringConstructorSample.class); + assertThat(MyObjectFactory.allParams) + .containsExactly(new Object[] {"foo"}, new Object[] {"bar"}); + } + + private void testFactory(boolean onSuite, Class sample) { + MyObjectFactory.allParams.clear(); + + XmlSuite suite = createXmlSuite("Test IObjectFactory2", "TmpTest", sample); + TestNG tng = create(suite); + + if (onSuite) { + suite.setObjectFactoryClass(MyObjectFactory.class); + } else { + tng.setObjectFactory(MyObjectFactory.class); + } + + tng.run(); + } + + @DataProvider + public static Object[][] dp() { + return new Object[][] {new Object[] {true}, new Object[] {false}}; + } +} diff --git a/testng-core/src/test/java/test/objectfactory/github1131/GitHub1131Test.java b/testng-core/src/test/java/test/objectfactory/github1131/GitHub1131Test.java deleted file mode 100644 index b4486d8240..0000000000 --- a/testng-core/src/test/java/test/objectfactory/github1131/GitHub1131Test.java +++ /dev/null @@ -1,51 +0,0 @@ -package test.objectfactory.github1131; - -import static org.assertj.core.api.Assertions.assertThat; - -import org.testng.TestNG; -import org.testng.annotations.DataProvider; -import org.testng.annotations.Test; -import org.testng.xml.XmlSuite; -import test.SimpleBaseTest; - -public class GitHub1131Test extends SimpleBaseTest { - - private void testFactory(boolean onSuite, Class sample) { - MyObjectFactory.allParams.clear(); - - XmlSuite suite = createXmlSuite("Test IObjectFactory2", "TmpTest", sample); - TestNG tng = create(suite); - - if (onSuite) { - suite.setObjectFactoryClass(MyObjectFactory.class); - } else { - tng.setObjectFactory(MyObjectFactory.class); - } - - tng.run(); - } - - @DataProvider - public static Object[][] dp() { - return new Object[][] {new Object[] {true}, new Object[] {false}}; - } - - @Test(dataProvider = "dp") - public void factoryWithEmptyConstructorShouldWork(boolean bool) { - testFactory(bool, EmptyConstructorSample.class); - assertThat(MyObjectFactory.allParams).isEmpty(); - } - - @Test(dataProvider = "dp") - public void factoryWithIntConstructorShouldWork(boolean bool) { - testFactory(bool, IntConstructorSample.class); - assertThat(MyObjectFactory.allParams).containsExactly(new Object[] {1}, new Object[] {2}); - } - - @Test(dataProvider = "dp") - public void factoryWithStringConstructorShouldWork(boolean bool) { - testFactory(bool, StringConstructorSample.class); - assertThat(MyObjectFactory.allParams) - .containsExactly(new Object[] {"foo"}, new Object[] {"bar"}); - } -} diff --git a/testng-core/src/test/java/test/objectfactory/github1827/GitHub1827Test.java b/testng-core/src/test/java/test/objectfactory/github1827/GitHub1827Test.java deleted file mode 100644 index 7a792e1aff..0000000000 --- a/testng-core/src/test/java/test/objectfactory/github1827/GitHub1827Test.java +++ /dev/null @@ -1,19 +0,0 @@ -package test.objectfactory.github1827; - -import org.testng.TestNG; -import org.testng.TestNGException; -import org.testng.annotations.Test; -import test.SimpleBaseTest; - -public class GitHub1827Test extends SimpleBaseTest { - - @Test( - expectedExceptions = TestNGException.class, - expectedExceptionsMessageRegExp = ".*Check to make sure it can be instantiated", - description = "GITHUB-1827") - public void ensureExceptionThrownWhenNoSuitableConstructorFound() { - - TestNG testng = create(GitHub1827Sample.class); - testng.run(); - } -} diff --git a/testng-core/src/test/java/test/objectfactory/issue2676/LocalSuiteAlteringListener.java b/testng-core/src/test/java/test/objectfactory/issue2676/LocalSuiteAlteringListener.java new file mode 100644 index 0000000000..c24ad891c4 --- /dev/null +++ b/testng-core/src/test/java/test/objectfactory/issue2676/LocalSuiteAlteringListener.java @@ -0,0 +1,13 @@ +package test.objectfactory.issue2676; + +import java.util.List; +import org.testng.IAlterSuiteListener; +import org.testng.xml.XmlSuite; + +public class LocalSuiteAlteringListener implements IAlterSuiteListener { + + @Override + public void alter(List suites) { + suites.forEach(each -> each.setObjectFactoryClass(LoggingObjectFactorySample.class)); + } +} diff --git a/testng-core/src/test/java/test/objectfactory/issue2676/LoggingObjectFactorySample.java b/testng-core/src/test/java/test/objectfactory/issue2676/LoggingObjectFactorySample.java new file mode 100644 index 0000000000..e596ac5b7e --- /dev/null +++ b/testng-core/src/test/java/test/objectfactory/issue2676/LoggingObjectFactorySample.java @@ -0,0 +1,16 @@ +package test.objectfactory.issue2676; + +import java.lang.reflect.Constructor; +import org.testng.ITestObjectFactory; +import org.testng.internal.objects.InstanceCreator; + +public class LoggingObjectFactorySample implements ITestObjectFactory { + + public static boolean wasInvoked = false; + + @Override + public T newInstance(Constructor constructor, Object... parameters) { + wasInvoked = true; + return InstanceCreator.newInstance(constructor, parameters); + } +} diff --git a/testng-core/src/test/java/test/objectfactory/issue2676/TestClassSample.java b/testng-core/src/test/java/test/objectfactory/issue2676/TestClassSample.java new file mode 100644 index 0000000000..3c84a97cc5 --- /dev/null +++ b/testng-core/src/test/java/test/objectfactory/issue2676/TestClassSample.java @@ -0,0 +1,9 @@ +package test.objectfactory.issue2676; + +import org.testng.annotations.Test; + +public class TestClassSample { + + @Test + public void sampleTestMethod() {} +} diff --git a/testng-core/src/test/resources/testng.xml b/testng-core/src/test/resources/testng.xml index 4559f414c8..d2a428da7e 100644 --- a/testng-core/src/test/resources/testng.xml +++ b/testng-core/src/test/resources/testng.xml @@ -207,7 +207,7 @@ - + @@ -432,7 +432,6 @@ -