Streamline data provider listener invocation

Closes #3045

Author: krmahadevan

CHANGES.txt

@@ -1,6 +1,7 @@
 Current
 7.10.0
 Fixed: GITHUB-3059: Support the ability to inject custom listener factory (Krishnan Mahadevan)
+Fixed: GITHUB-3045: IDataProviderListener - beforeDataProviderExecution and afterDataProviderExecution are called twice in special setup (Krishnan Mahadevan)
 Fixed: GITHUB-3038: java.lang.IllegalStateException: Results per method should NOT have been empty (Krishnan Mahadevan)
 Fixed: GITHUB-3022: Remove deprecated JUnit related support in TestNG (Krishnan Mahadevan)
 

testng-core/src/main/java/org/testng/DataProviderHolder.java

@@ -2,6 +2,8 @@
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Map;
+import org.testng.collections.Maps;
 import org.testng.collections.Sets;
 
 /**
@@ -10,11 +12,11 @@
  */
 public class DataProviderHolder {
 
-  private final Collection<IDataProviderListener> listeners = Sets.newHashSet();
+  private final Map<Class<?>, IDataProviderListener> listeners = Maps.newConcurrentMap();
   private final Collection<IDataProviderInterceptor> interceptors = Sets.newHashSet();
 
   public Collection<IDataProviderListener> getListeners() {
-    return Collections.unmodifiableCollection(listeners);
+    return Collections.unmodifiableCollection(listeners.values());
   }
 
   public Collection<IDataProviderInterceptor> getInterceptors() {
@@ -26,7 +28,7 @@ public void addListeners(Collection<IDataProviderListener> listeners) {
   }
 
   public void addListener(IDataProviderListener listener) {
-    listeners.add(listener);
+    listeners.putIfAbsent(listener.getClass(), listener);
   }
 
   public void addInterceptors(Collection<IDataProviderInterceptor> interceptors) {
@@ -38,7 +40,7 @@ public void addInterceptor(IDataProviderInterceptor interceptor) {
   }
 
   public void merge(DataProviderHolder other) {
-    this.listeners.addAll(other.getListeners());
+    addListeners(other.getListeners());
     this.interceptors.addAll(other.getInterceptors());
   }
 }

testng-core/src/test/java/test/dataprovider/DataProviderTest.java

@@ -22,6 +22,8 @@
 import org.testng.internal.collections.Pair;
 import org.testng.internal.reflect.MethodMatcherException;
 import org.testng.xml.XmlClass;
+import org.testng.xml.XmlInclude;
+import org.testng.xml.XmlSuite;
 import org.testng.xml.XmlTest;
 import test.InvokedMethodNameListener;
 import test.SimpleBaseTest;
@@ -54,6 +56,9 @@
 import test.dataprovider.issue2934.TestCaseSample.CoreListener;
 import test.dataprovider.issue2934.TestCaseSample.ToggleDataProvider;
 import test.dataprovider.issue2980.LoggingListener;
+import test.dataprovider.issue3045.DataProviderListener;
+import test.dataprovider.issue3045.DataProviderTestClassSample;
+import test.dataprovider.issue3045.DataProviderWithoutListenerTestClassSample;
 
 public class DataProviderTest extends SimpleBaseTest {
 
@@ -666,6 +671,48 @@ public Object[][] getSuiteFileNames() {
     };
   }
 
+  @Test(description = "GITHUB-3045")
+  public void testIfDataProviderListenerInvokedOnlyOncePerDataProvider() {
+    runTest(DataProviderTestClassSample.class, "");
+  }
+
+  @Test(description = "GITHUB-3045")
+  public void testIfDataProviderListenerInvokedOnlyOncePerDataProviderWhenListenerAddedViaSuite() {
+    runTest(DataProviderWithoutListenerTestClassSample.class, DataProviderListener.class.getName());
+  }
+
+  private static void runTest(Class<?> clazz, String listener) {
+    DataProviderListener.logs.clear();
+    TestNG testng = new TestNG();
+    XmlSuite xmlSuite = new XmlSuite();
+    if (!listener.isEmpty()) {
+      xmlSuite.addListener(DataProviderListener.class.getName());
+    }
+    xmlSuite.setName("suite1");
+    xmlTest(xmlSuite, clazz, "Test1", "normalTest");
+    xmlTest(xmlSuite, clazz, "Test2", "dataDrivenTest");
+    testng.setXmlSuites(Collections.singletonList(xmlSuite));
+    testng.setVerbose(2);
+    testng.run();
+    assertThat(DataProviderListener.logs).hasSize(2);
+    assertThat(DataProviderListener.logs)
+        .containsExactly(
+            "[Test2]-beforeDataProviderExecution-dataProvider",
+            "[Test2]-afterDataProviderExecution-dataProvider");
+  }
+
+  private static void xmlTest(XmlSuite xmlSuite, Class<?> clazz, String name, String methodName) {
+    XmlTest xmlTest = new XmlTest(xmlSuite);
+    xmlTest.setName(name);
+    xmlTest.setXmlClasses(List.of(xmlClass(clazz, methodName)));
+  }
+
+  private static XmlClass xmlClass(Class<?> clazz, String methodName) {
+    XmlClass xmlClass = new XmlClass(clazz);
+    xmlClass.setIncludedMethods(List.of(new XmlInclude(methodName)));
+    return xmlClass;
+  }
+
   private LoggingListener runDataProviderTest(boolean flag) {
     TestNG testng = new TestNG();
     testng.setTestClasses(new Class[] {test.dataprovider.issue2980.TestClassSample.class});

testng-core/src/test/java/test/dataprovider/issue3045/DataProviderListener.java

@@ -0,0 +1,35 @@
+package test.dataprovider.issue3045;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.testng.IDataProviderListener;
+import org.testng.IDataProviderMethod;
+import org.testng.ITestContext;
+import org.testng.ITestNGMethod;
+
+public class DataProviderListener implements IDataProviderListener {
+
+  public static List<String> logs = new ArrayList<>();
+
+  @Override
+  public void beforeDataProviderExecution(
+      IDataProviderMethod dataProviderMethod, ITestNGMethod method, ITestContext iTestContext) {
+    logs.add(
+        testName(iTestContext)
+            + "-beforeDataProviderExecution-"
+            + dataProviderMethod.getMethod().getName());
+  }
+
+  @Override
+  public void afterDataProviderExecution(
+      IDataProviderMethod dataProviderMethod, ITestNGMethod method, ITestContext iTestContext) {
+    logs.add(
+        testName(iTestContext)
+            + "-afterDataProviderExecution-"
+            + dataProviderMethod.getMethod().getName());
+  }
+
+  private static String testName(ITestContext ctx) {
+    return "[" + ctx.getName() + "]";
+  }
+}

testng-core/src/test/java/test/dataprovider/issue3045/DataProviderTestClassSample.java

@@ -0,0 +1,20 @@
+package test.dataprovider.issue3045;
+
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
+
+@Listeners({DataProviderListener.class})
+public class DataProviderTestClassSample {
+
+  @DataProvider
+  public Object[][] dataProvider() {
+    return new Object[][] {{"Test_1"}, {"Test_2"}, {"Test_3"}};
+  }
+
+  @Test(dataProvider = "dataProvider")
+  public void dataDrivenTest(String ignored) {}
+
+  @Test
+  public void normalTest() {}
+}

testng-core/src/test/java/test/dataprovider/issue3045/DataProviderWithoutListenerTestClassSample.java

@@ -0,0 +1,18 @@
+package test.dataprovider.issue3045;
+
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class DataProviderWithoutListenerTestClassSample {
+
+  @DataProvider
+  public Object[][] dataProvider() {
+    return new Object[][] {{"Test_1"}, {"Test_2"}, {"Test_3"}};
+  }
+
+  @Test(dataProvider = "dataProvider")
+  public void dataDrivenTest(String ignored) {}
+
+  @Test
+  public void normalTest() {}
+}