DataProvider: possibility to unload dataprovider class, when done with it #2739

dsankouski · 2022-03-10T13:31:04Z

With current Data providers implementation, it's code will stick around
in method area (JVM spec $2.5.4) for entire test run.

By specifying dataprovider class with it's full qualified name, and
by using new custom classloader to load it, when needed, JVM gets a
chance to unload dataprovider class, when we're done and deleted
all links to it

Fixes #2724 .

Did you remember to?

Add test case(s)
Update CHANGES.txt
Auto applied styling via ./gradlew autostyleApply

krmahadevan · 2022-03-11T06:44:11Z

@dsankouski - There are failures because the code style was not applied

Please Run ./gradlew autostyleApply to fix the violations and push in the changes

krmahadevan

@dsankouski - I still have the basic doubt around the following in this PR:

When I provide a string version of my class as my data provider class, does it mean that the class exists in my class path but I am just not loading it before hand and will trigger the loading only when I run the data driven test which uses this class?

testng-core/src/main/java/org/testng/internal/DataProviderLoader.java

testng-core/src/main/java/org/testng/internal/Parameters.java

testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java

testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java

testng-core/testng-core-build.gradle.kts

testng-core/src/main/java/org/testng/internal/DataProviderLoader.java

krmahadevan · 2022-03-11T08:49:56Z

testng-core/src/main/java/org/testng/internal/Parameters.java

