Running Test Classes with Inherited @Factory and @DataProvider Annotated Non-Static Methods Fail

[mbach979]

TestNG Version

7.6.1

Expected behavior

Tests with inherited @factory and @dataProvider annotated methods (non-static) from an abstract class fail while trying to instantiate the DataProvider.

public class InheritedTestFactoryAndDataProvider
{
	public static abstract class AbstractTestClassGenerator
	{
		@DataProvider(name="dataProvider")
		public Object[] dataProvider()
		{
			return new String[]{ "foo", "bar" };
		}
		
		@Factory(dataProvider="dataProvider")
		public Object[] testFactory(String value)
		{
			return new Object[] { new TestClass() };
		}
	}
	
	public static class TestClassGenerator extends AbstractTestClassGenerator
	{
		// assume other configuration etc. is happening here
	}
	
	public static class TestClass 
	{
		@Test
		public void test()
		{
		}
	}
	
}

Actual behavior

AFAICS the proceeding should work according to the docs/javadoc etc.

However this produces the following error:

Cannot instantiate class InheritedTestFactoryAndDataProvider$AbstractTestClassGenerator
	at org.testng.internal.objects.InstanceCreator.newInstance(InstanceCreator.java:41)
	at org.testng.internal.objects.InstanceCreator.newInstance(InstanceCreator.java:59)
	at org.testng.ITestObjectFactory.newInstance(ITestObjectFactory.java:10)
	at org.testng.internal.objects.SimpleObjectDispenser.dispense(SimpleObjectDispenser.java:60)
	at org.testng.internal.objects.GuiceBasedObjectDispenser.dispense(GuiceBasedObjectDispenser.java:28)
	at org.testng.internal.Parameters.findDataProvider(Parameters.java:626)
	at org.testng.internal.Parameters.findDataProvider(Parameters.java:510)
	at org.testng.internal.Parameters.handleParameters(Parameters.java:763)
	at org.testng.internal.FactoryMethod.invoke(FactoryMethod.java:160)
	at org.testng.internal.TestNGClassFinder.processFactory(TestNGClassFinder.java:174)
	at org.testng.internal.TestNGClassFinder.processMethod(TestNGClassFinder.java:138)
	at org.testng.internal.TestNGClassFinder.processClass(TestNGClassFinder.java:129)
	at org.testng.internal.TestNGClassFinder.<init>(TestNGClassFinder.java:67)
	at org.testng.TestRunner.initMethods(TestRunner.java:441)
	at org.testng.TestRunner.init(TestRunner.java:335)
	at org.testng.TestRunner.init(TestRunner.java:288)
	at org.testng.TestRunner.<init>(TestRunner.java:218)
	at org.testng.remote.support.RemoteTestNG6_12$1.newTestRunner(RemoteTestNG6_12.java:33)
	at org.testng.remote.support.RemoteTestNG6_12$DelegatingTestRunnerFactory.newTestRunner(RemoteTestNG6_12.java:66)
	at org.testng.ITestRunnerFactory.newTestRunner(ITestRunnerFactory.java:52)
	at org.testng.SuiteRunner$ProxyTestRunnerFactory.newTestRunner(SuiteRunner.java:706)
	at org.testng.SuiteRunner.init(SuiteRunner.java:225)
	at org.testng.SuiteRunner.<init>(SuiteRunner.java:115)
	at org.testng.TestNG.createSuiteRunner(TestNG.java:1349)
	at org.testng.TestNG.createSuiteRunners(TestNG.java:1325)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1167)
	at org.testng.TestNG.runSuites(TestNG.java:1099)
	at org.testng.TestNG.run(TestNG.java:1067)
	at org.testng.remote.AbstractRemoteTestNG.run(AbstractRemoteTestNG.java:115)
	at org.testng.remote.RemoteTestNG.initAndRun(RemoteTestNG.java:251)
	at org.testng.remote.RemoteTestNG.main(RemoteTestNG.java:77)
Caused by: java.lang.InstantiationException
	at java.base/jdk.internal.reflect.InstantiationExceptionConstructorAccessorImpl.newInstance(InstantiationExceptionConstructorAccessorImpl.java:48)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at org.testng.internal.objects.InstanceCreator.newInstance(InstanceCreator.java:38)
	... 30 more