+          && !dp.getDataProviderDynamicClass().isEmpty();
+      if (isDynamicDataProvider) {
+        try {
+          dataProviderClass = new DataProviderLoader()


We are creating on the fly instances of DataProviderLoader() to load the class that was specified as a string attribute in the @Test annotation. This means that this object of DataProviderLoader() immediately becomes eligible for a GC. So if the loader object gets GC'ed then all the classes that it caused to load would be unloaded as well.

So what is going to be the expected behaviour when the DataProviderLoader object got GC'ed even before a data provider was actually invoked? Wouldn't that cause ClassNotFoundException or NoClassDefFoundError for the test case which is using a dynamic data provider ?

Class holds a reference to it's classloader. So, in order to garbage collect a classloader, all class instances should be collected first.

We are clearing only the instance. So does it mean that when all instances associated with a class are GC'ed the corresponding class also would get GC'ed from the perm space and thus cause the ClassLoader also to do the same ?

That's right.

testng-core/src/main/java/org/testng/internal/DataProviderLoader.java

krmahadevan · 2022-04-20T03:16:15Z

@dsankouski - My bad for getting back late on this. Can you please help fix the merge conflicts and update this PR branch? Lets work on getting this merged asap.

juherr

The new tests are failling on j9 jvm. They should be fixed or skipped when j9 is used.

krmahadevan · 2022-04-21T03:30:06Z

@dsankouski

Can we please have this fixed ? https://github.com/cbeust/testng/runs/6096462677?check_suite_focus=true

org.testng.dataprovider.DynamicDataProviderLoadingTest > testDynamicDataProviderUnloaded failure marker
  Error:   0.2sec org.testng.dataprovider.DynamicDataProviderLoadingTest > testDynamicDataProviderUnloaded
  java.lang.IllegalArgumentException: javax.management.InstanceNotFoundException: com.sun.management:type=HotSpotDiagnostic
      at java.management/java.lang.management.ManagementFactory.newPlatformMXBeanProxy(ManagementFactory.java:623)
      at test.SimpleBaseTest$Companion.saveMemDump(SimpleBaseTest.kt:392)
      at org.testng.dataprovider.DynamicDataProviderLoadingTest.testDynamicDataProviderUnloaded(DynamicDataProviderLoadingTest.kt:40)
      Caused by: javax.management.InstanceNotFoundException: com.sun.management:type=HotSpotDiagnostic
          at java.management/com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.getMBean(DefaultMBeanServerInterceptor.java:1083)
          at java.management/com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.isInstanceOf(DefaultMBeanServerInterceptor.java:1379)
          at java.management/com.sun.jmx.mbeanserver.JmxMBeanServer.isInstanceOf(JmxMBeanServer.java:1091)
          at java.management/java.lang.management.ManagementFactory.isInstanceOf(ManagementFactory.java:651)
          at java.management/java.lang.management.ManagementFactory.newPlatformMXBeanProxy(ManagementFactory.java:611)
          ... 2 more

…h it With current Data providers implementation, it's code will stick around in method area (JVM spec $2.5.4) for the entire test run. By specifying dataprovider class with it's full qualified name, and by using new custom classloader to load it, when needed, JVM gets a chance to unload dataprovider class, when we're done with it. Testing dataprovider class unload is performed by analysing memory dumps. Also, there is a test(comparePerformanceAgainstCsvFiles) to measure performance of data as code approach against common data approch, where data is stored in csv files.

dsankouski · 2022-04-21T07:32:27Z

@krmahadevan @juherr, throw SkipException, in saveMemDump method, if jvm is not HotSpot

dsankouski requested review from juherr and krmahadevan as code owners March 10, 2022 13:31

krmahadevan reviewed Mar 11, 2022

View reviewed changes

testng-core/src/main/java/org/testng/internal/DataProviderLoader.java Outdated Show resolved Hide resolved

krmahadevan reviewed Mar 11, 2022

View reviewed changes

dsankouski force-pushed the dynamic_loading_dataprovider_class branch 2 times, most recently from 98475c7 to a0b7229 Compare March 12, 2022 15:44

krmahadevan reviewed Mar 15, 2022

View reviewed changes

testng-core/src/main/java/org/testng/internal/DataProviderLoader.java Show resolved Hide resolved

krmahadevan reviewed Mar 15, 2022

View reviewed changes

testng-core/src/main/java/org/testng/internal/DataProviderLoader.java Show resolved Hide resolved

krmahadevan reviewed Mar 15, 2022

View reviewed changes

testng-core/src/main/java/org/testng/internal/DataProviderLoader.java Show resolved Hide resolved

dsankouski force-pushed the dynamic_loading_dataprovider_class branch from 56f0713 to 2112e44 Compare March 28, 2022 16:55

dsankouski force-pushed the dynamic_loading_dataprovider_class branch from 2112e44 to 8bdc42b Compare April 20, 2022 14:07

dsankouski changed the title ~~WIP: DataProvider: possibility to dynamically load dataprovider class~~ DataProvider: possibility to unload dataprovider class, when done with it Apr 20, 2022

juherr requested changes Apr 21, 2022

View reviewed changes

dsankouski force-pushed the dynamic_loading_dataprovider_class branch from 8bdc42b to c1c74b6 Compare April 21, 2022 07:29

juherr approved these changes Apr 21, 2022

View reviewed changes

krmahadevan approved these changes Apr 21, 2022

View reviewed changes

krmahadevan merged commit aae0724 into testng-team:master Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DataProvider: possibility to unload dataprovider class, when done with it #2739

DataProvider: possibility to unload dataprovider class, when done with it #2739

dsankouski commented Mar 10, 2022 •

edited

Loading

krmahadevan commented Mar 11, 2022

krmahadevan left a comment

krmahadevan Mar 11, 2022

dsankouski Mar 12, 2022

krmahadevan Mar 15, 2022

dsankouski Mar 17, 2022

krmahadevan commented Apr 20, 2022

juherr left a comment

krmahadevan commented Apr 21, 2022

dsankouski commented Apr 21, 2022

DataProvider: possibility to unload dataprovider class, when done with it #2739

DataProvider: possibility to unload dataprovider class, when done with it #2739

Conversation

dsankouski commented Mar 10, 2022 • edited Loading

Did you remember to?

krmahadevan commented Mar 11, 2022

krmahadevan left a comment

Choose a reason for hiding this comment

krmahadevan Mar 11, 2022

Choose a reason for hiding this comment

dsankouski Mar 12, 2022

Choose a reason for hiding this comment

krmahadevan Mar 15, 2022

Choose a reason for hiding this comment

dsankouski Mar 17, 2022

Choose a reason for hiding this comment

krmahadevan commented Apr 20, 2022

juherr left a comment

Choose a reason for hiding this comment

krmahadevan commented Apr 21, 2022

dsankouski commented Apr 21, 2022

dsankouski commented Mar 10, 2022 •

edited

Loading