Is the issue reproducible on runner?

• Shell
• Maven
• Gradle
• Ant
• Eclipse
• IntelliJ
• NetBeans

Test case sample

See above.

Thoughts on Issue

Debugging the code it seems that there is an issue where the @dataProvider annotation is processed in JDK15TagFactory which always winds up setting the dataProviderClass member to a non-null value even if the dataProviderClass annotation attribute is not set. The non-null value in this case happens to be the abstract class where the annotation is read. Later on this value gets passed to org.testng.internal.Parameters.findDataProvider as seen in the stack above. This method assumes that dataProviderClass should be null if the dataProviderClass member is not set (at least that's my interpretation given the doc on the annotation and the context of the code) and therefore sets the shouldBeStatic flag, this then forces an attempt to re-instantiate the class, instead of just using the already instantiated instance of the factory class, which is the else case e.g.

        if (shouldBeStatic && (m.getModifiers() & Modifier.STATIC) == 0) {
          IObjectDispenser dispenser = Dispenser.newInstance(objectFactory);
          BasicAttributes basic = new BasicAttributes(clazz, dataProviderClass);
          CreationAttributes attributes = new CreationAttributes(context, basic, null);
          instanceToUse = dispenser.dispense(attributes);
        } else {
          instanceToUse = instance;
        }

Unfortunately, I'm not sure what the actual solution is, since I may be missing some context around the Guice implementation stuff, but at least in this case the logic around setting shouldBeStatic seems wrong, and causes the test case above to fail.

Contribution guidelines

Incase you plan to raise a pull request to fix this issue, please make sure you refer our Contributing section for detailed set of steps.

[juherr]

Hi, thanks for the report but what is the usecase which justifies so much complexity in tests?

A quick fix could be to forbid such kind of usage.

[mbach979]

The real world use case that I have is that I am performing integration tests for a database metadata retrieval library against multiple database products. There are many tests that need to be run against all supported database platforms where each driver can possibly have many property configurations that effect metadata reporting. The connection information (JDBC URL, JDBC properties, etc.) for all the databases is maintained in a single reference source (CSV) for convenience, which is what is driving the @dataProvider use. The @factory annotation is being used to instantiate the test classes 1) to simply manage the cartesian nature, and 2) allow for database connection caching across tests to improve performance.

Further complicating matters is that database drivers often don't play well with each other on the same classpath due to jar hell issues. This often means that the tests need to be run from separate projects (gradle/eclipse) in order to avoid jar hell on the classpath at test runtime. This is what is necessitating having a base AbstractTestGenerator which in my case takes the configuration file and a filter pattern for the configurations, and then actual subclasses for each database type. e.g.

testUtils/AbstractTestGenerator.java

public abstract class AbstractTestGenerator
{
    public AbtractTestGenerator(String configFile, String filterPattern)
    {
    }
    
    @DataProvider(name="dataProvider")
    public Object[] dataProvider()
    {
        // read config file
    }
    
    @Factory(dataProvider="dataProvider")
    public Object[] testFactory(DatabaseConnectionInfo dbConnInfo)
    {
	   // generate tests
     }
}

oracleTests/OracleTestGenerator.java

public class OracleTestGenerator extends AbstractTestGenerator
{
     public OracleTestGenerator()
     {
         super("databaseConfigs.csv", "oracle")
     }
}
...

This actually does work btw already in TestNG but requires an unnecessary override of the testFactory method in the subclass i.e.:

public class OracleTestGenerator extends AbstractTestGenerator
{
     public OracleTestGenerator()
     {
         super("databaseConfigs.csv", "oracle")
     }

     @Factory(dataProvider="dataProvider")
     @Override
     public Object[] testFactory(DatabaseConnectionInfo dbConnInfo)
     {
	   return super.testFactory(dbConnInfo);
     }
}

But all of this is a bit besides the point, since TestNG already supports this, so I'm not sure why disabling/disallowing it would be a good idea. As mentioned in the initial report, the issue simply seems to lie in the dataProviderClass getting set to not null though never specified on the annotation, and then subsequent code relying on the documented contract that the data provider method should be static if that value is not null.